From de8a59afdb98cc44ddbfd9b19e8b12aa5ee0fd6a Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:34:34 -0700 Subject: [PATCH 01/15] docs: design for MCP connection-gated tool discovery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...2026-06-08-mcp-connection-gating-design.md | 217 ++++++++++++++++++ 1 file changed, 217 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md diff --git a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md new file mode 100644 index 00000000..73bc84d7 --- /dev/null +++ b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md @@ -0,0 +1,217 @@ +# MCP Connection-Gated Tool Discovery — Design + +**Date:** 2026-06-08 +**Status:** Proposed +**Package(s):** `microsoft-agents-a365-tooling` (core); framework tooling extensions inherit behavior. + +## Problem + +The tooling gateway discovery response for MCP servers has gained connection-state +fields. A server may be configured for an agent but not yet have its required +downstream connections (e.g. a Salesforce or Zendesk connector) established by the +user. If the agent runs tools against such a server, calls fail at execution time. + +The runtime calls discovery on **every turn** when spinning up tools (there is no +caching of the server list — discovery re-runs each time `list_tool_servers` is +invoked). We want to use that per-turn discovery to **gate agent execution**: until +all required connections are present, the turn must not proceed. Instead the agent +should reply to the user with a link to set up the missing connections, and a later +turn (after the user connects) proceeds normally. + +## New Schema + +The wrapped gateway response now carries connection metadata at **two levels** — +per-server and response-level (aggregate): + +```json +{ + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", + "publisher": "Microsoft", + "url": "https://agent365.svc.cloud.dev.microsoft/agents/v2/servers/mcp_Salesforce", + "scope": "McpServers.Salesforce.All", + "audience": "", + "allConnectionsUrl": "", + "missingConnectionsUrl": "", + "connectivityStatus": "" + } + ], + "allConnectionsUrl": "", + "missingConnectionsUrl": "", + "connectivityStatus": "" +} +``` + +Notes: +- The **response-level** `connectivityStatus` / `missingConnectionsUrl` / + `allConnectionsUrl` are the authoritative aggregate signal used for gating and for + the single "fix everything" link surfaced to the user. +- The **per-server** copies are parsed and retained for diagnostics only; they do not + drive the gate decision. +- Key casing has been observed as both `connectivityStatus` and `ConnectivityStatus`. + Parsing is tolerant of known key-name variants. **Open item:** confirm the exact + JSON key casing with the gateway team and tighten if possible. + +## Compatibility Rules + +| Source | Aggregate `connectivityStatus` | Gate behavior | +|---|---|---| +| V2 gateway, all connected | `"Connected"` | proceeds | +| V2 gateway, missing connections | not `"Connected"` (e.g. `"Pending"`) | **blocks**, raises exception | +| V1 wrapped gateway (legacy) | always `"Connected"` | proceeds | +| Legacy raw-array gateway response | absent → `None` | proceeds (not gated) | +| Dev mode (`ToolingManifest.json`) | absent → `None` | proceeds (not gated) | + +**Gate rule:** block only when the aggregate `connectivity_status` is **present and +not equal to** `"Connected"`. Absent status is always treated as ready, so dev mode +and legacy callers are unaffected. + +## Design + +### 1. Model — `MCPServerConfig` + +Add four optional fields (all default `None`, preserving existing constructor calls): + +- `id: Optional[str]` +- `all_connections_url: Optional[str]` +- `missing_connections_url: Optional[str]` +- `connectivity_status: Optional[str]` + +These hold the **per-server** values. No validation change. + +### 2. Internal discovery result — `McpDiscoveryResult` + +New internal dataclass (not part of the public return type) to carry response-level +aggregate metadata alongside the parsed servers: + +```python +@dataclass +class McpDiscoveryResult: + servers: List[MCPServerConfig] + all_connections_url: Optional[str] = None + missing_connections_url: Optional[str] = None + connectivity_status: Optional[str] = None +``` + +### 3. Parsing changes + +- `_parse_server_config` reads `id`, `allConnectionsUrl`, `missingConnectionsUrl`, + `connectivityStatus` (tolerant casing) onto each `MCPServerConfig`. Gateway and + manifest share this path; manifest entries simply lack the fields → `None`. +- `_parse_gateway_response` returns `McpDiscoveryResult`: it parses the server list as + today and additionally extracts the **response-level** aggregate fields when the + wrapped `{"mcpServers": [...]}` shape is present. The legacy raw-array shape yields a + result with aggregate fields = `None`. +- `_load_servers_from_gateway` returns `McpDiscoveryResult`. +- The manifest/dev path wraps its server list in a `McpDiscoveryResult` with aggregate + fields = `None`. + +### 4. Readiness gate — core `list_tool_servers` + +After discovery and per-audience token attachment, evaluate the aggregate status: + +```python +if (result.connectivity_status is not None + and result.connectivity_status != "Connected"): + not_ready = [s for s in result.servers + if s.connectivity_status is not None + and s.connectivity_status != "Connected"] + raise McpConnectionsRequiredError( + missing_connections_url=result.missing_connections_url, + all_connections_url=result.all_connections_url, + connectivity_status=result.connectivity_status, + server_names=[s.mcp_server_name or s.mcp_server_unique_name + for s in not_ready], + ) +``` + +The public return type of `list_tool_servers` remains `List[MCPServerConfig]` +(`result.servers`). The gate is the only new externally observable behavior, and only +fires for V2 gateway responses with unsatisfied connections. + +### 5. Exception — `McpConnectionsRequiredError` + +New exception type in the tooling package, exported from +`microsoft_agents_a365.tooling`: + +```python +class McpConnectionsRequiredError(Exception): + def __init__( + self, + missing_connections_url: Optional[str], + all_connections_url: Optional[str], + connectivity_status: Optional[str], + server_names: List[str], + ) -> None: + ... +``` + +Exposes (per design decision) the **response-level** aggregate links plus the list of +not-`Connected` server names for context: +- `missing_connections_url` — single aggregate link to set up missing connections + (primary value the handler surfaces to the user). +- `all_connections_url` — aggregate link to view/manage all connections. +- `connectivity_status` — the aggregate status that triggered the block. +- `server_names` — names of servers that are not yet `Connected`. + +The `__str__`/message includes the missing-connections URL and server names so logs +are actionable. + +### 6. Per-turn behavior & propagation + +- No poll loop. Discovery already runs every turn (no list caching), so each turn + re-hits the gateway. Once the user completes the connections, a subsequent turn + passes the gate. +- Framework tooling extensions (`add_tool_servers_to_agent` in openai, + semantickernel, agentframework, googleadk, azureaifoundry) **do not catch** + `McpConnectionsRequiredError` — they let it propagate. +- The **agent's turn handler** (application code) catches it, replies to the user with + the `missing_connections_url`, and returns without running the model/tools. This + reply formatting is application responsibility; the SDK only signals and supplies the + URLs. Documentation/sample will show the recommended catch-and-reply pattern. + +## Out of Scope (YAGNI) + +- Polling / blocking wait loops within a turn. +- Partial execution with only the connected servers (decision: block entirely on any + unsatisfied aggregate status). +- The SDK itself sending the connection-setup reply (left to the turn handler). +- Caching discovery results across turns. + +## Testing + +- **Parser:** per-server new fields populated from a V2 element; absent in manifest + element → `None`. Tolerant casing variants map to the same field. +- **Aggregate parsing:** `_parse_gateway_response` extracts response-level fields for + wrapped shape; raw-array shape → aggregate `None`. +- **Gate fires:** aggregate `connectivity_status` not `"Connected"` raises + `McpConnectionsRequiredError` carrying the correct response-level URLs and the + not-Connected server names. +- **Gate passes:** aggregate `"Connected"`; aggregate absent (dev manifest); legacy + raw array; V1 wrapped all-`Connected`. +- **Exception payload:** `missing_connections_url` / `all_connections_url` / + `connectivity_status` / `server_names` correct; message string actionable. +- **Propagation:** extension `add_tool_servers_to_agent` does not swallow the exception + (it surfaces to the caller). + +## Affected Files + +- `libraries/microsoft-agents-a365-tooling/.../models/mcp_server_config.py` — new fields. +- `libraries/microsoft-agents-a365-tooling/.../models/__init__.py` — export + `McpDiscoveryResult` if placed in models (or keep internal to the service module). +- `libraries/microsoft-agents-a365-tooling/.../services/mcp_tool_server_configuration_service.py` + — parsing + gate. +- `libraries/microsoft-agents-a365-tooling/.../exceptions.py` (new) — + `McpConnectionsRequiredError`. +- `libraries/microsoft-agents-a365-tooling/.../__init__.py` — export exception. +- `tests/tooling/...` — parser, gate, exception tests. +- `libraries/microsoft-agents-a365-tooling/CHANGELOG.md` — changelog entry. +- Tooling extension docs/samples — catch-and-reply pattern (follow-up). + +## Open Items + +1. Confirm exact JSON key casing for `connectivityStatus` (response and server level). +2. Confirm the exact "ready" sentinel value is `"Connected"` (and any other terminal + states that should count as ready). From 76ef2c4d31643c4a97b33442b99054431deac771 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:36:20 -0700 Subject: [PATCH 02/15] docs: correct connectivityStatus values to Ready/Pending Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...2026-06-08-mcp-connection-gating-design.md | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md index 73bc84d7..8167096f 100644 --- a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md +++ b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md @@ -58,15 +58,19 @@ Notes: | Source | Aggregate `connectivityStatus` | Gate behavior | |---|---|---| -| V2 gateway, all connected | `"Connected"` | proceeds | -| V2 gateway, missing connections | not `"Connected"` (e.g. `"Pending"`) | **blocks**, raises exception | -| V1 wrapped gateway (legacy) | always `"Connected"` | proceeds | +| V2 gateway, all connections satisfied | `"Ready"` | proceeds | +| V2 gateway, missing connections | `"Pending"` | **blocks**, raises exception | | Legacy raw-array gateway response | absent → `None` | proceeds (not gated) | | Dev mode (`ToolingManifest.json`) | absent → `None` | proceeds (not gated) | +The gateway only ever emits `"Ready"` or `"Pending"` for `connectivityStatus` — never +`null` and never any other value. Sources that predate the field (legacy raw-array +responses, dev-mode manifests) omit it entirely, yielding `None`. + **Gate rule:** block only when the aggregate `connectivity_status` is **present and -not equal to** `"Connected"`. Absent status is always treated as ready, so dev mode -and legacy callers are unaffected. +not equal to** `"Ready"` (i.e. `"Pending"`). Absent status (`None`) is always treated +as ready, so dev mode and legacy callers are unaffected. The `!= "Ready"` form (rather +than `== "Pending"`) is deliberately defensive against any unexpected future value. ## Design @@ -114,10 +118,10 @@ After discovery and per-audience token attachment, evaluate the aggregate status ```python if (result.connectivity_status is not None - and result.connectivity_status != "Connected"): + and result.connectivity_status != "Ready"): not_ready = [s for s in result.servers if s.connectivity_status is not None - and s.connectivity_status != "Connected"] + and s.connectivity_status != "Ready"] raise McpConnectionsRequiredError( missing_connections_url=result.missing_connections_url, all_connections_url=result.all_connections_url, @@ -129,7 +133,7 @@ if (result.connectivity_status is not None The public return type of `list_tool_servers` remains `List[MCPServerConfig]` (`result.servers`). The gate is the only new externally observable behavior, and only -fires for V2 gateway responses with unsatisfied connections. +fires for gateway responses reporting `connectivityStatus: "Pending"`. ### 5. Exception — `McpConnectionsRequiredError` @@ -149,12 +153,12 @@ class McpConnectionsRequiredError(Exception): ``` Exposes (per design decision) the **response-level** aggregate links plus the list of -not-`Connected` server names for context: +not-`Ready` server names for context: - `missing_connections_url` — single aggregate link to set up missing connections (primary value the handler surfaces to the user). - `all_connections_url` — aggregate link to view/manage all connections. -- `connectivity_status` — the aggregate status that triggered the block. -- `server_names` — names of servers that are not yet `Connected`. +- `connectivity_status` — the aggregate status that triggered the block (`"Pending"`). +- `server_names` — names of servers that are not yet `Ready`. The `__str__`/message includes the missing-connections URL and server names so logs are actionable. @@ -186,11 +190,11 @@ are actionable. element → `None`. Tolerant casing variants map to the same field. - **Aggregate parsing:** `_parse_gateway_response` extracts response-level fields for wrapped shape; raw-array shape → aggregate `None`. -- **Gate fires:** aggregate `connectivity_status` not `"Connected"` raises +- **Gate fires:** aggregate `connectivity_status == "Pending"` raises `McpConnectionsRequiredError` carrying the correct response-level URLs and the - not-Connected server names. -- **Gate passes:** aggregate `"Connected"`; aggregate absent (dev manifest); legacy - raw array; V1 wrapped all-`Connected`. + not-Ready server names. +- **Gate passes:** aggregate `"Ready"`; aggregate absent (dev manifest); legacy + raw array. - **Exception payload:** `missing_connections_url` / `all_connections_url` / `connectivity_status` / `server_names` correct; message string actionable. - **Propagation:** extension `add_tool_servers_to_agent` does not swallow the exception @@ -213,5 +217,6 @@ are actionable. ## Open Items 1. Confirm exact JSON key casing for `connectivityStatus` (response and server level). -2. Confirm the exact "ready" sentinel value is `"Connected"` (and any other terminal - states that should count as ready). +2. Confirmed: `connectivityStatus` is always `"Ready"` or `"Pending"` (never `null` or + other values) from the V2 gateway. Field is absent only from legacy raw-array + responses and dev-mode manifests. From 780f9a433d44d4922fe37fb173b2cd87fa3a2abf Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:42:10 -0700 Subject: [PATCH 03/15] docs: confirm camelCase keys, resolve open items Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...2026-06-08-mcp-connection-gating-design.md | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md index 8167096f..d30028aa 100644 --- a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md +++ b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md @@ -50,9 +50,9 @@ Notes: the single "fix everything" link surfaced to the user. - The **per-server** copies are parsed and retained for diagnostics only; they do not drive the gate decision. -- Key casing has been observed as both `connectivityStatus` and `ConnectivityStatus`. - Parsing is tolerant of known key-name variants. **Open item:** confirm the exact - JSON key casing with the gateway team and tighten if possible. +- All JSON keys are camelCase: `connectivityStatus`, `allConnectionsUrl`, + `missingConnectionsUrl`, `id`, `mcpServerName`. Parsing uses these exact keys (no + case-variant fallback needed). ## Compatibility Rules @@ -60,12 +60,10 @@ Notes: |---|---|---| | V2 gateway, all connections satisfied | `"Ready"` | proceeds | | V2 gateway, missing connections | `"Pending"` | **blocks**, raises exception | -| Legacy raw-array gateway response | absent → `None` | proceeds (not gated) | -| Dev mode (`ToolingManifest.json`) | absent → `None` | proceeds (not gated) | +| Legacy `ToolingManifest.json` | absent → `None` | proceeds (not gated) | The gateway only ever emits `"Ready"` or `"Pending"` for `connectivityStatus` — never -`null` and never any other value. Sources that predate the field (legacy raw-array -responses, dev-mode manifests) omit it entirely, yielding `None`. +`null` and never any other value. Sources that predate the field (legacy manifests) omit it entirely, yielding `None`. **Gate rule:** block only when the aggregate `connectivity_status` is **present and not equal to** `"Ready"` (i.e. `"Pending"`). Absent status (`None`) is always treated @@ -102,7 +100,7 @@ class McpDiscoveryResult: ### 3. Parsing changes - `_parse_server_config` reads `id`, `allConnectionsUrl`, `missingConnectionsUrl`, - `connectivityStatus` (tolerant casing) onto each `MCPServerConfig`. Gateway and + `connectivityStatus` (exact camelCase key) onto each `MCPServerConfig`. Gateway and manifest share this path; manifest entries simply lack the fields → `None`. - `_parse_gateway_response` returns `McpDiscoveryResult`: it parses the server list as today and additionally extracts the **response-level** aggregate fields when the @@ -187,7 +185,7 @@ are actionable. ## Testing - **Parser:** per-server new fields populated from a V2 element; absent in manifest - element → `None`. Tolerant casing variants map to the same field. + element → `None`. - **Aggregate parsing:** `_parse_gateway_response` extracts response-level fields for wrapped shape; raw-array shape → aggregate `None`. - **Gate fires:** aggregate `connectivity_status == "Pending"` raises @@ -214,9 +212,11 @@ are actionable. - `libraries/microsoft-agents-a365-tooling/CHANGELOG.md` — changelog entry. - Tooling extension docs/samples — catch-and-reply pattern (follow-up). -## Open Items +## Resolved Decisions -1. Confirm exact JSON key casing for `connectivityStatus` (response and server level). -2. Confirmed: `connectivityStatus` is always `"Ready"` or `"Pending"` (never `null` or - other values) from the V2 gateway. Field is absent only from legacy raw-array - responses and dev-mode manifests. +1. JSON keys are camelCase (`connectivityStatus`, `allConnectionsUrl`, + `missingConnectionsUrl`, etc.) at both response and server level — parsed with exact + keys. +2. `connectivityStatus` is always `"Ready"` or `"Pending"` (never `null` or other + values) from the gateway. Field is absent only from legacy raw-array responses and + dev-mode manifests. From 07aee15435a817ad2bf2bebae3c9e5bb6699e028 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:49:35 -0700 Subject: [PATCH 04/15] docs: implementation plan for MCP connection gating Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../plans/2026-06-08-mcp-connection-gating.md | 943 ++++++++++++++++++ 1 file changed, 943 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-08-mcp-connection-gating.md diff --git a/docs/superpowers/plans/2026-06-08-mcp-connection-gating.md b/docs/superpowers/plans/2026-06-08-mcp-connection-gating.md new file mode 100644 index 00000000..fddda577 --- /dev/null +++ b/docs/superpowers/plans/2026-06-08-mcp-connection-gating.md @@ -0,0 +1,943 @@ +# MCP Connection-Gated Tool Discovery Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Block agent execution when a configured MCP server reports `connectivityStatus: "Pending"`, raising a typed error carrying the connection-setup URLs so the agent's turn handler can prompt the user instead of running tools that would fail. + +**Architecture:** Extend `MCPServerConfig` with the new per-server connection fields; capture the response-level (aggregate) connection metadata into a new internal `McpDiscoveryResult`; in core `McpToolServerConfigurationService.list_tool_servers`, raise `McpConnectionsRequiredError` when the aggregate status is present and not `"Ready"`. Dev-mode manifests and legacy raw-array responses omit the field → never gated. Framework extensions let the exception propagate to the turn handler. + +**Tech Stack:** Python 3.11+, `uv`, `pytest`, `ruff`. Package: `microsoft-agents-a365-tooling`. + +**Spec:** `docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md` + +--- + +## Background / orientation (read once) + +Key file: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` + +Relevant existing flow inside `list_tool_servers`: +- Dev: `servers = self._load_servers_from_manifest()` (returns `List[MCPServerConfig]`). +- Prod: `discovery = await self._load_servers_from_gateway(...)` then either OBO token + acquirer or a legacy V2-guard early-return. +- Both end with `_attach_per_audience_tokens(...)` then `return servers`. + +Parsing helpers: +- `_parse_server_config(server_element)` — maps one JSON object (gateway **or** manifest) + to `MCPServerConfig`. Lines ~685–741. +- `_parse_gateway_response(response)` — currently returns `List[MCPServerConfig]`; + handles wrapped `{"mcpServers": [...]}` and legacy raw-array shapes. Lines ~639–679. + +**Gate placement decision (refinement over spec wording):** enforce the gate in +`list_tool_servers` immediately after the gateway discovery call returns, *before* token +attachment and before the auth-context branching. Connection readiness is independent of +token exchange, so gating first also avoids unnecessary OBO exchanges when connections +aren't ready. The dev/manifest path is never gated (no aggregate field). + +**Conventions to follow:** +- Copyright header on every `.py` file (ruff `CPY` rule): + ```python + # Copyright (c) Microsoft Corporation. + # Licensed under the MIT License. + ``` +- Type hints on all params/returns. Never use `typing.Any`. Use `is not None` for None checks. +- Run tests: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py -v` +- Lint/format: `uv run --frozen ruff check .` and `uv run --frozen ruff format .` + +--- + +## File Structure + +- **Modify** `.../tooling/models/mcp_server_config.py` — add 4 optional fields. +- **Create** `.../tooling/exceptions.py` — `McpConnectionsRequiredError`. +- **Modify** `.../tooling/__init__.py` — export the exception. +- **Modify** `.../tooling/services/mcp_tool_server_configuration_service.py` — add + `McpDiscoveryResult`, per-server field parsing, aggregate parsing, gate enforcement. +- **Modify** `tests/tooling/test_mcp_server_configuration.py` — new tests. +- **Create** `tests/tooling/test_mcp_connections_required_error.py` — exception tests. +- **Modify** `.../tooling/CHANGELOG.md` — changelog entry. +- **Modify** `.../tooling/docs/design.md` — document the catch-and-reply pattern. + +Path prefix for all `.../tooling/...` entries: +`libraries/microsoft-agents-a365-tooling/microsoft_agents_a365` + +--- + +## Task 1: Add connection fields to `MCPServerConfig` + +**Files:** +- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py` +- Test: `tests/tooling/test_mcp_server_configuration.py` + +- [ ] **Step 1: Write the failing test** + +Add to class `TestMCPServerConfig` in `tests/tooling/test_mcp_server_configuration.py`: + +```python + def test_mcp_server_config_connection_fields_default_none(self): + """Connection fields default to None for backward compatibility.""" + config = MCPServerConfig( + mcp_server_name="TestServer", + mcp_server_unique_name="test_server", + ) + assert config.id is None + assert config.all_connections_url is None + assert config.missing_connections_url is None + assert config.connectivity_status is None + + def test_mcp_server_config_connection_fields_set(self): + """Connection fields are stored when provided.""" + config = MCPServerConfig( + mcp_server_name="TestServer", + mcp_server_unique_name="test_server", + id="3fa85f64-5717-4562-b3fc-2c963f66afa6", + all_connections_url="https://make.example/all", + missing_connections_url="https://make.example/missing", + connectivity_status="Pending", + ) + assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" + assert config.all_connections_url == "https://make.example/all" + assert config.missing_connections_url == "https://make.example/missing" + assert config.connectivity_status == "Pending" +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMCPServerConfig::test_mcp_server_config_connection_fields_set -v` +Expected: FAIL with `TypeError: __init__() got an unexpected keyword argument 'id'`. + +- [ ] **Step 3: Add the fields to the dataclass** + +In `mcp_server_config.py`, add these fields inside the `MCPServerConfig` dataclass, +after the existing `publisher` field (keep all new fields optional with `None` defaults): + +```python + #: Unique identifier (GUID) of the MCP server from the gateway, if provided. + id: Optional[str] = None + + #: Per-server URL to view/manage all connections for this server's connector. + all_connections_url: Optional[str] = None + + #: Per-server URL to set up the connections this server is missing. + missing_connections_url: Optional[str] = None + + #: Per-server connectivity status reported by the gateway ("Ready" or "Pending"). + #: None when the source predates the field (dev manifest / legacy raw-array gateway). + connectivity_status: Optional[str] = None +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMCPServerConfig -v` +Expected: PASS (all tests in the class). + +- [ ] **Step 5: Commit** + +```bash +git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py tests/tooling/test_mcp_server_configuration.py +git commit -m "feat(tooling): add connection fields to MCPServerConfig + +Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" +``` + +--- + +## Task 2: Add `McpConnectionsRequiredError` exception + +**Files:** +- Create: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py` +- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py` +- Test: `tests/tooling/test_mcp_connections_required_error.py` + +- [ ] **Step 1: Write the failing test** + +Create `tests/tooling/test_mcp_connections_required_error.py`: + +```python +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Unit tests for McpConnectionsRequiredError.""" + +from microsoft_agents_a365.tooling import McpConnectionsRequiredError + + +def test_exception_exposes_payload(): + err = McpConnectionsRequiredError( + missing_connections_url="https://make.example/missing", + all_connections_url="https://make.example/all", + connectivity_status="Pending", + server_names=["mcp_Salesforce", "mcp_Zendesk"], + ) + assert err.missing_connections_url == "https://make.example/missing" + assert err.all_connections_url == "https://make.example/all" + assert err.connectivity_status == "Pending" + assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] + + +def test_exception_message_is_actionable(): + err = McpConnectionsRequiredError( + missing_connections_url="https://make.example/missing", + all_connections_url=None, + connectivity_status="Pending", + server_names=["mcp_Salesforce"], + ) + message = str(err) + assert "mcp_Salesforce" in message + assert "https://make.example/missing" in message + + +def test_exception_is_exception_subclass(): + assert issubclass(McpConnectionsRequiredError, Exception) +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_connections_required_error.py -v` +Expected: FAIL with `ImportError: cannot import name 'McpConnectionsRequiredError'`. + +- [ ] **Step 3: Create the exception module** + +Create `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py`: + +```python +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Exceptions raised by the MCP tooling layer.""" + +from typing import List, Optional + + +class McpConnectionsRequiredError(Exception): + """Raised when one or more configured MCP servers are not yet connection-ready. + + The tooling gateway reports an aggregate ``connectivityStatus`` of ``"Pending"`` + when the agent's MCP servers have downstream connections that the user has not yet + established. The agent's turn handler should catch this error, reply to the user + with ``missing_connections_url``, and return without running the model/tools. + A later turn re-runs discovery and proceeds once the connections are in place. + """ + + def __init__( + self, + missing_connections_url: Optional[str], + all_connections_url: Optional[str], + connectivity_status: Optional[str], + server_names: List[str], + ) -> None: + self.missing_connections_url = missing_connections_url + self.all_connections_url = all_connections_url + self.connectivity_status = connectivity_status + self.server_names = server_names + servers_text = ", ".join(server_names) if server_names else "(unknown)" + super().__init__( + f"MCP servers [{servers_text}] require connection setup " + f"(connectivityStatus={connectivity_status}). " + f"Set up missing connections at: {missing_connections_url}" + ) +``` + +- [ ] **Step 4: Export from the package `__init__`** + +In `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py`, +add the import after the existing `from .services import ...` line: + +```python +from .exceptions import McpConnectionsRequiredError +``` + +and add `"McpConnectionsRequiredError"` to the `__all__` list (place it after +`"McpToolServerConfigurationService"`). + +- [ ] **Step 5: Run test to verify it passes** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_connections_required_error.py -v` +Expected: PASS (3 tests). + +- [ ] **Step 6: Commit** + +```bash +git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py tests/tooling/test_mcp_connections_required_error.py +git commit -m "feat(tooling): add McpConnectionsRequiredError + +Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" +``` + +--- + +## Task 3: Parse per-server connection fields in `_parse_server_config` + +**Files:** +- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` +- Test: `tests/tooling/test_mcp_server_configuration.py` + +- [ ] **Step 1: Write the failing test** + +Add to class `TestMcpToolServerConfigurationService` in +`tests/tooling/test_mcp_server_configuration.py`: + +```python + def test_parse_server_config_populates_connection_fields(self, service): + """Per-server connection fields are parsed from a V2 gateway element.""" + server_element = { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", + "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + + config = service._parse_server_config(server_element) + + assert config is not None + assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" + assert config.all_connections_url == "https://make.example/all" + assert config.missing_connections_url == "https://make.example/missing" + assert config.connectivity_status == "Pending" + + def test_parse_server_config_connection_fields_absent(self, service): + """Manifest elements without connection fields yield None.""" + server_element = { + "mcpServerName": "DevServer", + "mcpServerUniqueName": "dev_server", + "url": "https://dev.server/mcp", + } + + config = service._parse_server_config(server_element) + + assert config is not None + assert config.id is None + assert config.all_connections_url is None + assert config.missing_connections_url is None + assert config.connectivity_status is None +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMcpToolServerConfigurationService::test_parse_server_config_populates_connection_fields -v` +Expected: FAIL with `AssertionError` (e.g. `config.id` is `None`, not the GUID). + +- [ ] **Step 3: Map the new fields in `_parse_server_config`** + +In `mcp_tool_server_configuration_service.py`, inside `_parse_server_config`, locate the +`return MCPServerConfig(...)` call (around line 728). Immediately **before** that return, +add string-or-None extraction for the four fields: + +```python + id_raw = server_element.get("id") + server_id = str(id_raw) if id_raw is not None else None + + all_conn_raw = server_element.get("allConnectionsUrl") + all_connections_url = str(all_conn_raw) if all_conn_raw is not None else None + + missing_conn_raw = server_element.get("missingConnectionsUrl") + missing_connections_url = ( + str(missing_conn_raw) if missing_conn_raw is not None else None + ) + + status_raw = server_element.get("connectivityStatus") + connectivity_status = str(status_raw) if status_raw is not None else None +``` + +Then extend the `return MCPServerConfig(...)` call to pass them: + +```python + return MCPServerConfig( + mcp_server_name=mcp_server_name, + mcp_server_unique_name=mcp_server_unique_name, + url=final_url, + audience=audience, + scope=scope, + publisher=publisher, + id=server_id, + all_connections_url=all_connections_url, + missing_connections_url=missing_connections_url, + connectivity_status=connectivity_status, + ) +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMcpToolServerConfigurationService -v -k parse_server_config` +Expected: PASS (the two new tests plus the existing `_parse_server_config` tests). + +- [ ] **Step 5: Commit** + +```bash +git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py tests/tooling/test_mcp_server_configuration.py +git commit -m "feat(tooling): parse per-server MCP connection fields + +Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" +``` + +--- + +## Task 4: Capture aggregate metadata via `McpDiscoveryResult` + +This task adds the internal result wrapper and makes `_parse_gateway_response` / +`_load_servers_from_gateway` carry the response-level (aggregate) connection fields. +`list_tool_servers` is updated to unwrap `.servers` so external behavior is unchanged. + +**Files:** +- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` +- Test: `tests/tooling/test_mcp_server_configuration.py` + +- [ ] **Step 1: Write the failing test** + +Add to class `TestMcpToolServerConfigurationService`: + +```python + @pytest.mark.asyncio + async def test_parse_gateway_response_captures_aggregate(self, service): + """Response-level connection metadata is captured into McpDiscoveryResult.""" + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", + "connectivityStatus": "Pending", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=payload) + + result = await service._parse_gateway_response(mock_response) + + assert len(result.servers) == 1 + assert result.servers[0].mcp_server_name == "mcp_Salesforce" + assert result.all_connections_url == "https://make.example/all" + assert result.missing_connections_url == "https://make.example/missing" + assert result.connectivity_status == "Pending" + + @pytest.mark.asyncio + async def test_parse_gateway_response_raw_array_has_no_aggregate(self, service): + """Legacy raw-array responses produce a result with aggregate fields None.""" + payload = [ + { + "mcpServerName": "V1Server", + "mcpServerUniqueName": "v1_server", + "url": "https://v1.example.com/mcp", + } + ] + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=payload) + + result = await service._parse_gateway_response(mock_response) + + assert len(result.servers) == 1 + assert result.all_connections_url is None + assert result.missing_connections_url is None + assert result.connectivity_status is None +``` + +(`MagicMock` and `AsyncMock` are already imported at the top of the test file.) + +- [ ] **Step 2: Run test to verify it fails** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMcpToolServerConfigurationService::test_parse_gateway_response_captures_aggregate -v` +Expected: FAIL with `AttributeError: 'list' object has no attribute 'servers'` (current +return is a list). + +- [ ] **Step 3: Define `McpDiscoveryResult`** + +In `mcp_tool_server_configuration_service.py`, ensure `dataclass` is imported. The module +already has `from dataclasses import replace as dataclass_replace`; add a second import +line directly above it: + +```python +from dataclasses import dataclass +from dataclasses import replace as dataclass_replace +``` + +Then, in the `# TYPES` section (just below the `TokenAcquirer` definition near line 62), +add: + +```python +@dataclass +class McpDiscoveryResult: + """Internal result of MCP server discovery from the gateway. + + Carries the parsed server list plus the response-level (aggregate) connection + metadata used for connection gating. Sources that predate the connection fields + (legacy raw-array gateway responses, dev-mode manifests) leave the aggregate + fields as ``None``. + """ + + servers: List["MCPServerConfig"] + all_connections_url: Optional[str] = None + missing_connections_url: Optional[str] = None + connectivity_status: Optional[str] = None +``` + +- [ ] **Step 4: Update `_parse_gateway_response` to return `McpDiscoveryResult`** + +Replace the entire body of `_parse_gateway_response` (the method around lines 639–679) so +it returns a `McpDiscoveryResult`. Keep the existing wrapped/raw-array branching; add +aggregate extraction for the wrapped shape: + +```python + async def _parse_gateway_response( + self, response: aiohttp.ClientResponse + ) -> McpDiscoveryResult: + """ + Parses the response from the tooling gateway. + + Supports two response shapes: + - Wrapped: ``{"mcpServers": [...], "connectivityStatus": ..., ...}`` + - Raw array: ``[...]`` (legacy V1 gateway format, no aggregate metadata) + + Args: + response: HTTP response from the gateway. + + Returns: + McpDiscoveryResult: parsed servers plus response-level connection metadata + (aggregate fields are None for the legacy raw-array shape). + """ + config_data = await response.json(content_type=None) + + server_elements: Optional[List[object]] = None + all_connections_url: Optional[str] = None + missing_connections_url: Optional[str] = None + connectivity_status: Optional[str] = None + + if isinstance(config_data, list): + # Raw array format (legacy V1 gateway returns bare array, no aggregate). + self._logger.debug("Gateway returned raw array response") + server_elements = config_data + elif isinstance(config_data, dict) and isinstance(config_data.get("mcpServers"), list): + # Wrapped format: {"mcpServers": [...], aggregate connection fields} + self._logger.debug("Gateway returned wrapped mcpServers response") + server_elements = config_data["mcpServers"] + + all_raw = config_data.get("allConnectionsUrl") + all_connections_url = str(all_raw) if all_raw is not None else None + + missing_raw = config_data.get("missingConnectionsUrl") + missing_connections_url = str(missing_raw) if missing_raw is not None else None + + status_raw = config_data.get("connectivityStatus") + connectivity_status = str(status_raw) if status_raw is not None else None + else: + self._logger.warning( + 'Unexpected gateway response format: expected a list or {"mcpServers": [...]}' + ) + return McpDiscoveryResult(servers=[]) + + mcp_servers: List[MCPServerConfig] = [] + for server_element in server_elements: + if isinstance(server_element, dict): + server_config = self._parse_server_config(server_element) + if server_config is not None: + mcp_servers.append(server_config) + + return McpDiscoveryResult( + servers=mcp_servers, + all_connections_url=all_connections_url, + missing_connections_url=missing_connections_url, + connectivity_status=connectivity_status, + ) +``` + +- [ ] **Step 5: Update `_load_servers_from_gateway` to return `McpDiscoveryResult`** + +In `_load_servers_from_gateway` (around lines 485–537), change the return type annotation +from `List[MCPServerConfig]` to `McpDiscoveryResult`, and update the success branch. +Locate: + +```python + if response.status == 200: + mcp_servers = await self._parse_gateway_response(response) + self._logger.info( + f"Retrieved {len(mcp_servers)} MCP tool servers from tooling gateway" + ) + return mcp_servers +``` + +Replace with: + +```python + if response.status == 200: + discovery = await self._parse_gateway_response(response) + self._logger.info( + f"Retrieved {len(discovery.servers)} MCP tool servers " + f"from tooling gateway" + ) + return discovery +``` + +Also change the method signature return annotation line from: + +```python + ) -> List[MCPServerConfig]: +``` +to: +```python + ) -> McpDiscoveryResult: +``` + +- [ ] **Step 6: Update `list_tool_servers` prod branch to unwrap `.servers`** + +In `list_tool_servers`, the production branch currently is: + +```python + else: + servers = await self._load_servers_from_gateway( + agentic_app_id, auth_token, options, turn_context + ) +``` + +Replace with: + +```python + else: + discovery = await self._load_servers_from_gateway( + agentic_app_id, auth_token, options, turn_context + ) + servers = discovery.servers +``` + +(The gate is added in Task 5 — for now we just unwrap so existing behavior is preserved.) + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py -v` +Expected: PASS — the two new aggregate tests pass and all pre-existing tests still pass +(production `list_tool_servers` tests continue to work because `.servers` is unwrapped). + +- [ ] **Step 8: Commit** + +```bash +git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py tests/tooling/test_mcp_server_configuration.py +git commit -m "feat(tooling): capture aggregate connection metadata via McpDiscoveryResult + +Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" +``` + +--- + +## Task 5: Enforce the connection-readiness gate in `list_tool_servers` + +**Files:** +- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` +- Test: `tests/tooling/test_mcp_server_configuration.py` + +- [ ] **Step 1: Write the failing tests** + +Add a new test class to `tests/tooling/test_mcp_server_configuration.py`. The +`_gateway_response` helper builds the nested aiohttp mock used elsewhere in this file. + +```python +class TestConnectionGating: + """Tests for the connectivityStatus connection-readiness gate.""" + + @pytest.fixture + def service(self): + return McpToolServerConfigurationService() + + @staticmethod + def _gateway_response(payload): + """Build a patched aiohttp.ClientSession context manager returning payload.""" + mock_response = MagicMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value=payload) + mock_response_cm = MagicMock() + mock_response_cm.__aenter__ = AsyncMock(return_value=mock_response) + mock_response_cm.__aexit__ = AsyncMock(return_value=None) + mock_session = MagicMock() + mock_session.get = MagicMock(return_value=mock_response_cm) + mock_session_cm = MagicMock() + mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session) + mock_session_cm.__aexit__ = AsyncMock(return_value=None) + return patch("aiohttp.ClientSession", return_value=mock_session_cm) + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_raises_when_pending(self, service): + from microsoft_agents_a365.tooling import McpConnectionsRequiredError + + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Pending", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + with self._gateway_response(payload): + with pytest.raises(McpConnectionsRequiredError) as exc_info: + await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + err = exc_info.value + assert err.connectivity_status == "Pending" + assert err.missing_connections_url == "https://make.example/missing" + assert err.all_connections_url == "https://make.example/all" + assert "mcp_Salesforce" in err.server_names + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_passes_when_ready(self, service): + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Ready", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Ready", + } + with self._gateway_response(payload): + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "mcp_Salesforce" + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_passes_for_legacy_raw_array(self, service): + payload = [ + { + "mcpServerName": "V1Server", + "mcpServerUniqueName": "v1_server", + "url": "https://v1.example.com/mcp", + } + ] + with self._gateway_response(payload): + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "V1Server" + + @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") + @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) + @pytest.mark.asyncio + async def test_gate_not_applied_in_dev_mode(self, mock_load_manifest, service): + mock_load_manifest.return_value = [ + MCPServerConfig( + mcp_server_name="DevServer", + mcp_server_unique_name="dev_server", + url="https://dev.server/mcp", + ) + ] + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "DevServer" +``` + +(`MagicMock`, `AsyncMock`, `patch`, `os`, and `pytest` are already imported at the top of +the existing test file.) + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestConnectionGating -v` +Expected: `test_gate_raises_when_pending` FAILS (no exception raised — gate not yet +implemented); the other three may pass already. + +- [ ] **Step 3: Import the exception in the service module** + +In `mcp_tool_server_configuration_service.py`, with the other local imports (near the +`from ..models import ...` / `from ..utils import Constants` block), add: + +```python +from ..exceptions import McpConnectionsRequiredError +``` + +- [ ] **Step 4: Add the gate helper method** + +Add a private method to `McpToolServerConfigurationService` (place it just after +`list_tool_servers`, before the `# ENVIRONMENT DETECTION` section comment): + +```python + # Sentinel aggregate status that means all connections are satisfied. + _CONNECTIVITY_READY = "Ready" + + def _enforce_connection_readiness(self, discovery: "McpDiscoveryResult") -> None: + """Raise if the aggregate connectivity status indicates missing connections. + + Blocks only when the response-level ``connectivity_status`` is present and not + ``"Ready"`` (i.e. ``"Pending"``). Absent status (legacy raw-array gateway + responses, dev-mode manifests) is always treated as ready, so those paths are + never gated. The ``!= "Ready"`` form is intentionally defensive against any + unexpected future status value. + + Raises: + McpConnectionsRequiredError: when connections are not yet ready. + """ + status = discovery.connectivity_status + if status is None or status == self._CONNECTIVITY_READY: + return + + not_ready = [ + s + for s in discovery.servers + if s.connectivity_status is not None + and s.connectivity_status != self._CONNECTIVITY_READY + ] + server_names = [ + s.mcp_server_name or s.mcp_server_unique_name for s in not_ready + ] + self._logger.info( + f"MCP connection gate blocking turn: connectivityStatus={status}, " + f"servers={server_names}" + ) + raise McpConnectionsRequiredError( + missing_connections_url=discovery.missing_connections_url, + all_connections_url=discovery.all_connections_url, + connectivity_status=status, + server_names=server_names, + ) +``` + +- [ ] **Step 5: Call the gate from the prod branch of `list_tool_servers`** + +Update the production branch added in Task 4 to enforce the gate immediately after +discovery, before token branching: + +```python + else: + discovery = await self._load_servers_from_gateway( + agentic_app_id, auth_token, options, turn_context + ) + # Gate execution when configured MCP servers are not connection-ready. + # Runs before token attachment because readiness is independent of tokens. + self._enforce_connection_readiness(discovery) + servers = discovery.servers +``` + +- [ ] **Step 6: Run the gate tests to verify they pass** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestConnectionGating -v` +Expected: PASS (4 tests). + +- [ ] **Step 7: Run the full tooling test module** + +Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py -v` +Expected: PASS (all tests, including pre-existing). + +- [ ] **Step 8: Commit** + +```bash +git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py tests/tooling/test_mcp_server_configuration.py +git commit -m "feat(tooling): gate tool discovery on MCP connectivityStatus + +Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" +``` + +--- + +## Task 6: Documentation and changelog + +**Files:** +- Modify: `libraries/microsoft-agents-a365-tooling/CHANGELOG.md` +- Modify: `libraries/microsoft-agents-a365-tooling/docs/design.md` + +- [ ] **Step 1: Add CHANGELOG entries** + +In `libraries/microsoft-agents-a365-tooling/CHANGELOG.md`, under +`## [Unreleased]` → `### Added`, append these bullets: + +```markdown +- Added connection-readiness gating to `McpToolServerConfigurationService.list_tool_servers()`. When the tooling gateway reports an aggregate `connectivityStatus` of `"Pending"`, a `McpConnectionsRequiredError` is raised carrying the response-level `missingConnectionsUrl` / `allConnectionsUrl` and the names of the not-`Ready` servers, so the agent's turn handler can prompt the user to set up connections instead of running tools that would fail. Dev-mode manifests and legacy raw-array gateway responses omit the field and are never gated +- Added `McpConnectionsRequiredError` exception (exported from `microsoft_agents_a365.tooling`) with `missing_connections_url`, `all_connections_url`, `connectivity_status`, and `server_names` attributes +- Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload +- Added internal `McpDiscoveryResult` dataclass; `_parse_gateway_response()` and `_load_servers_from_gateway()` now return it to carry response-level connection metadata alongside the server list (the public return type of `list_tool_servers()` remains `List[MCPServerConfig]`) +``` + +- [ ] **Step 2: Document the catch-and-reply pattern in design.md** + +In `libraries/microsoft-agents-a365-tooling/docs/design.md`, add a new subsection (place +it after the existing `list_tool_servers` documentation, around line 227). Use this +content: + +````markdown +### Connection gating + +When the tooling gateway reports that an agent's MCP servers have unsatisfied downstream +connections, `list_tool_servers()` raises `McpConnectionsRequiredError`. Discovery runs +every turn, so the agent's turn handler should catch the error, reply with the +connection-setup link, and return — a later turn proceeds automatically once the user has +connected. + +```python +from microsoft_agents_a365.tooling import McpConnectionsRequiredError + +try: + servers = await config_service.list_tool_servers( + agentic_app_id=agentic_app_id, + auth_token=auth_token, + authorization=auth, + auth_handler_name=auth_handler_name, + turn_context=context, + ) +except McpConnectionsRequiredError as err: + await context.send_activity( + f"Before I can help, please set up the required connections for " + f"{', '.join(err.server_names)}: {err.missing_connections_url}" + ) + return # Skip running the model/tools this turn. +``` + +The gate fires only for gateway responses with aggregate `connectivityStatus == "Pending"`. +Dev-mode manifests and legacy raw-array responses omit the field and are never gated. +```` + +- [ ] **Step 3: Lint and format the whole change set** + +Run: `uv run --frozen ruff check .` +Expected: no errors (fix any reported with `uv run --frozen ruff check . --fix`). + +Run: `uv run --frozen ruff format .` +Expected: files formatted (or "X files left unchanged"). + +- [ ] **Step 4: Run the full tooling test suite once more** + +Run: `uv run --frozen pytest tests/tooling/ -v -m "not integration"` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add libraries/microsoft-agents-a365-tooling/CHANGELOG.md libraries/microsoft-agents-a365-tooling/docs/design.md +git commit -m "docs(tooling): document MCP connection gating + +Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" +``` + +--- + +## Final verification + +- [ ] Run lint: `uv run --frozen ruff check .` → no errors. +- [ ] Run format check: `uv run --frozen ruff format --check .` → clean. +- [ ] Run tooling tests: `uv run --frozen pytest tests/tooling/ -v -m "not integration"` → all pass. +- [ ] Confirm `McpConnectionsRequiredError` importable: `uv run --frozen python -c "from microsoft_agents_a365.tooling import McpConnectionsRequiredError; print('ok')"` → prints `ok`. + +## Notes on scope + +- Framework tooling extensions (`openai`, `semantickernel`, `agentframework`, + `googleadk`, `azureaifoundry`) require **no code change**: their + `add_tool_servers_to_agent` calls `list_tool_servers` and does not catch the new + exception, so it propagates to the turn handler as designed. +- No poll/wait loop is implemented — per-turn discovery (no caching) provides the retry + loop naturally. From 58f5af25c674add95f72dac78c64dd55aa8b9a84 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:59:50 -0700 Subject: [PATCH 05/15] feat(tooling): add connection fields to MCPServerConfig Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tooling/models/mcp_server_config.py | 13 ++++++++++ .../tooling/test_mcp_server_configuration.py | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py index dd57ff98..a7ed2631 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py @@ -37,6 +37,19 @@ class MCPServerConfig: #: Publisher identifier for the MCP server. publisher: Optional[str] = None + #: Unique identifier (GUID) of the MCP server from the gateway, if provided. + id: Optional[str] = None + + #: Per-server URL to view/manage all connections for this server's connector. + all_connections_url: Optional[str] = None + + #: Per-server URL to set up the connections this server is missing. + missing_connections_url: Optional[str] = None + + #: Per-server connectivity status reported by the gateway ("Ready" or "Pending"). + #: None when the source predates the field (dev manifest / legacy raw-array gateway). + connectivity_status: Optional[str] = None + def __post_init__(self): """Validate the configuration after initialization.""" if not self.mcp_server_name: diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 1c365799..7603063c 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -48,6 +48,32 @@ def test_mcp_server_config_validation(self): with pytest.raises(ValueError, match="mcp_server_unique_name cannot be empty"): MCPServerConfig(mcp_server_name="test", mcp_server_unique_name="") + def test_mcp_server_config_connection_fields_default_none(self): + """Connection fields default to None for backward compatibility.""" + config = MCPServerConfig( + mcp_server_name="TestServer", + mcp_server_unique_name="test_server", + ) + assert config.id is None + assert config.all_connections_url is None + assert config.missing_connections_url is None + assert config.connectivity_status is None + + def test_mcp_server_config_connection_fields_set(self): + """Connection fields are stored when provided.""" + config = MCPServerConfig( + mcp_server_name="TestServer", + mcp_server_unique_name="test_server", + id="3fa85f64-5717-4562-b3fc-2c963f66afa6", + all_connections_url="https://make.example/all", + missing_connections_url="https://make.example/missing", + connectivity_status="Pending", + ) + assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" + assert config.all_connections_url == "https://make.example/all" + assert config.missing_connections_url == "https://make.example/missing" + assert config.connectivity_status == "Pending" + class TestMcpToolServerConfigurationService: """Tests for McpToolServerConfigurationService.""" From 72106de72ba06e703163a817b2dea118c210bafa Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 13:03:36 -0700 Subject: [PATCH 06/15] feat(tooling): add McpConnectionsRequiredError Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../microsoft_agents_a365/tooling/__init__.py | 2 ++ .../tooling/exceptions.py | 35 +++++++++++++++++++ .../test_mcp_connections_required_error.py | 35 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py create mode 100644 tests/tooling/test_mcp_connections_required_error.py diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py index edee7a4a..6a3fc15b 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py @@ -8,6 +8,7 @@ Provides base utilities and common helper functions. """ +from .exceptions import McpConnectionsRequiredError from .models import MCPServerConfig from .services import McpToolServerConfigurationService from .utils import Constants @@ -22,6 +23,7 @@ __all__ = [ "MCPServerConfig", "McpToolServerConfigurationService", + "McpConnectionsRequiredError", "Constants", "get_tooling_gateway_for_digital_worker", "get_mcp_base_url", diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py new file mode 100644 index 00000000..2745ab88 --- /dev/null +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py @@ -0,0 +1,35 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Exceptions raised by the MCP tooling layer.""" + +from typing import List, Optional + + +class McpConnectionsRequiredError(Exception): + """Raised when one or more configured MCP servers are not yet connection-ready. + + The tooling gateway reports an aggregate ``connectivityStatus`` of ``"Pending"`` + when the agent's MCP servers have downstream connections that the user has not yet + established. The agent's turn handler should catch this error, reply to the user + with ``missing_connections_url``, and return without running the model/tools. + A later turn re-runs discovery and proceeds once the connections are in place. + """ + + def __init__( + self, + missing_connections_url: Optional[str], + all_connections_url: Optional[str], + connectivity_status: Optional[str], + server_names: List[str], + ) -> None: + self.missing_connections_url = missing_connections_url + self.all_connections_url = all_connections_url + self.connectivity_status = connectivity_status + self.server_names = server_names + servers_text = ", ".join(server_names) if server_names else "(unknown)" + super().__init__( + f"MCP servers [{servers_text}] require connection setup " + f"(connectivityStatus={connectivity_status}). " + f"Set up missing connections at: {missing_connections_url}" + ) diff --git a/tests/tooling/test_mcp_connections_required_error.py b/tests/tooling/test_mcp_connections_required_error.py new file mode 100644 index 00000000..fd8ef3ca --- /dev/null +++ b/tests/tooling/test_mcp_connections_required_error.py @@ -0,0 +1,35 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Unit tests for McpConnectionsRequiredError.""" + +from microsoft_agents_a365.tooling import McpConnectionsRequiredError + + +def test_exception_exposes_payload(): + err = McpConnectionsRequiredError( + missing_connections_url="https://make.example/missing", + all_connections_url="https://make.example/all", + connectivity_status="Pending", + server_names=["mcp_Salesforce", "mcp_Zendesk"], + ) + assert err.missing_connections_url == "https://make.example/missing" + assert err.all_connections_url == "https://make.example/all" + assert err.connectivity_status == "Pending" + assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] + + +def test_exception_message_is_actionable(): + err = McpConnectionsRequiredError( + missing_connections_url="https://make.example/missing", + all_connections_url=None, + connectivity_status="Pending", + server_names=["mcp_Salesforce"], + ) + message = str(err) + assert "mcp_Salesforce" in message + assert "https://make.example/missing" in message + + +def test_exception_is_exception_subclass(): + assert issubclass(McpConnectionsRequiredError, Exception) From 50e796568bcc6f6cb1fb9b36f86d6e0c8dba23d8 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 13:09:45 -0700 Subject: [PATCH 07/15] feat(tooling): parse per-server MCP connection fields Add parsing of four new per-server connection fields from JSON: - id: Server identifier (GUID) - allConnectionsUrl: Connection info endpoint - missingConnectionsUrl: Missing connections endpoint - connectivityStatus: Current connectivity status Fields are extracted from camelCase JSON keys and stored as optional string attributes on MCPServerConfig. When absent (manifest elements), all four fields default to None for backward compatibility. Closes Task 3: Parse per-server connection fields in _parse_server_config Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../mcp_tool_server_configuration_service.py | 18 ++++++++++ .../tooling/test_mcp_server_configuration.py | 36 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py index 5ee987f9..055e8529 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py @@ -725,6 +725,20 @@ def _parse_server_config(self, server_element: Dict[str, object]) -> Optional[MC publisher_raw = server_element.get("publisher") publisher = str(publisher_raw) if publisher_raw is not None else None + id_raw = server_element.get("id") + server_id = str(id_raw) if id_raw is not None else None + + all_conn_raw = server_element.get("allConnectionsUrl") + all_connections_url = str(all_conn_raw) if all_conn_raw is not None else None + + missing_conn_raw = server_element.get("missingConnectionsUrl") + missing_connections_url = ( + str(missing_conn_raw) if missing_conn_raw is not None else None + ) + + status_raw = server_element.get("connectivityStatus") + connectivity_status = str(status_raw) if status_raw is not None else None + return MCPServerConfig( mcp_server_name=mcp_server_name, mcp_server_unique_name=mcp_server_unique_name, @@ -732,6 +746,10 @@ def _parse_server_config(self, server_element: Dict[str, object]) -> Optional[MC audience=audience, scope=scope, publisher=publisher, + id=server_id, + all_connections_url=all_connections_url, + missing_connections_url=missing_connections_url, + connectivity_status=connectivity_status, ) except Exception as exc: diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 7603063c..1cbcbf21 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -186,6 +186,42 @@ def test_parse_gateway_server_config_without_custom_url(self, mock_build_url, se assert config.url == "https://default.server/agents/servers/GatewayServer" mock_build_url.assert_called_once_with("GatewayServer") + def test_parse_server_config_populates_connection_fields(self, service): + """Per-server connection fields are parsed from a V2 gateway element.""" + server_element = { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", + "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + + config = service._parse_server_config(server_element) + + assert config is not None + assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" + assert config.all_connections_url == "https://make.example/all" + assert config.missing_connections_url == "https://make.example/missing" + assert config.connectivity_status == "Pending" + + def test_parse_server_config_connection_fields_absent(self, service): + """Manifest elements without connection fields yield None.""" + server_element = { + "mcpServerName": "DevServer", + "mcpServerUniqueName": "dev_server", + "url": "https://dev.server/mcp", + } + + config = service._parse_server_config(server_element) + + assert config is not None + assert config.id is None + assert config.all_connections_url is None + assert config.missing_connections_url is None + assert config.connectivity_status is None + @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) def test_is_development_scenario(self, service): """Test development scenario detection.""" From 431cec6909831c2cefed766b4811aabdae8abf9e Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 13:20:32 -0700 Subject: [PATCH 08/15] feat(tooling): capture aggregate connection metadata via McpDiscoveryResult Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../mcp_tool_server_configuration_service.py | 68 ++++++++++++++----- .../tooling/test_mcp_server_configuration.py | 47 +++++++++++++ 2 files changed, 99 insertions(+), 16 deletions(-) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py index 055e8529..a93a265e 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py @@ -23,6 +23,7 @@ import logging import os import uuid +from dataclasses import dataclass from dataclasses import replace as dataclass_replace from pathlib import Path from typing import Awaitable, Callable, Dict, List, Optional @@ -62,6 +63,22 @@ TokenAcquirer = Callable[["MCPServerConfig", str], Awaitable[Optional[str]]] +@dataclass +class McpDiscoveryResult: + """Internal result of MCP server discovery from the gateway. + + Carries the parsed server list plus the response-level (aggregate) connection + metadata used for connection gating. Sources that predate the connection fields + (legacy raw-array gateway responses, dev-mode manifests) leave the aggregate + fields as ``None``. + """ + + servers: List["MCPServerConfig"] + all_connections_url: Optional[str] = None + missing_connections_url: Optional[str] = None + connectivity_status: Optional[str] = None + + # ============================================================================== # CONSTANTS # ============================================================================== @@ -155,9 +172,10 @@ async def list_tool_servers( # BEARER_TOKEN_ takes precedence; BEARER_TOKEN is the fallback. acquire: TokenAcquirer = self._create_dev_token_acquirer() else: - servers = await self._load_servers_from_gateway( + discovery = await self._load_servers_from_gateway( agentic_app_id, auth_token, options, turn_context ) + servers = discovery.servers if ( authorization is not None and auth_handler_name is not None @@ -488,7 +506,7 @@ async def _load_servers_from_gateway( auth_token: str, options: ToolOptions, turn_context: Optional[TurnContext] = None, - ) -> List[MCPServerConfig]: + ) -> McpDiscoveryResult: """ Reads MCP server configurations from tooling gateway endpoint for production scenario. @@ -500,7 +518,7 @@ async def _load_servers_from_gateway( ``activity.id``. A new UUID is generated when not provided. Returns: - List[MCPServerConfig]: List of MCP server configurations from tooling gateway. + McpDiscoveryResult: Discovery result with server list and aggregate connection metadata. Raises: Exception: If there's an error communicating with the tooling gateway. @@ -515,11 +533,12 @@ async def _load_servers_from_gateway( async with aiohttp.ClientSession(timeout=timeout) as session: async with session.get(config_endpoint, headers=headers) as response: if response.status == 200: - mcp_servers = await self._parse_gateway_response(response) + discovery = await self._parse_gateway_response(response) self._logger.info( - f"Retrieved {len(mcp_servers)} MCP tool servers from tooling gateway" + f"Retrieved {len(discovery.servers)} MCP tool servers " + f"from tooling gateway" ) - return mcp_servers + return discovery else: raise Exception(f"HTTP {response.status}: {await response.text()}") @@ -636,38 +655,50 @@ def _resolve_agent_id_for_header( # Priority 4: Application name from AGENT365_APPLICATION_NAME env or pyproject.toml return RuntimeUtility.get_application_name() - async def _parse_gateway_response( - self, response: aiohttp.ClientResponse - ) -> List[MCPServerConfig]: + async def _parse_gateway_response(self, response: aiohttp.ClientResponse) -> McpDiscoveryResult: """ Parses the response from the tooling gateway. Supports two response shapes: - - Wrapped: ``{"mcpServers": [...]}`` - - Raw array: ``[...]`` (legacy V1 gateway format) + - Wrapped: ``{"mcpServers": [...], "connectivityStatus": ..., ...}`` + - Raw array: ``[...]`` (legacy V1 gateway format, no aggregate metadata) Args: response: HTTP response from the gateway. Returns: - List of parsed MCP server configurations. + McpDiscoveryResult: parsed servers plus response-level connection metadata + (aggregate fields are None for the legacy raw-array shape). """ config_data = await response.json(content_type=None) server_elements: Optional[List[object]] = None + all_connections_url: Optional[str] = None + missing_connections_url: Optional[str] = None + connectivity_status: Optional[str] = None + if isinstance(config_data, list): - # Raw array format (legacy V1 gateway returns bare array) + # Raw array format (legacy V1 gateway returns bare array, no aggregate). self._logger.debug("Gateway returned raw array response") server_elements = config_data elif isinstance(config_data, dict) and isinstance(config_data.get("mcpServers"), list): - # Wrapped format: {"mcpServers": [...]} + # Wrapped format: {"mcpServers": [...], aggregate connection fields} self._logger.debug("Gateway returned wrapped mcpServers response") server_elements = config_data["mcpServers"] + + all_raw = config_data.get("allConnectionsUrl") + all_connections_url = str(all_raw) if all_raw is not None else None + + missing_raw = config_data.get("missingConnectionsUrl") + missing_connections_url = str(missing_raw) if missing_raw is not None else None + + status_raw = config_data.get("connectivityStatus") + connectivity_status = str(status_raw) if status_raw is not None else None else: self._logger.warning( 'Unexpected gateway response format: expected a list or {"mcpServers": [...]}' ) - return [] + return McpDiscoveryResult(servers=[]) mcp_servers: List[MCPServerConfig] = [] for server_element in server_elements: @@ -676,7 +707,12 @@ async def _parse_gateway_response( if server_config is not None: mcp_servers.append(server_config) - return mcp_servers + return McpDiscoveryResult( + servers=mcp_servers, + all_connections_url=all_connections_url, + missing_connections_url=missing_connections_url, + connectivity_status=connectivity_status, + ) # -------------------------------------------------------------------------- # CONFIGURATION PARSING HELPERS diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 1cbcbf21..ea9965c2 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -384,6 +384,53 @@ async def test_legacy_prod_path_ok_for_v1_only_servers(self, service): assert len(servers) == 1 assert servers[0].mcp_server_name == "V1Server" + @pytest.mark.asyncio + async def test_parse_gateway_response_captures_aggregate(self, service): + """Response-level connection metadata is captured into McpDiscoveryResult.""" + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", + "connectivityStatus": "Pending", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=payload) + + result = await service._parse_gateway_response(mock_response) + + assert len(result.servers) == 1 + assert result.servers[0].mcp_server_name == "mcp_Salesforce" + assert result.all_connections_url == "https://make.example/all" + assert result.missing_connections_url == "https://make.example/missing" + assert result.connectivity_status == "Pending" + + @pytest.mark.asyncio + async def test_parse_gateway_response_raw_array_has_no_aggregate(self, service): + """Legacy raw-array responses produce a result with aggregate fields None.""" + payload = [ + { + "mcpServerName": "V1Server", + "mcpServerUniqueName": "v1_server", + "url": "https://v1.example.com/mcp", + } + ] + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=payload) + + result = await service._parse_gateway_response(mock_response) + + assert len(result.servers) == 1 + assert result.all_connections_url is None + assert result.missing_connections_url is None + assert result.connectivity_status is None + class TestResolveTokenScopeForServer: """Tests for resolve_token_scope_for_server() utility function.""" From 27292b1c10fdc2ea599e2020b357b70553ea07de Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 13:31:13 -0700 Subject: [PATCH 09/15] feat(tooling): gate tool discovery on MCP connectivityStatus Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../mcp_tool_server_configuration_service.py | 41 +++++++ .../tooling/test_mcp_server_configuration.py | 109 ++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py index a93a265e..04131916 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py @@ -34,6 +34,7 @@ from microsoft_agents.hosting.core import Authorization, TurnContext # Local imports +from ..exceptions import McpConnectionsRequiredError from ..models import ChatHistoryMessage, ChatMessageRequest, MCPServerConfig, ToolOptions from ..utils import Constants from ..utils.utility import ( @@ -175,6 +176,9 @@ async def list_tool_servers( discovery = await self._load_servers_from_gateway( agentic_app_id, auth_token, options, turn_context ) + # Gate execution when configured MCP servers are not connection-ready. + # Runs before token attachment because readiness is independent of tokens. + self._enforce_connection_readiness(discovery) servers = discovery.servers if ( authorization is not None @@ -206,6 +210,43 @@ async def list_tool_servers( servers = await self._attach_per_audience_tokens(servers, acquire) return servers + # Sentinel aggregate status that means all connections are satisfied. + _CONNECTIVITY_READY = "Ready" + + def _enforce_connection_readiness(self, discovery: "McpDiscoveryResult") -> None: + """Raise if the aggregate connectivity status indicates missing connections. + + Blocks only when the response-level ``connectivity_status`` is present and not + ``"Ready"`` (i.e. ``"Pending"``). Absent status (legacy raw-array gateway + responses, dev-mode manifests) is always treated as ready, so those paths are + never gated. The ``!= "Ready"`` form is intentionally defensive against any + unexpected future status value. + + Raises: + McpConnectionsRequiredError: when connections are not yet ready. + """ + status = discovery.connectivity_status + if status is None or status == self._CONNECTIVITY_READY: + return + + not_ready = [ + s + for s in discovery.servers + if s.connectivity_status is not None + and s.connectivity_status != self._CONNECTIVITY_READY + ] + server_names = [s.mcp_server_name or s.mcp_server_unique_name for s in not_ready] + self._logger.info( + f"MCP connection gate blocking turn: connectivityStatus={status}, " + f"servers={server_names}" + ) + raise McpConnectionsRequiredError( + missing_connections_url=discovery.missing_connections_url, + all_connections_url=discovery.all_connections_url, + connectivity_status=status, + server_names=server_names, + ) + # -------------------------------------------------------------------------- # ENVIRONMENT DETECTION # -------------------------------------------------------------------------- diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index ea9965c2..c384a1f1 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -8,6 +8,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from microsoft_agents_a365.tooling import McpConnectionsRequiredError from microsoft_agents_a365.tooling.models import MCPServerConfig from microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service import ( McpToolServerConfigurationService, @@ -987,3 +988,111 @@ def test_skips_empty_blueprint_id(self, service, create_test_jwt): result = service._resolve_agent_id_for_header(token, mock_context) assert result == "token-appid" + + +class TestConnectionGating: + """Tests for the connectivityStatus connection-readiness gate.""" + + @pytest.fixture + def service(self): + return McpToolServerConfigurationService() + + @staticmethod + def _gateway_response(payload): + """Build a patched aiohttp.ClientSession context manager returning payload.""" + mock_response = MagicMock() + mock_response.status = 200 + mock_response.json = AsyncMock(return_value=payload) + mock_response_cm = MagicMock() + mock_response_cm.__aenter__ = AsyncMock(return_value=mock_response) + mock_response_cm.__aexit__ = AsyncMock(return_value=None) + mock_session = MagicMock() + mock_session.get = MagicMock(return_value=mock_response_cm) + mock_session_cm = MagicMock() + mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session) + mock_session_cm.__aexit__ = AsyncMock(return_value=None) + return patch("aiohttp.ClientSession", return_value=mock_session_cm) + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_raises_when_pending(self, service): + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Pending", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + with self._gateway_response(payload): + with pytest.raises(McpConnectionsRequiredError) as exc_info: + await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + err = exc_info.value + assert err.connectivity_status == "Pending" + assert err.missing_connections_url == "https://make.example/missing" + assert err.all_connections_url == "https://make.example/all" + assert "mcp_Salesforce" in err.server_names + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_passes_when_ready(self, service): + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Ready", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Ready", + } + with self._gateway_response(payload): + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "mcp_Salesforce" + + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_passes_for_legacy_raw_array(self, service): + payload = [ + { + "mcpServerName": "V1Server", + "mcpServerUniqueName": "v1_server", + "url": "https://v1.example.com/mcp", + } + ] + with self._gateway_response(payload): + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "V1Server" + + @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") + @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) + @pytest.mark.asyncio + async def test_gate_not_applied_in_dev_mode(self, mock_load_manifest, service): + mock_load_manifest.return_value = [ + MCPServerConfig( + mcp_server_name="DevServer", + mcp_server_unique_name="dev_server", + url="https://dev.server/mcp", + ) + ] + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "DevServer" From be076098f583b88e729019eaed8095ab188d66f8 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:30:17 -0700 Subject: [PATCH 10/15] docs(tooling): document MCP connection gating Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CHANGELOG.md | 4 +++ .../docs/design.md | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md index 73e4267c..dfed482d 100644 --- a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md +++ b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md @@ -35,3 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `ChatHistoryMessage` Pydantic model for representing individual messages in chat history - Added `ChatMessageRequest` Pydantic model for the chat history API request payload - Added `py.typed` marker for PEP 561 compliance, enabling type checker support +- Added connection-readiness gating to `McpToolServerConfigurationService.list_tool_servers()`. When the tooling gateway reports an aggregate `connectivityStatus` of `"Pending"`, a `McpConnectionsRequiredError` is raised carrying the response-level `missingConnectionsUrl` / `allConnectionsUrl` and the names of the not-`Ready` servers, so the agent's turn handler can prompt the user to set up connections instead of running tools that would fail. Dev-mode manifests and legacy raw-array gateway responses omit the field and are never gated +- Added `McpConnectionsRequiredError` exception (exported from `microsoft_agents_a365.tooling`) with `missing_connections_url`, `all_connections_url`, `connectivity_status`, and `server_names` attributes +- Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload +- Added internal `McpDiscoveryResult` dataclass; `_parse_gateway_response()` and `_load_servers_from_gateway()` now return it to carry response-level connection metadata alongside the server list (the public return type of `list_tool_servers()` remains `List[MCPServerConfig]`) diff --git a/libraries/microsoft-agents-a365-tooling/docs/design.md b/libraries/microsoft-agents-a365-tooling/docs/design.md index bf83f8b1..c0a6def6 100644 --- a/libraries/microsoft-agents-a365-tooling/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling/docs/design.md @@ -108,6 +108,36 @@ User-Agent: Agent365SDK/0.1.0 (...) The gateway returns the same JSON structure, but `mcpServerUniqueName` contains the full endpoint URL. +### Connection gating + +When the tooling gateway reports that an agent's MCP servers have unsatisfied downstream +connections, `list_tool_servers()` raises `McpConnectionsRequiredError`. Discovery runs +every turn, so the agent's turn handler should catch the error, reply with the +connection-setup link, and return — a later turn proceeds automatically once the user has +connected. + +```python +from microsoft_agents_a365.tooling import McpConnectionsRequiredError + +try: + servers = await config_service.list_tool_servers( + agentic_app_id=agentic_app_id, + auth_token=auth_token, + authorization=auth, + auth_handler_name=auth_handler_name, + turn_context=context, + ) +except McpConnectionsRequiredError as err: + await context.send_activity( + f"Before I can help, please set up the required connections for " + f"{', '.join(err.server_names)}: {err.missing_connections_url}" + ) + return # Skip running the model/tools this turn. +``` + +The gate fires only for gateway responses with aggregate `connectivityStatus == "Pending"`. +Dev-mode manifests and legacy raw-array responses omit the field and are never gated. + ### MCPServerConfig ([models/mcp_server_config.py](../microsoft_agents_a365/tooling/models/mcp_server_config.py)) Data class representing an MCP server configuration: From 57ec3f8506075861676ea03e481e355b4cd43919 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:36:40 -0700 Subject: [PATCH 11/15] test(tooling): cover aggregate-Pending gate with no per-server flag Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tooling/test_mcp_server_configuration.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index c384a1f1..077777c4 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -1080,6 +1080,35 @@ async def test_gate_passes_for_legacy_raw_array(self, service): assert len(servers) == 1 assert servers[0].mcp_server_name == "V1Server" + @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) + @pytest.mark.asyncio + async def test_gate_raises_when_aggregate_pending_but_no_server_flagged(self, service): + from microsoft_agents_a365.tooling import McpConnectionsRequiredError + + payload = { + "mcpServers": [ + { + "mcpServerName": "mcp_Salesforce", + "mcpServerUniqueName": "mcp_Salesforce", + "url": "https://gw.example/mcp_Salesforce", + "connectivityStatus": "Ready", + } + ], + "allConnectionsUrl": "https://make.example/all", + "missingConnectionsUrl": "https://make.example/missing", + "connectivityStatus": "Pending", + } + with self._gateway_response(payload): + with pytest.raises(McpConnectionsRequiredError) as exc_info: + await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + err = exc_info.value + assert err.connectivity_status == "Pending" + assert err.missing_connections_url == "https://make.example/missing" + assert err.server_names == [] + assert "(unknown)" in str(err) + @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) @pytest.mark.asyncio From 70cf98d650367bd92dd00524d7d12c896811d95c Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:43:42 -0700 Subject: [PATCH 12/15] refactor(tooling): drop all_connections_url from McpConnectionsRequiredError The user-facing connection error now carries only missing_connections_url, connectivity_status, and server_names. The aggregate allConnectionsUrl is no longer surfaced on the exception (still parsed onto MCPServerConfig). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- libraries/microsoft-agents-a365-tooling/CHANGELOG.md | 4 ++-- .../microsoft_agents_a365/tooling/exceptions.py | 2 -- .../tooling/services/mcp_tool_server_configuration_service.py | 1 - tests/tooling/test_mcp_connections_required_error.py | 3 --- tests/tooling/test_mcp_server_configuration.py | 1 - 5 files changed, 2 insertions(+), 9 deletions(-) diff --git a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md index dfed482d..8aa1fd93 100644 --- a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md +++ b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md @@ -35,7 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `ChatHistoryMessage` Pydantic model for representing individual messages in chat history - Added `ChatMessageRequest` Pydantic model for the chat history API request payload - Added `py.typed` marker for PEP 561 compliance, enabling type checker support -- Added connection-readiness gating to `McpToolServerConfigurationService.list_tool_servers()`. When the tooling gateway reports an aggregate `connectivityStatus` of `"Pending"`, a `McpConnectionsRequiredError` is raised carrying the response-level `missingConnectionsUrl` / `allConnectionsUrl` and the names of the not-`Ready` servers, so the agent's turn handler can prompt the user to set up connections instead of running tools that would fail. Dev-mode manifests and legacy raw-array gateway responses omit the field and are never gated -- Added `McpConnectionsRequiredError` exception (exported from `microsoft_agents_a365.tooling`) with `missing_connections_url`, `all_connections_url`, `connectivity_status`, and `server_names` attributes +- Added connection-readiness gating to `McpToolServerConfigurationService.list_tool_servers()`. When the tooling gateway reports an aggregate `connectivityStatus` of `"Pending"`, a `McpConnectionsRequiredError` is raised carrying the response-level `missingConnectionsUrl` and the names of the not-`Ready` servers, so the agent's turn handler can prompt the user to set up connections instead of running tools that would fail. Dev-mode manifests and legacy raw-array gateway responses omit the field and are never gated +- Added `McpConnectionsRequiredError` exception (exported from `microsoft_agents_a365.tooling`) with `missing_connections_url`, `connectivity_status`, and `server_names` attributes - Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload - Added internal `McpDiscoveryResult` dataclass; `_parse_gateway_response()` and `_load_servers_from_gateway()` now return it to carry response-level connection metadata alongside the server list (the public return type of `list_tool_servers()` remains `List[MCPServerConfig]`) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py index 2745ab88..25fa8e14 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py @@ -19,12 +19,10 @@ class McpConnectionsRequiredError(Exception): def __init__( self, missing_connections_url: Optional[str], - all_connections_url: Optional[str], connectivity_status: Optional[str], server_names: List[str], ) -> None: self.missing_connections_url = missing_connections_url - self.all_connections_url = all_connections_url self.connectivity_status = connectivity_status self.server_names = server_names servers_text = ", ".join(server_names) if server_names else "(unknown)" diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py index 04131916..f67eb53f 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py @@ -242,7 +242,6 @@ def _enforce_connection_readiness(self, discovery: "McpDiscoveryResult") -> None ) raise McpConnectionsRequiredError( missing_connections_url=discovery.missing_connections_url, - all_connections_url=discovery.all_connections_url, connectivity_status=status, server_names=server_names, ) diff --git a/tests/tooling/test_mcp_connections_required_error.py b/tests/tooling/test_mcp_connections_required_error.py index fd8ef3ca..06e6c6ac 100644 --- a/tests/tooling/test_mcp_connections_required_error.py +++ b/tests/tooling/test_mcp_connections_required_error.py @@ -9,12 +9,10 @@ def test_exception_exposes_payload(): err = McpConnectionsRequiredError( missing_connections_url="https://make.example/missing", - all_connections_url="https://make.example/all", connectivity_status="Pending", server_names=["mcp_Salesforce", "mcp_Zendesk"], ) assert err.missing_connections_url == "https://make.example/missing" - assert err.all_connections_url == "https://make.example/all" assert err.connectivity_status == "Pending" assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] @@ -22,7 +20,6 @@ def test_exception_exposes_payload(): def test_exception_message_is_actionable(): err = McpConnectionsRequiredError( missing_connections_url="https://make.example/missing", - all_connections_url=None, connectivity_status="Pending", server_names=["mcp_Salesforce"], ) diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 077777c4..0386bc1a 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -1037,7 +1037,6 @@ async def test_gate_raises_when_pending(self, service): err = exc_info.value assert err.connectivity_status == "Pending" assert err.missing_connections_url == "https://make.example/missing" - assert err.all_connections_url == "https://make.example/all" assert "mcp_Salesforce" in err.server_names @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) From 22b3eab66ee7e7659e5bb4429973dd74d5f99e99 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:46:26 -0700 Subject: [PATCH 13/15] chore: remove superpowers spec and plan docs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../plans/2026-06-08-mcp-connection-gating.md | 943 ------------------ ...2026-06-08-mcp-connection-gating-design.md | 222 ----- 2 files changed, 1165 deletions(-) delete mode 100644 docs/superpowers/plans/2026-06-08-mcp-connection-gating.md delete mode 100644 docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md diff --git a/docs/superpowers/plans/2026-06-08-mcp-connection-gating.md b/docs/superpowers/plans/2026-06-08-mcp-connection-gating.md deleted file mode 100644 index fddda577..00000000 --- a/docs/superpowers/plans/2026-06-08-mcp-connection-gating.md +++ /dev/null @@ -1,943 +0,0 @@ -# MCP Connection-Gated Tool Discovery Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Block agent execution when a configured MCP server reports `connectivityStatus: "Pending"`, raising a typed error carrying the connection-setup URLs so the agent's turn handler can prompt the user instead of running tools that would fail. - -**Architecture:** Extend `MCPServerConfig` with the new per-server connection fields; capture the response-level (aggregate) connection metadata into a new internal `McpDiscoveryResult`; in core `McpToolServerConfigurationService.list_tool_servers`, raise `McpConnectionsRequiredError` when the aggregate status is present and not `"Ready"`. Dev-mode manifests and legacy raw-array responses omit the field → never gated. Framework extensions let the exception propagate to the turn handler. - -**Tech Stack:** Python 3.11+, `uv`, `pytest`, `ruff`. Package: `microsoft-agents-a365-tooling`. - -**Spec:** `docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md` - ---- - -## Background / orientation (read once) - -Key file: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` - -Relevant existing flow inside `list_tool_servers`: -- Dev: `servers = self._load_servers_from_manifest()` (returns `List[MCPServerConfig]`). -- Prod: `discovery = await self._load_servers_from_gateway(...)` then either OBO token - acquirer or a legacy V2-guard early-return. -- Both end with `_attach_per_audience_tokens(...)` then `return servers`. - -Parsing helpers: -- `_parse_server_config(server_element)` — maps one JSON object (gateway **or** manifest) - to `MCPServerConfig`. Lines ~685–741. -- `_parse_gateway_response(response)` — currently returns `List[MCPServerConfig]`; - handles wrapped `{"mcpServers": [...]}` and legacy raw-array shapes. Lines ~639–679. - -**Gate placement decision (refinement over spec wording):** enforce the gate in -`list_tool_servers` immediately after the gateway discovery call returns, *before* token -attachment and before the auth-context branching. Connection readiness is independent of -token exchange, so gating first also avoids unnecessary OBO exchanges when connections -aren't ready. The dev/manifest path is never gated (no aggregate field). - -**Conventions to follow:** -- Copyright header on every `.py` file (ruff `CPY` rule): - ```python - # Copyright (c) Microsoft Corporation. - # Licensed under the MIT License. - ``` -- Type hints on all params/returns. Never use `typing.Any`. Use `is not None` for None checks. -- Run tests: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py -v` -- Lint/format: `uv run --frozen ruff check .` and `uv run --frozen ruff format .` - ---- - -## File Structure - -- **Modify** `.../tooling/models/mcp_server_config.py` — add 4 optional fields. -- **Create** `.../tooling/exceptions.py` — `McpConnectionsRequiredError`. -- **Modify** `.../tooling/__init__.py` — export the exception. -- **Modify** `.../tooling/services/mcp_tool_server_configuration_service.py` — add - `McpDiscoveryResult`, per-server field parsing, aggregate parsing, gate enforcement. -- **Modify** `tests/tooling/test_mcp_server_configuration.py` — new tests. -- **Create** `tests/tooling/test_mcp_connections_required_error.py` — exception tests. -- **Modify** `.../tooling/CHANGELOG.md` — changelog entry. -- **Modify** `.../tooling/docs/design.md` — document the catch-and-reply pattern. - -Path prefix for all `.../tooling/...` entries: -`libraries/microsoft-agents-a365-tooling/microsoft_agents_a365` - ---- - -## Task 1: Add connection fields to `MCPServerConfig` - -**Files:** -- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py` -- Test: `tests/tooling/test_mcp_server_configuration.py` - -- [ ] **Step 1: Write the failing test** - -Add to class `TestMCPServerConfig` in `tests/tooling/test_mcp_server_configuration.py`: - -```python - def test_mcp_server_config_connection_fields_default_none(self): - """Connection fields default to None for backward compatibility.""" - config = MCPServerConfig( - mcp_server_name="TestServer", - mcp_server_unique_name="test_server", - ) - assert config.id is None - assert config.all_connections_url is None - assert config.missing_connections_url is None - assert config.connectivity_status is None - - def test_mcp_server_config_connection_fields_set(self): - """Connection fields are stored when provided.""" - config = MCPServerConfig( - mcp_server_name="TestServer", - mcp_server_unique_name="test_server", - id="3fa85f64-5717-4562-b3fc-2c963f66afa6", - all_connections_url="https://make.example/all", - missing_connections_url="https://make.example/missing", - connectivity_status="Pending", - ) - assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" - assert config.all_connections_url == "https://make.example/all" - assert config.missing_connections_url == "https://make.example/missing" - assert config.connectivity_status == "Pending" -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMCPServerConfig::test_mcp_server_config_connection_fields_set -v` -Expected: FAIL with `TypeError: __init__() got an unexpected keyword argument 'id'`. - -- [ ] **Step 3: Add the fields to the dataclass** - -In `mcp_server_config.py`, add these fields inside the `MCPServerConfig` dataclass, -after the existing `publisher` field (keep all new fields optional with `None` defaults): - -```python - #: Unique identifier (GUID) of the MCP server from the gateway, if provided. - id: Optional[str] = None - - #: Per-server URL to view/manage all connections for this server's connector. - all_connections_url: Optional[str] = None - - #: Per-server URL to set up the connections this server is missing. - missing_connections_url: Optional[str] = None - - #: Per-server connectivity status reported by the gateway ("Ready" or "Pending"). - #: None when the source predates the field (dev manifest / legacy raw-array gateway). - connectivity_status: Optional[str] = None -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMCPServerConfig -v` -Expected: PASS (all tests in the class). - -- [ ] **Step 5: Commit** - -```bash -git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/models/mcp_server_config.py tests/tooling/test_mcp_server_configuration.py -git commit -m "feat(tooling): add connection fields to MCPServerConfig - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" -``` - ---- - -## Task 2: Add `McpConnectionsRequiredError` exception - -**Files:** -- Create: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py` -- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py` -- Test: `tests/tooling/test_mcp_connections_required_error.py` - -- [ ] **Step 1: Write the failing test** - -Create `tests/tooling/test_mcp_connections_required_error.py`: - -```python -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -"""Unit tests for McpConnectionsRequiredError.""" - -from microsoft_agents_a365.tooling import McpConnectionsRequiredError - - -def test_exception_exposes_payload(): - err = McpConnectionsRequiredError( - missing_connections_url="https://make.example/missing", - all_connections_url="https://make.example/all", - connectivity_status="Pending", - server_names=["mcp_Salesforce", "mcp_Zendesk"], - ) - assert err.missing_connections_url == "https://make.example/missing" - assert err.all_connections_url == "https://make.example/all" - assert err.connectivity_status == "Pending" - assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] - - -def test_exception_message_is_actionable(): - err = McpConnectionsRequiredError( - missing_connections_url="https://make.example/missing", - all_connections_url=None, - connectivity_status="Pending", - server_names=["mcp_Salesforce"], - ) - message = str(err) - assert "mcp_Salesforce" in message - assert "https://make.example/missing" in message - - -def test_exception_is_exception_subclass(): - assert issubclass(McpConnectionsRequiredError, Exception) -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_connections_required_error.py -v` -Expected: FAIL with `ImportError: cannot import name 'McpConnectionsRequiredError'`. - -- [ ] **Step 3: Create the exception module** - -Create `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py`: - -```python -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -"""Exceptions raised by the MCP tooling layer.""" - -from typing import List, Optional - - -class McpConnectionsRequiredError(Exception): - """Raised when one or more configured MCP servers are not yet connection-ready. - - The tooling gateway reports an aggregate ``connectivityStatus`` of ``"Pending"`` - when the agent's MCP servers have downstream connections that the user has not yet - established. The agent's turn handler should catch this error, reply to the user - with ``missing_connections_url``, and return without running the model/tools. - A later turn re-runs discovery and proceeds once the connections are in place. - """ - - def __init__( - self, - missing_connections_url: Optional[str], - all_connections_url: Optional[str], - connectivity_status: Optional[str], - server_names: List[str], - ) -> None: - self.missing_connections_url = missing_connections_url - self.all_connections_url = all_connections_url - self.connectivity_status = connectivity_status - self.server_names = server_names - servers_text = ", ".join(server_names) if server_names else "(unknown)" - super().__init__( - f"MCP servers [{servers_text}] require connection setup " - f"(connectivityStatus={connectivity_status}). " - f"Set up missing connections at: {missing_connections_url}" - ) -``` - -- [ ] **Step 4: Export from the package `__init__`** - -In `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py`, -add the import after the existing `from .services import ...` line: - -```python -from .exceptions import McpConnectionsRequiredError -``` - -and add `"McpConnectionsRequiredError"` to the `__all__` list (place it after -`"McpToolServerConfigurationService"`). - -- [ ] **Step 5: Run test to verify it passes** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_connections_required_error.py -v` -Expected: PASS (3 tests). - -- [ ] **Step 6: Commit** - -```bash -git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py tests/tooling/test_mcp_connections_required_error.py -git commit -m "feat(tooling): add McpConnectionsRequiredError - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" -``` - ---- - -## Task 3: Parse per-server connection fields in `_parse_server_config` - -**Files:** -- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` -- Test: `tests/tooling/test_mcp_server_configuration.py` - -- [ ] **Step 1: Write the failing test** - -Add to class `TestMcpToolServerConfigurationService` in -`tests/tooling/test_mcp_server_configuration.py`: - -```python - def test_parse_server_config_populates_connection_fields(self, service): - """Per-server connection fields are parsed from a V2 gateway element.""" - server_element = { - "mcpServerName": "mcp_Salesforce", - "mcpServerUniqueName": "mcp_Salesforce", - "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", - "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", - "allConnectionsUrl": "https://make.example/all", - "missingConnectionsUrl": "https://make.example/missing", - "connectivityStatus": "Pending", - } - - config = service._parse_server_config(server_element) - - assert config is not None - assert config.id == "3fa85f64-5717-4562-b3fc-2c963f66afa6" - assert config.all_connections_url == "https://make.example/all" - assert config.missing_connections_url == "https://make.example/missing" - assert config.connectivity_status == "Pending" - - def test_parse_server_config_connection_fields_absent(self, service): - """Manifest elements without connection fields yield None.""" - server_element = { - "mcpServerName": "DevServer", - "mcpServerUniqueName": "dev_server", - "url": "https://dev.server/mcp", - } - - config = service._parse_server_config(server_element) - - assert config is not None - assert config.id is None - assert config.all_connections_url is None - assert config.missing_connections_url is None - assert config.connectivity_status is None -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMcpToolServerConfigurationService::test_parse_server_config_populates_connection_fields -v` -Expected: FAIL with `AssertionError` (e.g. `config.id` is `None`, not the GUID). - -- [ ] **Step 3: Map the new fields in `_parse_server_config`** - -In `mcp_tool_server_configuration_service.py`, inside `_parse_server_config`, locate the -`return MCPServerConfig(...)` call (around line 728). Immediately **before** that return, -add string-or-None extraction for the four fields: - -```python - id_raw = server_element.get("id") - server_id = str(id_raw) if id_raw is not None else None - - all_conn_raw = server_element.get("allConnectionsUrl") - all_connections_url = str(all_conn_raw) if all_conn_raw is not None else None - - missing_conn_raw = server_element.get("missingConnectionsUrl") - missing_connections_url = ( - str(missing_conn_raw) if missing_conn_raw is not None else None - ) - - status_raw = server_element.get("connectivityStatus") - connectivity_status = str(status_raw) if status_raw is not None else None -``` - -Then extend the `return MCPServerConfig(...)` call to pass them: - -```python - return MCPServerConfig( - mcp_server_name=mcp_server_name, - mcp_server_unique_name=mcp_server_unique_name, - url=final_url, - audience=audience, - scope=scope, - publisher=publisher, - id=server_id, - all_connections_url=all_connections_url, - missing_connections_url=missing_connections_url, - connectivity_status=connectivity_status, - ) -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMcpToolServerConfigurationService -v -k parse_server_config` -Expected: PASS (the two new tests plus the existing `_parse_server_config` tests). - -- [ ] **Step 5: Commit** - -```bash -git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py tests/tooling/test_mcp_server_configuration.py -git commit -m "feat(tooling): parse per-server MCP connection fields - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" -``` - ---- - -## Task 4: Capture aggregate metadata via `McpDiscoveryResult` - -This task adds the internal result wrapper and makes `_parse_gateway_response` / -`_load_servers_from_gateway` carry the response-level (aggregate) connection fields. -`list_tool_servers` is updated to unwrap `.servers` so external behavior is unchanged. - -**Files:** -- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` -- Test: `tests/tooling/test_mcp_server_configuration.py` - -- [ ] **Step 1: Write the failing test** - -Add to class `TestMcpToolServerConfigurationService`: - -```python - @pytest.mark.asyncio - async def test_parse_gateway_response_captures_aggregate(self, service): - """Response-level connection metadata is captured into McpDiscoveryResult.""" - payload = { - "mcpServers": [ - { - "mcpServerName": "mcp_Salesforce", - "mcpServerUniqueName": "mcp_Salesforce", - "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", - "connectivityStatus": "Pending", - } - ], - "allConnectionsUrl": "https://make.example/all", - "missingConnectionsUrl": "https://make.example/missing", - "connectivityStatus": "Pending", - } - mock_response = MagicMock() - mock_response.json = AsyncMock(return_value=payload) - - result = await service._parse_gateway_response(mock_response) - - assert len(result.servers) == 1 - assert result.servers[0].mcp_server_name == "mcp_Salesforce" - assert result.all_connections_url == "https://make.example/all" - assert result.missing_connections_url == "https://make.example/missing" - assert result.connectivity_status == "Pending" - - @pytest.mark.asyncio - async def test_parse_gateway_response_raw_array_has_no_aggregate(self, service): - """Legacy raw-array responses produce a result with aggregate fields None.""" - payload = [ - { - "mcpServerName": "V1Server", - "mcpServerUniqueName": "v1_server", - "url": "https://v1.example.com/mcp", - } - ] - mock_response = MagicMock() - mock_response.json = AsyncMock(return_value=payload) - - result = await service._parse_gateway_response(mock_response) - - assert len(result.servers) == 1 - assert result.all_connections_url is None - assert result.missing_connections_url is None - assert result.connectivity_status is None -``` - -(`MagicMock` and `AsyncMock` are already imported at the top of the test file.) - -- [ ] **Step 2: Run test to verify it fails** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestMcpToolServerConfigurationService::test_parse_gateway_response_captures_aggregate -v` -Expected: FAIL with `AttributeError: 'list' object has no attribute 'servers'` (current -return is a list). - -- [ ] **Step 3: Define `McpDiscoveryResult`** - -In `mcp_tool_server_configuration_service.py`, ensure `dataclass` is imported. The module -already has `from dataclasses import replace as dataclass_replace`; add a second import -line directly above it: - -```python -from dataclasses import dataclass -from dataclasses import replace as dataclass_replace -``` - -Then, in the `# TYPES` section (just below the `TokenAcquirer` definition near line 62), -add: - -```python -@dataclass -class McpDiscoveryResult: - """Internal result of MCP server discovery from the gateway. - - Carries the parsed server list plus the response-level (aggregate) connection - metadata used for connection gating. Sources that predate the connection fields - (legacy raw-array gateway responses, dev-mode manifests) leave the aggregate - fields as ``None``. - """ - - servers: List["MCPServerConfig"] - all_connections_url: Optional[str] = None - missing_connections_url: Optional[str] = None - connectivity_status: Optional[str] = None -``` - -- [ ] **Step 4: Update `_parse_gateway_response` to return `McpDiscoveryResult`** - -Replace the entire body of `_parse_gateway_response` (the method around lines 639–679) so -it returns a `McpDiscoveryResult`. Keep the existing wrapped/raw-array branching; add -aggregate extraction for the wrapped shape: - -```python - async def _parse_gateway_response( - self, response: aiohttp.ClientResponse - ) -> McpDiscoveryResult: - """ - Parses the response from the tooling gateway. - - Supports two response shapes: - - Wrapped: ``{"mcpServers": [...], "connectivityStatus": ..., ...}`` - - Raw array: ``[...]`` (legacy V1 gateway format, no aggregate metadata) - - Args: - response: HTTP response from the gateway. - - Returns: - McpDiscoveryResult: parsed servers plus response-level connection metadata - (aggregate fields are None for the legacy raw-array shape). - """ - config_data = await response.json(content_type=None) - - server_elements: Optional[List[object]] = None - all_connections_url: Optional[str] = None - missing_connections_url: Optional[str] = None - connectivity_status: Optional[str] = None - - if isinstance(config_data, list): - # Raw array format (legacy V1 gateway returns bare array, no aggregate). - self._logger.debug("Gateway returned raw array response") - server_elements = config_data - elif isinstance(config_data, dict) and isinstance(config_data.get("mcpServers"), list): - # Wrapped format: {"mcpServers": [...], aggregate connection fields} - self._logger.debug("Gateway returned wrapped mcpServers response") - server_elements = config_data["mcpServers"] - - all_raw = config_data.get("allConnectionsUrl") - all_connections_url = str(all_raw) if all_raw is not None else None - - missing_raw = config_data.get("missingConnectionsUrl") - missing_connections_url = str(missing_raw) if missing_raw is not None else None - - status_raw = config_data.get("connectivityStatus") - connectivity_status = str(status_raw) if status_raw is not None else None - else: - self._logger.warning( - 'Unexpected gateway response format: expected a list or {"mcpServers": [...]}' - ) - return McpDiscoveryResult(servers=[]) - - mcp_servers: List[MCPServerConfig] = [] - for server_element in server_elements: - if isinstance(server_element, dict): - server_config = self._parse_server_config(server_element) - if server_config is not None: - mcp_servers.append(server_config) - - return McpDiscoveryResult( - servers=mcp_servers, - all_connections_url=all_connections_url, - missing_connections_url=missing_connections_url, - connectivity_status=connectivity_status, - ) -``` - -- [ ] **Step 5: Update `_load_servers_from_gateway` to return `McpDiscoveryResult`** - -In `_load_servers_from_gateway` (around lines 485–537), change the return type annotation -from `List[MCPServerConfig]` to `McpDiscoveryResult`, and update the success branch. -Locate: - -```python - if response.status == 200: - mcp_servers = await self._parse_gateway_response(response) - self._logger.info( - f"Retrieved {len(mcp_servers)} MCP tool servers from tooling gateway" - ) - return mcp_servers -``` - -Replace with: - -```python - if response.status == 200: - discovery = await self._parse_gateway_response(response) - self._logger.info( - f"Retrieved {len(discovery.servers)} MCP tool servers " - f"from tooling gateway" - ) - return discovery -``` - -Also change the method signature return annotation line from: - -```python - ) -> List[MCPServerConfig]: -``` -to: -```python - ) -> McpDiscoveryResult: -``` - -- [ ] **Step 6: Update `list_tool_servers` prod branch to unwrap `.servers`** - -In `list_tool_servers`, the production branch currently is: - -```python - else: - servers = await self._load_servers_from_gateway( - agentic_app_id, auth_token, options, turn_context - ) -``` - -Replace with: - -```python - else: - discovery = await self._load_servers_from_gateway( - agentic_app_id, auth_token, options, turn_context - ) - servers = discovery.servers -``` - -(The gate is added in Task 5 — for now we just unwrap so existing behavior is preserved.) - -- [ ] **Step 7: Run tests to verify they pass** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py -v` -Expected: PASS — the two new aggregate tests pass and all pre-existing tests still pass -(production `list_tool_servers` tests continue to work because `.servers` is unwrapped). - -- [ ] **Step 8: Commit** - -```bash -git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py tests/tooling/test_mcp_server_configuration.py -git commit -m "feat(tooling): capture aggregate connection metadata via McpDiscoveryResult - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" -``` - ---- - -## Task 5: Enforce the connection-readiness gate in `list_tool_servers` - -**Files:** -- Modify: `libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py` -- Test: `tests/tooling/test_mcp_server_configuration.py` - -- [ ] **Step 1: Write the failing tests** - -Add a new test class to `tests/tooling/test_mcp_server_configuration.py`. The -`_gateway_response` helper builds the nested aiohttp mock used elsewhere in this file. - -```python -class TestConnectionGating: - """Tests for the connectivityStatus connection-readiness gate.""" - - @pytest.fixture - def service(self): - return McpToolServerConfigurationService() - - @staticmethod - def _gateway_response(payload): - """Build a patched aiohttp.ClientSession context manager returning payload.""" - mock_response = MagicMock() - mock_response.status = 200 - mock_response.json = AsyncMock(return_value=payload) - mock_response_cm = MagicMock() - mock_response_cm.__aenter__ = AsyncMock(return_value=mock_response) - mock_response_cm.__aexit__ = AsyncMock(return_value=None) - mock_session = MagicMock() - mock_session.get = MagicMock(return_value=mock_response_cm) - mock_session_cm = MagicMock() - mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session) - mock_session_cm.__aexit__ = AsyncMock(return_value=None) - return patch("aiohttp.ClientSession", return_value=mock_session_cm) - - @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) - @pytest.mark.asyncio - async def test_gate_raises_when_pending(self, service): - from microsoft_agents_a365.tooling import McpConnectionsRequiredError - - payload = { - "mcpServers": [ - { - "mcpServerName": "mcp_Salesforce", - "mcpServerUniqueName": "mcp_Salesforce", - "url": "https://gw.example/mcp_Salesforce", - "connectivityStatus": "Pending", - } - ], - "allConnectionsUrl": "https://make.example/all", - "missingConnectionsUrl": "https://make.example/missing", - "connectivityStatus": "Pending", - } - with self._gateway_response(payload): - with pytest.raises(McpConnectionsRequiredError) as exc_info: - await service.list_tool_servers( - agentic_app_id="test-app-id", auth_token="test-token" - ) - err = exc_info.value - assert err.connectivity_status == "Pending" - assert err.missing_connections_url == "https://make.example/missing" - assert err.all_connections_url == "https://make.example/all" - assert "mcp_Salesforce" in err.server_names - - @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) - @pytest.mark.asyncio - async def test_gate_passes_when_ready(self, service): - payload = { - "mcpServers": [ - { - "mcpServerName": "mcp_Salesforce", - "mcpServerUniqueName": "mcp_Salesforce", - "url": "https://gw.example/mcp_Salesforce", - "connectivityStatus": "Ready", - } - ], - "allConnectionsUrl": "https://make.example/all", - "missingConnectionsUrl": "https://make.example/missing", - "connectivityStatus": "Ready", - } - with self._gateway_response(payload): - servers = await service.list_tool_servers( - agentic_app_id="test-app-id", auth_token="test-token" - ) - assert len(servers) == 1 - assert servers[0].mcp_server_name == "mcp_Salesforce" - - @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) - @pytest.mark.asyncio - async def test_gate_passes_for_legacy_raw_array(self, service): - payload = [ - { - "mcpServerName": "V1Server", - "mcpServerUniqueName": "v1_server", - "url": "https://v1.example.com/mcp", - } - ] - with self._gateway_response(payload): - servers = await service.list_tool_servers( - agentic_app_id="test-app-id", auth_token="test-token" - ) - assert len(servers) == 1 - assert servers[0].mcp_server_name == "V1Server" - - @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") - @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) - @pytest.mark.asyncio - async def test_gate_not_applied_in_dev_mode(self, mock_load_manifest, service): - mock_load_manifest.return_value = [ - MCPServerConfig( - mcp_server_name="DevServer", - mcp_server_unique_name="dev_server", - url="https://dev.server/mcp", - ) - ] - servers = await service.list_tool_servers( - agentic_app_id="test-app-id", auth_token="test-token" - ) - assert len(servers) == 1 - assert servers[0].mcp_server_name == "DevServer" -``` - -(`MagicMock`, `AsyncMock`, `patch`, `os`, and `pytest` are already imported at the top of -the existing test file.) - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestConnectionGating -v` -Expected: `test_gate_raises_when_pending` FAILS (no exception raised — gate not yet -implemented); the other three may pass already. - -- [ ] **Step 3: Import the exception in the service module** - -In `mcp_tool_server_configuration_service.py`, with the other local imports (near the -`from ..models import ...` / `from ..utils import Constants` block), add: - -```python -from ..exceptions import McpConnectionsRequiredError -``` - -- [ ] **Step 4: Add the gate helper method** - -Add a private method to `McpToolServerConfigurationService` (place it just after -`list_tool_servers`, before the `# ENVIRONMENT DETECTION` section comment): - -```python - # Sentinel aggregate status that means all connections are satisfied. - _CONNECTIVITY_READY = "Ready" - - def _enforce_connection_readiness(self, discovery: "McpDiscoveryResult") -> None: - """Raise if the aggregate connectivity status indicates missing connections. - - Blocks only when the response-level ``connectivity_status`` is present and not - ``"Ready"`` (i.e. ``"Pending"``). Absent status (legacy raw-array gateway - responses, dev-mode manifests) is always treated as ready, so those paths are - never gated. The ``!= "Ready"`` form is intentionally defensive against any - unexpected future status value. - - Raises: - McpConnectionsRequiredError: when connections are not yet ready. - """ - status = discovery.connectivity_status - if status is None or status == self._CONNECTIVITY_READY: - return - - not_ready = [ - s - for s in discovery.servers - if s.connectivity_status is not None - and s.connectivity_status != self._CONNECTIVITY_READY - ] - server_names = [ - s.mcp_server_name or s.mcp_server_unique_name for s in not_ready - ] - self._logger.info( - f"MCP connection gate blocking turn: connectivityStatus={status}, " - f"servers={server_names}" - ) - raise McpConnectionsRequiredError( - missing_connections_url=discovery.missing_connections_url, - all_connections_url=discovery.all_connections_url, - connectivity_status=status, - server_names=server_names, - ) -``` - -- [ ] **Step 5: Call the gate from the prod branch of `list_tool_servers`** - -Update the production branch added in Task 4 to enforce the gate immediately after -discovery, before token branching: - -```python - else: - discovery = await self._load_servers_from_gateway( - agentic_app_id, auth_token, options, turn_context - ) - # Gate execution when configured MCP servers are not connection-ready. - # Runs before token attachment because readiness is independent of tokens. - self._enforce_connection_readiness(discovery) - servers = discovery.servers -``` - -- [ ] **Step 6: Run the gate tests to verify they pass** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py::TestConnectionGating -v` -Expected: PASS (4 tests). - -- [ ] **Step 7: Run the full tooling test module** - -Run: `uv run --frozen pytest tests/tooling/test_mcp_server_configuration.py -v` -Expected: PASS (all tests, including pre-existing). - -- [ ] **Step 8: Commit** - -```bash -git add libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py tests/tooling/test_mcp_server_configuration.py -git commit -m "feat(tooling): gate tool discovery on MCP connectivityStatus - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" -``` - ---- - -## Task 6: Documentation and changelog - -**Files:** -- Modify: `libraries/microsoft-agents-a365-tooling/CHANGELOG.md` -- Modify: `libraries/microsoft-agents-a365-tooling/docs/design.md` - -- [ ] **Step 1: Add CHANGELOG entries** - -In `libraries/microsoft-agents-a365-tooling/CHANGELOG.md`, under -`## [Unreleased]` → `### Added`, append these bullets: - -```markdown -- Added connection-readiness gating to `McpToolServerConfigurationService.list_tool_servers()`. When the tooling gateway reports an aggregate `connectivityStatus` of `"Pending"`, a `McpConnectionsRequiredError` is raised carrying the response-level `missingConnectionsUrl` / `allConnectionsUrl` and the names of the not-`Ready` servers, so the agent's turn handler can prompt the user to set up connections instead of running tools that would fail. Dev-mode manifests and legacy raw-array gateway responses omit the field and are never gated -- Added `McpConnectionsRequiredError` exception (exported from `microsoft_agents_a365.tooling`) with `missing_connections_url`, `all_connections_url`, `connectivity_status`, and `server_names` attributes -- Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload -- Added internal `McpDiscoveryResult` dataclass; `_parse_gateway_response()` and `_load_servers_from_gateway()` now return it to carry response-level connection metadata alongside the server list (the public return type of `list_tool_servers()` remains `List[MCPServerConfig]`) -``` - -- [ ] **Step 2: Document the catch-and-reply pattern in design.md** - -In `libraries/microsoft-agents-a365-tooling/docs/design.md`, add a new subsection (place -it after the existing `list_tool_servers` documentation, around line 227). Use this -content: - -````markdown -### Connection gating - -When the tooling gateway reports that an agent's MCP servers have unsatisfied downstream -connections, `list_tool_servers()` raises `McpConnectionsRequiredError`. Discovery runs -every turn, so the agent's turn handler should catch the error, reply with the -connection-setup link, and return — a later turn proceeds automatically once the user has -connected. - -```python -from microsoft_agents_a365.tooling import McpConnectionsRequiredError - -try: - servers = await config_service.list_tool_servers( - agentic_app_id=agentic_app_id, - auth_token=auth_token, - authorization=auth, - auth_handler_name=auth_handler_name, - turn_context=context, - ) -except McpConnectionsRequiredError as err: - await context.send_activity( - f"Before I can help, please set up the required connections for " - f"{', '.join(err.server_names)}: {err.missing_connections_url}" - ) - return # Skip running the model/tools this turn. -``` - -The gate fires only for gateway responses with aggregate `connectivityStatus == "Pending"`. -Dev-mode manifests and legacy raw-array responses omit the field and are never gated. -```` - -- [ ] **Step 3: Lint and format the whole change set** - -Run: `uv run --frozen ruff check .` -Expected: no errors (fix any reported with `uv run --frozen ruff check . --fix`). - -Run: `uv run --frozen ruff format .` -Expected: files formatted (or "X files left unchanged"). - -- [ ] **Step 4: Run the full tooling test suite once more** - -Run: `uv run --frozen pytest tests/tooling/ -v -m "not integration"` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add libraries/microsoft-agents-a365-tooling/CHANGELOG.md libraries/microsoft-agents-a365-tooling/docs/design.md -git commit -m "docs(tooling): document MCP connection gating - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>" -``` - ---- - -## Final verification - -- [ ] Run lint: `uv run --frozen ruff check .` → no errors. -- [ ] Run format check: `uv run --frozen ruff format --check .` → clean. -- [ ] Run tooling tests: `uv run --frozen pytest tests/tooling/ -v -m "not integration"` → all pass. -- [ ] Confirm `McpConnectionsRequiredError` importable: `uv run --frozen python -c "from microsoft_agents_a365.tooling import McpConnectionsRequiredError; print('ok')"` → prints `ok`. - -## Notes on scope - -- Framework tooling extensions (`openai`, `semantickernel`, `agentframework`, - `googleadk`, `azureaifoundry`) require **no code change**: their - `add_tool_servers_to_agent` calls `list_tool_servers` and does not catch the new - exception, so it propagates to the turn handler as designed. -- No poll/wait loop is implemented — per-turn discovery (no caching) provides the retry - loop naturally. diff --git a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md b/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md deleted file mode 100644 index d30028aa..00000000 --- a/docs/superpowers/specs/2026-06-08-mcp-connection-gating-design.md +++ /dev/null @@ -1,222 +0,0 @@ -# MCP Connection-Gated Tool Discovery — Design - -**Date:** 2026-06-08 -**Status:** Proposed -**Package(s):** `microsoft-agents-a365-tooling` (core); framework tooling extensions inherit behavior. - -## Problem - -The tooling gateway discovery response for MCP servers has gained connection-state -fields. A server may be configured for an agent but not yet have its required -downstream connections (e.g. a Salesforce or Zendesk connector) established by the -user. If the agent runs tools against such a server, calls fail at execution time. - -The runtime calls discovery on **every turn** when spinning up tools (there is no -caching of the server list — discovery re-runs each time `list_tool_servers` is -invoked). We want to use that per-turn discovery to **gate agent execution**: until -all required connections are present, the turn must not proceed. Instead the agent -should reply to the user with a link to set up the missing connections, and a later -turn (after the user connects) proceeds normally. - -## New Schema - -The wrapped gateway response now carries connection metadata at **two levels** — -per-server and response-level (aggregate): - -```json -{ - "mcpServers": [ - { - "mcpServerName": "mcp_Salesforce", - "id": "3fa85f64-5717-4562-b3fc-2c963f66afa6", - "publisher": "Microsoft", - "url": "https://agent365.svc.cloud.dev.microsoft/agents/v2/servers/mcp_Salesforce", - "scope": "McpServers.Salesforce.All", - "audience": "", - "allConnectionsUrl": "", - "missingConnectionsUrl": "", - "connectivityStatus": "" - } - ], - "allConnectionsUrl": "", - "missingConnectionsUrl": "", - "connectivityStatus": "" -} -``` - -Notes: -- The **response-level** `connectivityStatus` / `missingConnectionsUrl` / - `allConnectionsUrl` are the authoritative aggregate signal used for gating and for - the single "fix everything" link surfaced to the user. -- The **per-server** copies are parsed and retained for diagnostics only; they do not - drive the gate decision. -- All JSON keys are camelCase: `connectivityStatus`, `allConnectionsUrl`, - `missingConnectionsUrl`, `id`, `mcpServerName`. Parsing uses these exact keys (no - case-variant fallback needed). - -## Compatibility Rules - -| Source | Aggregate `connectivityStatus` | Gate behavior | -|---|---|---| -| V2 gateway, all connections satisfied | `"Ready"` | proceeds | -| V2 gateway, missing connections | `"Pending"` | **blocks**, raises exception | -| Legacy `ToolingManifest.json` | absent → `None` | proceeds (not gated) | - -The gateway only ever emits `"Ready"` or `"Pending"` for `connectivityStatus` — never -`null` and never any other value. Sources that predate the field (legacy manifests) omit it entirely, yielding `None`. - -**Gate rule:** block only when the aggregate `connectivity_status` is **present and -not equal to** `"Ready"` (i.e. `"Pending"`). Absent status (`None`) is always treated -as ready, so dev mode and legacy callers are unaffected. The `!= "Ready"` form (rather -than `== "Pending"`) is deliberately defensive against any unexpected future value. - -## Design - -### 1. Model — `MCPServerConfig` - -Add four optional fields (all default `None`, preserving existing constructor calls): - -- `id: Optional[str]` -- `all_connections_url: Optional[str]` -- `missing_connections_url: Optional[str]` -- `connectivity_status: Optional[str]` - -These hold the **per-server** values. No validation change. - -### 2. Internal discovery result — `McpDiscoveryResult` - -New internal dataclass (not part of the public return type) to carry response-level -aggregate metadata alongside the parsed servers: - -```python -@dataclass -class McpDiscoveryResult: - servers: List[MCPServerConfig] - all_connections_url: Optional[str] = None - missing_connections_url: Optional[str] = None - connectivity_status: Optional[str] = None -``` - -### 3. Parsing changes - -- `_parse_server_config` reads `id`, `allConnectionsUrl`, `missingConnectionsUrl`, - `connectivityStatus` (exact camelCase key) onto each `MCPServerConfig`. Gateway and - manifest share this path; manifest entries simply lack the fields → `None`. -- `_parse_gateway_response` returns `McpDiscoveryResult`: it parses the server list as - today and additionally extracts the **response-level** aggregate fields when the - wrapped `{"mcpServers": [...]}` shape is present. The legacy raw-array shape yields a - result with aggregate fields = `None`. -- `_load_servers_from_gateway` returns `McpDiscoveryResult`. -- The manifest/dev path wraps its server list in a `McpDiscoveryResult` with aggregate - fields = `None`. - -### 4. Readiness gate — core `list_tool_servers` - -After discovery and per-audience token attachment, evaluate the aggregate status: - -```python -if (result.connectivity_status is not None - and result.connectivity_status != "Ready"): - not_ready = [s for s in result.servers - if s.connectivity_status is not None - and s.connectivity_status != "Ready"] - raise McpConnectionsRequiredError( - missing_connections_url=result.missing_connections_url, - all_connections_url=result.all_connections_url, - connectivity_status=result.connectivity_status, - server_names=[s.mcp_server_name or s.mcp_server_unique_name - for s in not_ready], - ) -``` - -The public return type of `list_tool_servers` remains `List[MCPServerConfig]` -(`result.servers`). The gate is the only new externally observable behavior, and only -fires for gateway responses reporting `connectivityStatus: "Pending"`. - -### 5. Exception — `McpConnectionsRequiredError` - -New exception type in the tooling package, exported from -`microsoft_agents_a365.tooling`: - -```python -class McpConnectionsRequiredError(Exception): - def __init__( - self, - missing_connections_url: Optional[str], - all_connections_url: Optional[str], - connectivity_status: Optional[str], - server_names: List[str], - ) -> None: - ... -``` - -Exposes (per design decision) the **response-level** aggregate links plus the list of -not-`Ready` server names for context: -- `missing_connections_url` — single aggregate link to set up missing connections - (primary value the handler surfaces to the user). -- `all_connections_url` — aggregate link to view/manage all connections. -- `connectivity_status` — the aggregate status that triggered the block (`"Pending"`). -- `server_names` — names of servers that are not yet `Ready`. - -The `__str__`/message includes the missing-connections URL and server names so logs -are actionable. - -### 6. Per-turn behavior & propagation - -- No poll loop. Discovery already runs every turn (no list caching), so each turn - re-hits the gateway. Once the user completes the connections, a subsequent turn - passes the gate. -- Framework tooling extensions (`add_tool_servers_to_agent` in openai, - semantickernel, agentframework, googleadk, azureaifoundry) **do not catch** - `McpConnectionsRequiredError` — they let it propagate. -- The **agent's turn handler** (application code) catches it, replies to the user with - the `missing_connections_url`, and returns without running the model/tools. This - reply formatting is application responsibility; the SDK only signals and supplies the - URLs. Documentation/sample will show the recommended catch-and-reply pattern. - -## Out of Scope (YAGNI) - -- Polling / blocking wait loops within a turn. -- Partial execution with only the connected servers (decision: block entirely on any - unsatisfied aggregate status). -- The SDK itself sending the connection-setup reply (left to the turn handler). -- Caching discovery results across turns. - -## Testing - -- **Parser:** per-server new fields populated from a V2 element; absent in manifest - element → `None`. -- **Aggregate parsing:** `_parse_gateway_response` extracts response-level fields for - wrapped shape; raw-array shape → aggregate `None`. -- **Gate fires:** aggregate `connectivity_status == "Pending"` raises - `McpConnectionsRequiredError` carrying the correct response-level URLs and the - not-Ready server names. -- **Gate passes:** aggregate `"Ready"`; aggregate absent (dev manifest); legacy - raw array. -- **Exception payload:** `missing_connections_url` / `all_connections_url` / - `connectivity_status` / `server_names` correct; message string actionable. -- **Propagation:** extension `add_tool_servers_to_agent` does not swallow the exception - (it surfaces to the caller). - -## Affected Files - -- `libraries/microsoft-agents-a365-tooling/.../models/mcp_server_config.py` — new fields. -- `libraries/microsoft-agents-a365-tooling/.../models/__init__.py` — export - `McpDiscoveryResult` if placed in models (or keep internal to the service module). -- `libraries/microsoft-agents-a365-tooling/.../services/mcp_tool_server_configuration_service.py` - — parsing + gate. -- `libraries/microsoft-agents-a365-tooling/.../exceptions.py` (new) — - `McpConnectionsRequiredError`. -- `libraries/microsoft-agents-a365-tooling/.../__init__.py` — export exception. -- `tests/tooling/...` — parser, gate, exception tests. -- `libraries/microsoft-agents-a365-tooling/CHANGELOG.md` — changelog entry. -- Tooling extension docs/samples — catch-and-reply pattern (follow-up). - -## Resolved Decisions - -1. JSON keys are camelCase (`connectivityStatus`, `allConnectionsUrl`, - `missingConnectionsUrl`, etc.) at both response and server level — parsed with exact - keys. -2. `connectivityStatus` is always `"Ready"` or `"Pending"` (never `null` or other - values) from the gateway. Field is absent only from legacy raw-array responses and - dev-mode manifests. From 168e8b528acef9542ddedf4221818596a9e6fbe7 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 15 Jun 2026 14:02:43 -0700 Subject: [PATCH 14/15] Relocate MCP connection gating to agentframework extension Move connection-readiness gating out of the core tooling package and into the Agent Framework extension. Core now only parses the per-server schema fields (connectivityStatus, allConnectionsUrl, missingConnectionsUrl) onto each MCPServerConfig and never gates or raises. Core: - Remove the internal McpDiscoveryResult dataclass, the deprecated _enforce_connection_readiness method, and the _CONNECTIVITY_READY constant. - _parse_gateway_response/_load_servers_from_gateway return List[MCPServerConfig] again; response-level aggregate fields are ignored. - Delete exceptions.py and drop McpConnectionsRequiredError from the public API. Extension (agentframework): - Own McpConnectionsRequiredError (all_connections_url, connectivity_status, server_names) and export it from the package root. - Gate at registration time in add_tool_servers_to_agent: when any discovered server is Pending, raise before building the agent so the turn handler can prompt the user with all_connections_url. Replaces the per-server FunctionTool placeholder mechanism. Raising at construction time (not inside a FunctionTool) is required because Agent Framework's tool-call loop swallows exceptions raised inside a tool and returns 'Error: Function failed.' to the model, which would drop the setup URL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../docs/design.md | 48 +++- .../extensions/agentframework/__init__.py | 2 + .../extensions/agentframework/exceptions.py | 44 ++++ .../services/mcp_tool_registration_service.py | 61 ++++- .../CHANGELOG.md | 6 +- .../docs/design.md | 38 +-- .../microsoft_agents_a365/tooling/__init__.py | 2 - .../tooling/exceptions.py | 33 --- .../mcp_tool_server_configuration_service.py | 118 +++------ .../test_mcp_tool_registration_service.py | 233 ++++++++++++++++++ .../test_mcp_connections_required_error.py | 32 --- .../tooling/test_mcp_server_configuration.py | 88 ++++--- 12 files changed, 475 insertions(+), 230 deletions(-) create mode 100644 libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py delete mode 100644 libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py delete mode 100644 tests/tooling/test_mcp_connections_required_error.py diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md index 7d8af5ee..7851236d 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md @@ -41,7 +41,8 @@ McpToolRegistrationService.add_tool_servers_to_agent() │ ├── Resolve agent identity ├── Exchange token for MCP scope - ├── Create MCPStreamableHTTPTool for each server + ├── Gate: raise McpConnectionsRequiredError if any server is Pending + ├── Create MCPStreamableHTTPTool for each (Ready) server └── Create ChatAgent with all tools │ ▼ @@ -76,6 +77,51 @@ mcp_tool = MCPStreamableHTTPTool( ) ``` +### Connection-readiness gating + +MCP server discovery runs every turn. The gateway reports each server's +`connectivityStatus` (`"Ready"` or `"Pending"`) along with an `allConnectionsUrl` the user +can visit to view and manage the connectors required by that server. When **any** discovered +server is `Pending`, `add_tool_servers_to_agent` raises `McpConnectionsRequiredError` +**before** building the agent, so the developer's turn handler can prompt the user to set up +connections and return. A later turn re-runs discovery and proceeds automatically once the +connections are in place. + +The gate raises at agent-construction time — not from inside a tool call — on purpose. Agent +Framework's tool-call loop catches exceptions raised inside a `FunctionTool` and reflects them +to the model as an opaque error string (`"Error: Function failed."` by default), which would +drop the setup URL before it reached the user. Raising during construction lets the typed +exception propagate intact to the turn handler. + +```python +from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpToolRegistrationService, + McpConnectionsRequiredError, +) + +service = McpToolRegistrationService() + +try: + agent = await service.add_tool_servers_to_agent( + chat_client=chat_client, + agent_instructions="You are a helpful assistant.", + initial_tools=[], + auth=auth_context, + auth_handler_name="graph", + turn_context=turn_context, + ) +except McpConnectionsRequiredError as err: + await turn_context.send_activity( + f"Before I can help, please set up the required connections for " + f"{', '.join(err.server_names)}: {err.all_connections_url}" + ) + return # Skip running the model/tools this turn. +``` + +`McpConnectionsRequiredError` exposes `all_connections_url`, `connectivity_status`, and +`server_names`. It is owned and exported by this extension (`microsoft_agents_a365.tooling` +core only parses the per-server connection metadata; it never gates or raises). + ### Chat History API The service provides methods to send chat history to the MCP platform for real-time threat protection analysis. This enables security scanning of conversation content. diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py index c4ba4378..f26814e1 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/__init__.py @@ -21,8 +21,10 @@ __version__ = "1.0.0" # Import services from the services module +from .exceptions import McpConnectionsRequiredError from .services import McpToolRegistrationService __all__ = [ "McpToolRegistrationService", + "McpConnectionsRequiredError", ] diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py new file mode 100644 index 00000000..673078ad --- /dev/null +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py @@ -0,0 +1,44 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Exceptions raised by the Agent Framework MCP tooling extension.""" + +from typing import List, Optional + + +class McpConnectionsRequiredError(Exception): + """Raised when one or more configured MCP servers are not yet connection-ready. + + The tooling gateway reports a per-server ``connectivityStatus`` of ``"Pending"`` + when an MCP server has downstream connections the user has not yet established. + ``McpToolRegistrationService.add_tool_servers_to_agent`` discovers the servers + every turn and raises this error *before* building the agent when any server is + Pending, so the agent's turn handler can catch it, reply to the user with + ``all_connections_url``, and return without running the model/tools. A later turn + re-runs discovery and proceeds once the connections are in place. + + The error is raised at agent-construction time (not from inside a tool call) so it + propagates to the developer's turn handler intact — Agent Framework's tool-call loop + swallows exceptions raised inside a ``FunctionTool`` and reflects them to the model + as an opaque error string, which would drop the setup URL. + + ``all_connections_url`` lets the user view and manage the full set of connectors + required by the affected servers — including ones already set up — rather than being + scoped strictly to the ones that happen to be missing right now. + """ + + def __init__( + self, + all_connections_url: Optional[str], + connectivity_status: Optional[str], + server_names: List[str], + ) -> None: + self.all_connections_url = all_connections_url + self.connectivity_status = connectivity_status + self.server_names = server_names + servers_text = ", ".join(server_names) if server_names else "(unknown)" + super().__init__( + f"MCP servers [{servers_text}] require connection setup " + f"(connectivityStatus={connectivity_status}). " + f"Set up connections at: {all_connections_url}" + ) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py index f9c8d8b9..7f557885 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py @@ -28,10 +28,18 @@ is_development_environment, ) +from ..exceptions import McpConnectionsRequiredError + # Default timeout for MCP server HTTP requests (in seconds) MCP_HTTP_CLIENT_TIMEOUT_SECONDS = 90.0 +# Sentinel per-server status that means a server's downstream connections are +# already in place. Anything else (typically "Pending") triggers the +# connection-readiness gate, which raises McpConnectionsRequiredError before the +# agent is built so the turn handler can prompt the user to set up connections. +_CONNECTIVITY_READY = "Ready" + class McpToolRegistrationService: """ @@ -114,11 +122,59 @@ async def add_tool_servers_to_agent( ) self._logger.info(f"Loaded {len(server_configs)} MCP server configurations") + for c in server_configs: + _name = c.mcp_server_name or c.mcp_server_unique_name + self._logger.info( + " per-server gateway state: name=%s connectivity_status=%r missing_url=%s", + _name, + c.connectivity_status, + c.missing_connections_url, + ) + + # Connection-readiness gate. Discovery runs every turn; when the + # gateway flags any MCP server's downstream connection(s) as not yet + # set up (``connectivityStatus == "Pending"``), raise before building + # the agent so the developer's turn handler can catch the error, + # prompt the user with ``all_connections_url``, and return without + # running the model/tools. A later turn re-runs discovery and + # proceeds automatically once the connections are in place. + # + # The gate raises here — at agent-construction time — rather than + # from inside a tool call on purpose: Agent Framework's tool-call + # loop swallows exceptions raised inside a ``FunctionTool`` and + # reflects them to the model as an opaque error string, which would + # drop the setup URL before it ever reaches the user. + pending = [ + c + for c in server_configs + if c.connectivity_status is not None + and c.connectivity_status != _CONNECTIVITY_READY + ] + if pending: + pending_names = [c.mcp_server_name or c.mcp_server_unique_name for c in pending] + # Surface all_connections_url so the user can view and manage the + # full set of connectors required by the affected servers. Use the + # first pending server that provides one (they are environment-scoped + # and typically identical across servers). + all_connections_url = next( + (c.all_connections_url for c in pending if c.all_connections_url), + None, + ) + self._logger.info( + "MCP connection gate blocking turn: pending servers=%s, setup URL=%s", + pending_names, + all_connections_url, + ) + raise McpConnectionsRequiredError( + all_connections_url=all_connections_url, + connectivity_status=pending[0].connectivity_status, + server_names=pending_names, + ) # Create the agent with all tools (initial + MCP tools) all_tools = list(initial_tools) - # Add servers as MCPStreamableHTTPTool instances + # Add each Ready server as an MCPStreamableHTTPTool instance. for config in server_configs: # Use mcp_server_name if available (not None or empty), otherwise fall back to mcp_server_unique_name server_name = config.mcp_server_name or config.mcp_server_unique_name @@ -183,6 +239,9 @@ async def add_tool_servers_to_agent( self._logger.info(f"Agent created with {len(all_tools)} total tools") return agent + except McpConnectionsRequiredError: + # Connection-readiness gate — propagate intact to the turn handler. + raise except Exception as ex: self._logger.error(f"Failed to add tool servers to agent: {ex}") raise diff --git a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md index 8aa1fd93..59264b69 100644 --- a/libraries/microsoft-agents-a365-tooling/CHANGELOG.md +++ b/libraries/microsoft-agents-a365-tooling/CHANGELOG.md @@ -35,7 +35,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `ChatHistoryMessage` Pydantic model for representing individual messages in chat history - Added `ChatMessageRequest` Pydantic model for the chat history API request payload - Added `py.typed` marker for PEP 561 compliance, enabling type checker support -- Added connection-readiness gating to `McpToolServerConfigurationService.list_tool_servers()`. When the tooling gateway reports an aggregate `connectivityStatus` of `"Pending"`, a `McpConnectionsRequiredError` is raised carrying the response-level `missingConnectionsUrl` and the names of the not-`Ready` servers, so the agent's turn handler can prompt the user to set up connections instead of running tools that would fail. Dev-mode manifests and legacy raw-array gateway responses omit the field and are never gated -- Added `McpConnectionsRequiredError` exception (exported from `microsoft_agents_a365.tooling`) with `missing_connections_url`, `connectivity_status`, and `server_names` attributes -- Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload -- Added internal `McpDiscoveryResult` dataclass; `_parse_gateway_response()` and `_load_servers_from_gateway()` now return it to carry response-level connection metadata alongside the server list (the public return type of `list_tool_servers()` remains `List[MCPServerConfig]`) +- Added `id`, `all_connections_url`, `missing_connections_url`, and `connectivity_status` fields to `MCPServerConfig`, parsed from the per-server gateway payload (`allConnectionsUrl`, `missingConnectionsUrl`, `connectivityStatus`). The core service parses this connection metadata onto each server but does not gate on it — connection-readiness gating is owned by the framework-specific tooling extensions +- `_parse_gateway_response()` and `_load_servers_from_gateway()` return `List[MCPServerConfig]`; the wrapped `{"mcpServers": [...]}` and legacy raw-array gateway response shapes are both supported, and response-level (aggregate) connection fields are ignored diff --git a/libraries/microsoft-agents-a365-tooling/docs/design.md b/libraries/microsoft-agents-a365-tooling/docs/design.md index c0a6def6..3d9209d1 100644 --- a/libraries/microsoft-agents-a365-tooling/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling/docs/design.md @@ -108,35 +108,15 @@ User-Agent: Agent365SDK/0.1.0 (...) The gateway returns the same JSON structure, but `mcpServerUniqueName` contains the full endpoint URL. -### Connection gating - -When the tooling gateway reports that an agent's MCP servers have unsatisfied downstream -connections, `list_tool_servers()` raises `McpConnectionsRequiredError`. Discovery runs -every turn, so the agent's turn handler should catch the error, reply with the -connection-setup link, and return — a later turn proceeds automatically once the user has -connected. - -```python -from microsoft_agents_a365.tooling import McpConnectionsRequiredError - -try: - servers = await config_service.list_tool_servers( - agentic_app_id=agentic_app_id, - auth_token=auth_token, - authorization=auth, - auth_handler_name=auth_handler_name, - turn_context=context, - ) -except McpConnectionsRequiredError as err: - await context.send_activity( - f"Before I can help, please set up the required connections for " - f"{', '.join(err.server_names)}: {err.missing_connections_url}" - ) - return # Skip running the model/tools this turn. -``` - -The gate fires only for gateway responses with aggregate `connectivityStatus == "Pending"`. -Dev-mode manifests and legacy raw-array responses omit the field and are never gated. +### Connection metadata (per-server) + +The core service parses each server's connection metadata — `connectivityStatus`, +`allConnectionsUrl`, and `missingConnectionsUrl` — onto the returned `MCPServerConfig` +objects, but it does **not** gate on them. Connection-readiness gating is the +responsibility of the framework-specific tooling extensions (e.g. +`microsoft-agents-a365-tooling-extensions-agentframework`), which decide how to surface a +`Pending` server and raise/handle the appropriate error. See the relevant extension's +design document for details. ### MCPServerConfig ([models/mcp_server_config.py](../microsoft_agents_a365/tooling/models/mcp_server_config.py)) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py index 6a3fc15b..edee7a4a 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/__init__.py @@ -8,7 +8,6 @@ Provides base utilities and common helper functions. """ -from .exceptions import McpConnectionsRequiredError from .models import MCPServerConfig from .services import McpToolServerConfigurationService from .utils import Constants @@ -23,7 +22,6 @@ __all__ = [ "MCPServerConfig", "McpToolServerConfigurationService", - "McpConnectionsRequiredError", "Constants", "get_tooling_gateway_for_digital_worker", "get_mcp_base_url", diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py deleted file mode 100644 index 25fa8e14..00000000 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/exceptions.py +++ /dev/null @@ -1,33 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -"""Exceptions raised by the MCP tooling layer.""" - -from typing import List, Optional - - -class McpConnectionsRequiredError(Exception): - """Raised when one or more configured MCP servers are not yet connection-ready. - - The tooling gateway reports an aggregate ``connectivityStatus`` of ``"Pending"`` - when the agent's MCP servers have downstream connections that the user has not yet - established. The agent's turn handler should catch this error, reply to the user - with ``missing_connections_url``, and return without running the model/tools. - A later turn re-runs discovery and proceeds once the connections are in place. - """ - - def __init__( - self, - missing_connections_url: Optional[str], - connectivity_status: Optional[str], - server_names: List[str], - ) -> None: - self.missing_connections_url = missing_connections_url - self.connectivity_status = connectivity_status - self.server_names = server_names - servers_text = ", ".join(server_names) if server_names else "(unknown)" - super().__init__( - f"MCP servers [{servers_text}] require connection setup " - f"(connectivityStatus={connectivity_status}). " - f"Set up missing connections at: {missing_connections_url}" - ) diff --git a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py index f67eb53f..1a0d018f 100644 --- a/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py +++ b/libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py @@ -23,7 +23,6 @@ import logging import os import uuid -from dataclasses import dataclass from dataclasses import replace as dataclass_replace from pathlib import Path from typing import Awaitable, Callable, Dict, List, Optional @@ -34,7 +33,6 @@ from microsoft_agents.hosting.core import Authorization, TurnContext # Local imports -from ..exceptions import McpConnectionsRequiredError from ..models import ChatHistoryMessage, ChatMessageRequest, MCPServerConfig, ToolOptions from ..utils import Constants from ..utils.utility import ( @@ -64,22 +62,6 @@ TokenAcquirer = Callable[["MCPServerConfig", str], Awaitable[Optional[str]]] -@dataclass -class McpDiscoveryResult: - """Internal result of MCP server discovery from the gateway. - - Carries the parsed server list plus the response-level (aggregate) connection - metadata used for connection gating. Sources that predate the connection fields - (legacy raw-array gateway responses, dev-mode manifests) leave the aggregate - fields as ``None``. - """ - - servers: List["MCPServerConfig"] - all_connections_url: Optional[str] = None - missing_connections_url: Optional[str] = None - connectivity_status: Optional[str] = None - - # ============================================================================== # CONSTANTS # ============================================================================== @@ -173,13 +155,16 @@ async def list_tool_servers( # BEARER_TOKEN_ takes precedence; BEARER_TOKEN is the fallback. acquire: TokenAcquirer = self._create_dev_token_acquirer() else: - discovery = await self._load_servers_from_gateway( + servers = await self._load_servers_from_gateway( agentic_app_id, auth_token, options, turn_context ) - # Gate execution when configured MCP servers are not connection-ready. - # Runs before token attachment because readiness is independent of tokens. - self._enforce_connection_readiness(discovery) - servers = discovery.servers + # Connection-readiness gating is the responsibility of the + # framework-specific tooling extensions (e.g. + # ``microsoft-agents-a365-tooling-extensions-agentframework``). + # Core simply parses and returns the per-server connection + # metadata (``connectivity_status``, ``all_connections_url``, + # ``missing_connections_url``) on each ``MCPServerConfig`` and + # never gates or raises on it. if ( authorization is not None and auth_handler_name is not None @@ -210,42 +195,6 @@ async def list_tool_servers( servers = await self._attach_per_audience_tokens(servers, acquire) return servers - # Sentinel aggregate status that means all connections are satisfied. - _CONNECTIVITY_READY = "Ready" - - def _enforce_connection_readiness(self, discovery: "McpDiscoveryResult") -> None: - """Raise if the aggregate connectivity status indicates missing connections. - - Blocks only when the response-level ``connectivity_status`` is present and not - ``"Ready"`` (i.e. ``"Pending"``). Absent status (legacy raw-array gateway - responses, dev-mode manifests) is always treated as ready, so those paths are - never gated. The ``!= "Ready"`` form is intentionally defensive against any - unexpected future status value. - - Raises: - McpConnectionsRequiredError: when connections are not yet ready. - """ - status = discovery.connectivity_status - if status is None or status == self._CONNECTIVITY_READY: - return - - not_ready = [ - s - for s in discovery.servers - if s.connectivity_status is not None - and s.connectivity_status != self._CONNECTIVITY_READY - ] - server_names = [s.mcp_server_name or s.mcp_server_unique_name for s in not_ready] - self._logger.info( - f"MCP connection gate blocking turn: connectivityStatus={status}, " - f"servers={server_names}" - ) - raise McpConnectionsRequiredError( - missing_connections_url=discovery.missing_connections_url, - connectivity_status=status, - server_names=server_names, - ) - # -------------------------------------------------------------------------- # ENVIRONMENT DETECTION # -------------------------------------------------------------------------- @@ -546,7 +495,7 @@ async def _load_servers_from_gateway( auth_token: str, options: ToolOptions, turn_context: Optional[TurnContext] = None, - ) -> McpDiscoveryResult: + ) -> List[MCPServerConfig]: """ Reads MCP server configurations from tooling gateway endpoint for production scenario. @@ -558,7 +507,8 @@ async def _load_servers_from_gateway( ``activity.id``. A new UUID is generated when not provided. Returns: - McpDiscoveryResult: Discovery result with server list and aggregate connection metadata. + List[MCPServerConfig]: Parsed MCP server configurations, each carrying its + per-server connection metadata. Raises: Exception: If there's an error communicating with the tooling gateway. @@ -573,12 +523,11 @@ async def _load_servers_from_gateway( async with aiohttp.ClientSession(timeout=timeout) as session: async with session.get(config_endpoint, headers=headers) as response: if response.status == 200: - discovery = await self._parse_gateway_response(response) + servers = await self._parse_gateway_response(response) self._logger.info( - f"Retrieved {len(discovery.servers)} MCP tool servers " - f"from tooling gateway" + f"Retrieved {len(servers)} MCP tool servers from tooling gateway" ) - return discovery + return servers else: raise Exception(f"HTTP {response.status}: {await response.text()}") @@ -695,50 +644,44 @@ def _resolve_agent_id_for_header( # Priority 4: Application name from AGENT365_APPLICATION_NAME env or pyproject.toml return RuntimeUtility.get_application_name() - async def _parse_gateway_response(self, response: aiohttp.ClientResponse) -> McpDiscoveryResult: + async def _parse_gateway_response( + self, response: aiohttp.ClientResponse + ) -> List[MCPServerConfig]: """ Parses the response from the tooling gateway. Supports two response shapes: - Wrapped: ``{"mcpServers": [...], "connectivityStatus": ..., ...}`` - - Raw array: ``[...]`` (legacy V1 gateway format, no aggregate metadata) + - Raw array: ``[...]`` (legacy V1 gateway format) + + Per-server connection metadata (``connectivityStatus``, ``allConnectionsUrl``, + ``missingConnectionsUrl``) is parsed onto each ``MCPServerConfig`` by + ``_parse_server_config``. Response-level (aggregate) fields are ignored — gating + is the responsibility of the framework-specific tooling extensions. Args: response: HTTP response from the gateway. Returns: - McpDiscoveryResult: parsed servers plus response-level connection metadata - (aggregate fields are None for the legacy raw-array shape). + List[MCPServerConfig]: parsed MCP server configurations. """ config_data = await response.json(content_type=None) server_elements: Optional[List[object]] = None - all_connections_url: Optional[str] = None - missing_connections_url: Optional[str] = None - connectivity_status: Optional[str] = None if isinstance(config_data, list): - # Raw array format (legacy V1 gateway returns bare array, no aggregate). + # Raw array format (legacy V1 gateway returns bare array). self._logger.debug("Gateway returned raw array response") server_elements = config_data elif isinstance(config_data, dict) and isinstance(config_data.get("mcpServers"), list): - # Wrapped format: {"mcpServers": [...], aggregate connection fields} + # Wrapped format: {"mcpServers": [...], ...} self._logger.debug("Gateway returned wrapped mcpServers response") server_elements = config_data["mcpServers"] - - all_raw = config_data.get("allConnectionsUrl") - all_connections_url = str(all_raw) if all_raw is not None else None - - missing_raw = config_data.get("missingConnectionsUrl") - missing_connections_url = str(missing_raw) if missing_raw is not None else None - - status_raw = config_data.get("connectivityStatus") - connectivity_status = str(status_raw) if status_raw is not None else None else: self._logger.warning( 'Unexpected gateway response format: expected a list or {"mcpServers": [...]}' ) - return McpDiscoveryResult(servers=[]) + return [] mcp_servers: List[MCPServerConfig] = [] for server_element in server_elements: @@ -747,12 +690,7 @@ async def _parse_gateway_response(self, response: aiohttp.ClientResponse) -> Mcp if server_config is not None: mcp_servers.append(server_config) - return McpDiscoveryResult( - servers=mcp_servers, - all_connections_url=all_connections_url, - missing_connections_url=missing_connections_url, - connectivity_status=connectivity_status, - ) + return mcp_servers # -------------------------------------------------------------------------- # CONFIGURATION PARSING HELPERS diff --git a/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py b/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py index d67b0577..c9f04005 100644 --- a/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py +++ b/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py @@ -64,6 +64,9 @@ def mock_mcp_server_config(self): config.url = "https://test-mcp-server.example.com/api" config.headers = None # per-audience headers attached by list_tool_servers config.audience = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" # V2: GUID audience + config.connectivity_status = None # legacy / Ready — exercise the real-tool branch + config.missing_connections_url = None + config.all_connections_url = None return config @pytest.fixture @@ -493,6 +496,8 @@ async def test_full_client_lifecycle_single_server( mock_server_config.mcp_server_unique_name = "test-server-unique" mock_server_config.url = "https://test.example.com/api" mock_server_config.headers = None # per-audience headers attached by list_tool_servers + mock_server_config.connectivity_status = None # legacy / Ready + mock_server_config.missing_connections_url = None mock_http_client_instance = MagicMock() @@ -566,18 +571,24 @@ async def test_full_client_lifecycle_multiple_servers( mock_server_config1.mcp_server_unique_name = "server-1-unique" mock_server_config1.url = "https://server1.example.com/api" mock_server_config1.headers = None # per-audience headers attached by list_tool_servers + mock_server_config1.connectivity_status = None + mock_server_config1.missing_connections_url = None mock_server_config2 = Mock() mock_server_config2.mcp_server_name = "server-2" mock_server_config2.mcp_server_unique_name = "server-2-unique" mock_server_config2.url = "https://server2.example.com/api" mock_server_config2.headers = None + mock_server_config2.connectivity_status = None + mock_server_config2.missing_connections_url = None mock_server_config3 = Mock() mock_server_config3.mcp_server_name = "server-3" mock_server_config3.mcp_server_unique_name = "server-3-unique" mock_server_config3.url = "https://server3.example.com/api" mock_server_config3.headers = None + mock_server_config3.connectivity_status = None + mock_server_config3.missing_connections_url = None # Create unique mock clients for each server mock_clients = [MagicMock() for _ in range(3)] @@ -677,6 +688,8 @@ async def test_cleanup_called_twice_after_creating_clients( mock_server_config.mcp_server_unique_name = "test-server-unique" mock_server_config.url = "https://test.example.com/api" mock_server_config.headers = None # per-audience headers attached by list_tool_servers + mock_server_config.connectivity_status = None # legacy / Ready + mock_server_config.missing_connections_url = None mock_http_client_instance = MagicMock() @@ -729,6 +742,226 @@ async def test_cleanup_called_twice_after_creating_clients( assert len(service._http_clients) == 0 +class TestPendingServerGate: + """Tests for the connection-readiness gate in ``add_tool_servers_to_agent``. + + When the gateway flags any MCP server's downstream connections as + ``Pending``, ``add_tool_servers_to_agent`` must raise + ``McpConnectionsRequiredError`` (carrying ``all_connections_url``) *before* + building the agent, so the developer's turn handler can prompt the user to + set up connections. Raising at registration time — rather than from inside a + tool call — ensures the exception (and the setup URL) reaches the turn + handler, because Agent Framework's tool-call loop swallows exceptions raised + inside a ``FunctionTool``. When every server is Ready, the agent is built + normally with real ``MCPStreamableHTTPTool`` instances. + """ + + @pytest.fixture + def service(self): + return McpToolRegistrationService() + + @pytest.fixture + def mock_chat_client(self): + return Mock() + + @pytest.fixture + def mock_turn_context(self): + ctx = Mock() + ctx.activity = Mock() + ctx.activity.conversation = Mock() + ctx.activity.conversation.id = "conv-1" + ctx.activity.id = "msg-1" + return ctx + + @pytest.fixture + def mock_auth(self): + auth = Mock() + auth.exchange_token = AsyncMock() + auth.exchange_token.return_value = Mock(token="discovery-token") + return auth + + @staticmethod + def _config(name, status, all_url=None): + c = Mock() + c.mcp_server_name = name + c.mcp_server_unique_name = name + c.url = f"https://gw.example/{name}" + c.headers = None + c.connectivity_status = status + # The gate surfaces all_connections_url (view/manage all connectors + # required by the server, including ones already set up), not the + # narrower missing_connections_url. + c.all_connections_url = all_url + c.missing_connections_url = None + return c + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_pending_server_raises_before_building_agent( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """A Pending server raises McpConnectionsRequiredError; no agent is built.""" + from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpConnectionsRequiredError, + ) + + pending = self._config("mcp_Salesforce", "Pending", "https://make.example/all/salesforce") + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[pending], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.httpx.AsyncClient" + ) as mock_httpx_client, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.MCPStreamableHTTPTool" + ) as mock_mcp_tool, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + with pytest.raises(McpConnectionsRequiredError) as exc_info: + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # The gate fires before any tool wiring or agent construction. + mock_httpx_client.assert_not_called() + mock_mcp_tool.assert_not_called() + mock_raw_agent.assert_not_called() + # The exception surfaces the setup URL, status, and server name. + assert exc_info.value.all_connections_url == "https://make.example/all/salesforce" + assert exc_info.value.connectivity_status == "Pending" + assert exc_info.value.server_names == ["mcp_Salesforce"] + assert "https://make.example/all/salesforce" in str(exc_info.value) + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_mixed_ready_and_pending_raises( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """Any Pending server blocks the whole turn, even alongside Ready servers.""" + from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpConnectionsRequiredError, + ) + + ready = self._config("mcp_Mail", "Ready") + pending = self._config("mcp_Salesforce", "Pending", "https://make.example/all/sf") + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[ready, pending], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + with pytest.raises(McpConnectionsRequiredError) as exc_info: + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + mock_raw_agent.assert_not_called() + assert exc_info.value.server_names == ["mcp_Salesforce"] + assert exc_info.value.all_connections_url == "https://make.example/all/sf" + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_all_ready_builds_agent_with_real_tools( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """All-Ready (and legacy None) servers → real MCPStreamableHTTPTool; agent built.""" + ready = self._config("mcp_Mail", "Ready") + legacy = self._config("mcp_Files", None) # legacy / no status + captured_tools = [] + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[ready, legacy], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.httpx.AsyncClient" + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.MCPStreamableHTTPTool" + ) as mock_mcp_tool, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # Both servers wired as real MCP tools; agent constructed. + assert mock_mcp_tool.call_count == 2 + mock_raw_agent.assert_called_once() + assert len(captured_tools) == 2 + + +class TestExtensionExceptionExport: + """The connection-gating exception is owned and exported by this extension.""" + + def test_exception_importable_from_package_root(self): + from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpConnectionsRequiredError, + ) + + assert issubclass(McpConnectionsRequiredError, Exception) + + def test_exception_exposes_payload(self): + from microsoft_agents_a365.tooling.extensions.agentframework import ( + McpConnectionsRequiredError, + ) + + err = McpConnectionsRequiredError( + all_connections_url="https://make.example/all", + connectivity_status="Pending", + server_names=["mcp_Salesforce", "mcp_Zendesk"], + ) + assert err.all_connections_url == "https://make.example/all" + assert err.connectivity_status == "Pending" + assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] + message = str(err) + assert "mcp_Salesforce" in message + assert "https://make.example/all" in message + + class TestMcpHttpClientTimeoutConstant: """Tests for the MCP_HTTP_CLIENT_TIMEOUT_SECONDS constant.""" diff --git a/tests/tooling/test_mcp_connections_required_error.py b/tests/tooling/test_mcp_connections_required_error.py deleted file mode 100644 index 06e6c6ac..00000000 --- a/tests/tooling/test_mcp_connections_required_error.py +++ /dev/null @@ -1,32 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -"""Unit tests for McpConnectionsRequiredError.""" - -from microsoft_agents_a365.tooling import McpConnectionsRequiredError - - -def test_exception_exposes_payload(): - err = McpConnectionsRequiredError( - missing_connections_url="https://make.example/missing", - connectivity_status="Pending", - server_names=["mcp_Salesforce", "mcp_Zendesk"], - ) - assert err.missing_connections_url == "https://make.example/missing" - assert err.connectivity_status == "Pending" - assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] - - -def test_exception_message_is_actionable(): - err = McpConnectionsRequiredError( - missing_connections_url="https://make.example/missing", - connectivity_status="Pending", - server_names=["mcp_Salesforce"], - ) - message = str(err) - assert "mcp_Salesforce" in message - assert "https://make.example/missing" in message - - -def test_exception_is_exception_subclass(): - assert issubclass(McpConnectionsRequiredError, Exception) diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 0386bc1a..1949b1ee 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -8,7 +8,6 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from microsoft_agents_a365.tooling import McpConnectionsRequiredError from microsoft_agents_a365.tooling.models import MCPServerConfig from microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service import ( McpToolServerConfigurationService, @@ -386,8 +385,8 @@ async def test_legacy_prod_path_ok_for_v1_only_servers(self, service): assert servers[0].mcp_server_name == "V1Server" @pytest.mark.asyncio - async def test_parse_gateway_response_captures_aggregate(self, service): - """Response-level connection metadata is captured into McpDiscoveryResult.""" + async def test_parse_gateway_response_wrapped_returns_servers(self, service): + """Wrapped responses return a List[MCPServerConfig] with per-server metadata.""" payload = { "mcpServers": [ { @@ -395,6 +394,8 @@ async def test_parse_gateway_response_captures_aggregate(self, service): "mcpServerUniqueName": "mcp_Salesforce", "url": "https://gw.example/agents/v2/servers/mcp_Salesforce", "connectivityStatus": "Pending", + "allConnectionsUrl": "https://make.example/all/salesforce", + "missingConnectionsUrl": "https://make.example/missing/salesforce", } ], "allConnectionsUrl": "https://make.example/all", @@ -404,17 +405,18 @@ async def test_parse_gateway_response_captures_aggregate(self, service): mock_response = MagicMock() mock_response.json = AsyncMock(return_value=payload) - result = await service._parse_gateway_response(mock_response) + servers = await service._parse_gateway_response(mock_response) - assert len(result.servers) == 1 - assert result.servers[0].mcp_server_name == "mcp_Salesforce" - assert result.all_connections_url == "https://make.example/all" - assert result.missing_connections_url == "https://make.example/missing" - assert result.connectivity_status == "Pending" + assert isinstance(servers, list) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "mcp_Salesforce" + assert servers[0].connectivity_status == "Pending" + assert servers[0].all_connections_url == "https://make.example/all/salesforce" + assert servers[0].missing_connections_url == "https://make.example/missing/salesforce" @pytest.mark.asyncio - async def test_parse_gateway_response_raw_array_has_no_aggregate(self, service): - """Legacy raw-array responses produce a result with aggregate fields None.""" + async def test_parse_gateway_response_raw_array_returns_servers(self, service): + """Legacy raw-array responses return a list; per-server metadata is None.""" payload = [ { "mcpServerName": "V1Server", @@ -425,12 +427,13 @@ async def test_parse_gateway_response_raw_array_has_no_aggregate(self, service): mock_response = MagicMock() mock_response.json = AsyncMock(return_value=payload) - result = await service._parse_gateway_response(mock_response) + servers = await service._parse_gateway_response(mock_response) - assert len(result.servers) == 1 - assert result.all_connections_url is None - assert result.missing_connections_url is None - assert result.connectivity_status is None + assert isinstance(servers, list) + assert len(servers) == 1 + assert servers[0].connectivity_status is None + assert servers[0].all_connections_url is None + assert servers[0].missing_connections_url is None class TestResolveTokenScopeForServer: @@ -991,7 +994,9 @@ def test_skips_empty_blueprint_id(self, service, create_test_jwt): class TestConnectionGating: - """Tests for the connectivityStatus connection-readiness gate.""" + """Core no longer gates on connectivityStatus — it parses the per-server + connection metadata onto each MCPServerConfig and returns the servers + unchanged. Gating is the responsibility of the framework extensions.""" @pytest.fixture def service(self): @@ -1015,7 +1020,16 @@ def _gateway_response(payload): @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) @pytest.mark.asyncio - async def test_gate_raises_when_pending(self, service): + async def test_pending_servers_returned_without_raising(self, service): + """Pending servers flow through ``list_tool_servers`` unchanged. + + Connection gating has been removed from the core service. It is now + the responsibility of framework-specific extensions (see e.g. + ``microsoft-agents-a365-tooling-extensions-agentframework``), which raise + ``McpConnectionsRequiredError`` before building the agent when a server is + Pending. Core simply returns the configs with their per-server connection + metadata intact. + """ payload = { "mcpServers": [ { @@ -1023,6 +1037,7 @@ async def test_gate_raises_when_pending(self, service): "mcpServerUniqueName": "mcp_Salesforce", "url": "https://gw.example/mcp_Salesforce", "connectivityStatus": "Pending", + "missingConnectionsUrl": "https://make.example/missing/salesforce", } ], "allConnectionsUrl": "https://make.example/all", @@ -1030,14 +1045,13 @@ async def test_gate_raises_when_pending(self, service): "connectivityStatus": "Pending", } with self._gateway_response(payload): - with pytest.raises(McpConnectionsRequiredError) as exc_info: - await service.list_tool_servers( - agentic_app_id="test-app-id", auth_token="test-token" - ) - err = exc_info.value - assert err.connectivity_status == "Pending" - assert err.missing_connections_url == "https://make.example/missing" - assert "mcp_Salesforce" in err.server_names + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "mcp_Salesforce" + assert servers[0].connectivity_status == "Pending" + assert servers[0].missing_connections_url == "https://make.example/missing/salesforce" @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) @pytest.mark.asyncio @@ -1081,9 +1095,11 @@ async def test_gate_passes_for_legacy_raw_array(self, service): @patch.dict(os.environ, {"ENVIRONMENT": "Production"}) @pytest.mark.asyncio - async def test_gate_raises_when_aggregate_pending_but_no_server_flagged(self, service): - from microsoft_agents_a365.tooling import McpConnectionsRequiredError - + async def test_aggregate_pending_returns_servers_without_raising(self, service): + """Even when the response-level aggregate is Pending, ``list_tool_servers`` + returns the servers without raising — core does not act on the aggregate + (or per-server) connection metadata. + """ payload = { "mcpServers": [ { @@ -1098,15 +1114,11 @@ async def test_gate_raises_when_aggregate_pending_but_no_server_flagged(self, se "connectivityStatus": "Pending", } with self._gateway_response(payload): - with pytest.raises(McpConnectionsRequiredError) as exc_info: - await service.list_tool_servers( - agentic_app_id="test-app-id", auth_token="test-token" - ) - err = exc_info.value - assert err.connectivity_status == "Pending" - assert err.missing_connections_url == "https://make.example/missing" - assert err.server_names == [] - assert "(unknown)" in str(err) + servers = await service.list_tool_servers( + agentic_app_id="test-app-id", auth_token="test-token" + ) + assert len(servers) == 1 + assert servers[0].mcp_server_name == "mcp_Salesforce" @patch.object(McpToolServerConfigurationService, "_load_servers_from_manifest") @patch.dict(os.environ, {"ENVIRONMENT": "Development"}) From 867d24bd83bd6aefa4aeefad84a009e4e3743758 Mon Sep 17 00:00:00 2001 From: Grant Harris <96964444+gwharris7@users.noreply.github.com> Date: Mon, 15 Jun 2026 16:46:59 -0700 Subject: [PATCH 15/15] Make MCP connection gating non-blocking per server Switch the agentframework extension from blocking gating (raise McpConnectionsRequiredError before building the agent when any server is Pending) to non-blocking per-server gating: - Ready servers (and legacy sources with no connectivityStatus) are wired as live MCPStreamableHTTPTool instances, unchanged. - A Pending server is registered as a single placeholder FunctionTool named after the server (max_invocations=1). When the model invokes it, the placeholder RETURNS a static setup message including missing_connections_url (fallback all_connections_url) for the model to relay. The agent is always built and Ready tools stay usable; the user is only prompted about connections if their request actually needs the Pending server. The placeholder returns the message rather than raising it: Agent Framework swallows exceptions raised inside a tool and converts UserInputRequiredException into tool-result content, so neither reaches the turn handler. Returning the text is the only way to surface the URL through the model. McpConnectionsRequiredError stays exported for developers who want custom blocking gating; it now carries missing_connections_url, connectivity_status, server_names, and shares the new format_mcp_connections_required_message helper with the placeholder so the message wording has a single source of truth. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../docs/design.md | 86 ++++---- .../extensions/agentframework/exceptions.py | 83 +++++--- .../services/mcp_tool_registration_service.py | 134 +++++++----- .../test_mcp_tool_registration_service.py | 196 ++++++++++++------ .../tooling/test_mcp_server_configuration.py | 7 +- 5 files changed, 328 insertions(+), 178 deletions(-) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md index 7851236d..a9b31e5f 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/docs/design.md @@ -41,8 +41,7 @@ McpToolRegistrationService.add_tool_servers_to_agent() │ ├── Resolve agent identity ├── Exchange token for MCP scope - ├── Gate: raise McpConnectionsRequiredError if any server is Pending - ├── Create MCPStreamableHTTPTool for each (Ready) server + ├── Gate (per server, non-blocking): Ready → MCPStreamableHTTPTool; Pending → placeholder tool └── Create ChatAgent with all tools │ ▼ @@ -77,50 +76,65 @@ mcp_tool = MCPStreamableHTTPTool( ) ``` -### Connection-readiness gating - -MCP server discovery runs every turn. The gateway reports each server's -`connectivityStatus` (`"Ready"` or `"Pending"`) along with an `allConnectionsUrl` the user -can visit to view and manage the connectors required by that server. When **any** discovered -server is `Pending`, `add_tool_servers_to_agent` raises `McpConnectionsRequiredError` -**before** building the agent, so the developer's turn handler can prompt the user to set up -connections and return. A later turn re-runs discovery and proceeds automatically once the -connections are in place. - -The gate raises at agent-construction time — not from inside a tool call — on purpose. Agent -Framework's tool-call loop catches exceptions raised inside a `FunctionTool` and reflects them -to the model as an opaque error string (`"Error: Function failed."` by default), which would -drop the setup URL before it reached the user. Raising during construction lets the typed -exception propagate intact to the turn handler. +### Connection-readiness gating (non-blocking, per server) + +MCP server discovery runs every turn. The gateway reports each server's `connectivityStatus` +(`"Ready"` or `"Pending"`) along with a `missingConnectionsUrl` the user can visit to set up the +connection(s) that server needs. Gating is **per server and non-blocking**: + +- A **Ready** server (or a legacy source with no `connectivityStatus`) is wired as a live + `MCPStreamableHTTPTool`, exactly as before. +- A **Pending** server is wired as a single **placeholder tool** named after the server. The agent + is still built and the Ready servers remain fully usable for the turn. Only if the model invokes + the placeholder — because the user's request actually needs that server — does it return a static + message (including `missingConnectionsUrl`) for the model to relay to the user. If the turn never + needs the Pending server, the user is never bothered. A later turn re-runs discovery and wires the + server for real once its connections are in place. + +The placeholder **returns** the message rather than raising it. Agent Framework's tool-call loop +catches exceptions raised inside a `FunctionTool` and reflects them to the model as an opaque error +string (`"Error: Function failed."` by default), and it converts `UserInputRequiredException` into +tool-result content rather than propagating it — so returning the text is the only way to surface +the setup URL through the model. Because surfacing flows through the model, exact verbatim delivery +is best-effort; the placeholder's description instructs the model to relay the message and URL +verbatim, and `max_invocations=1` stops the model from looping on it within a turn. + +> **Note:** A Pending server is represented by one placeholder named after the server (its real +> sub-tools are invisible until it connects). The model routes the user's intent to it by server +> name plus description — best-effort, not guaranteed. ```python from microsoft_agents_a365.tooling.extensions.agentframework import ( McpToolRegistrationService, - McpConnectionsRequiredError, ) service = McpToolRegistrationService() -try: - agent = await service.add_tool_servers_to_agent( - chat_client=chat_client, - agent_instructions="You are a helpful assistant.", - initial_tools=[], - auth=auth_context, - auth_handler_name="graph", - turn_context=turn_context, - ) -except McpConnectionsRequiredError as err: - await turn_context.send_activity( - f"Before I can help, please set up the required connections for " - f"{', '.join(err.server_names)}: {err.all_connections_url}" - ) - return # Skip running the model/tools this turn. +# Non-blocking: Pending servers become placeholders, so no special handling is needed in the +# turn handler. The agent is always built and Ready tools always run. +agent = await service.add_tool_servers_to_agent( + chat_client=chat_client, + agent_instructions="You are a helpful assistant.", + initial_tools=[], + auth=auth_context, + auth_handler_name="graph", + turn_context=turn_context, +) +``` + +For developers who instead want **blocking** behavior (abort the whole turn until every server is +connected), the extension still exports `McpConnectionsRequiredError`. Inspect the discovered +servers yourself and raise it before building the agent: + +```python +from microsoft_agents_a365.tooling.extensions.agentframework import McpConnectionsRequiredError ``` -`McpConnectionsRequiredError` exposes `all_connections_url`, `connectivity_status`, and -`server_names`. It is owned and exported by this extension (`microsoft_agents_a365.tooling` -core only parses the per-server connection metadata; it never gates or raises). +`McpConnectionsRequiredError` exposes `missing_connections_url`, `connectivity_status`, and +`server_names`, and its message is built from the same `format_mcp_connections_required_message` +helper the placeholder uses. It is owned and exported by this extension +(`microsoft_agents_a365.tooling` core only parses the per-server connection metadata; it never +gates or raises). ### Chat History API diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py index 673078ad..83b9e501 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/exceptions.py @@ -1,44 +1,77 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -"""Exceptions raised by the Agent Framework MCP tooling extension.""" +"""Exceptions and message helpers for the Agent Framework MCP tooling extension.""" from typing import List, Optional +def format_mcp_connections_required_message( + *, + server_names: List[str], + connectivity_status: Optional[str], + missing_connections_url: Optional[str], +) -> str: + """Build the static, user-facing message shown when an MCP server needs connection setup. + + This single helper is the source of truth for the wording so that the message a Pending + server's placeholder tool returns to the model is identical to the message carried by + ``McpConnectionsRequiredError``. + + Args: + server_names: Names of the MCP server(s) whose downstream connections are not set up. + connectivity_status: The gateway-reported ``connectivityStatus`` (typically ``"Pending"``). + missing_connections_url: URL the user opens to set up the missing connection(s). May be + ``None`` when the gateway did not supply one. + + Returns: + A human-readable message suitable for relaying verbatim to the end user. + """ + servers_text = ", ".join(server_names) if server_names else "(unknown)" + message = ( + f"The tool(s) from MCP server(s) [{servers_text}] can't be used yet because the " + f"required data connection(s) aren't set up (connectivityStatus={connectivity_status})." + ) + if missing_connections_url: + message += f" Set up the missing connection(s) here: {missing_connections_url}" + else: + message += " Ask your administrator to set up the required connection(s)." + return message + + class McpConnectionsRequiredError(Exception): - """Raised when one or more configured MCP servers are not yet connection-ready. - - The tooling gateway reports a per-server ``connectivityStatus`` of ``"Pending"`` - when an MCP server has downstream connections the user has not yet established. - ``McpToolRegistrationService.add_tool_servers_to_agent`` discovers the servers - every turn and raises this error *before* building the agent when any server is - Pending, so the agent's turn handler can catch it, reply to the user with - ``all_connections_url``, and return without running the model/tools. A later turn - re-runs discovery and proceeds once the connections are in place. - - The error is raised at agent-construction time (not from inside a tool call) so it - propagates to the developer's turn handler intact — Agent Framework's tool-call loop - swallows exceptions raised inside a ``FunctionTool`` and reflects them to the model - as an opaque error string, which would drop the setup URL. - - ``all_connections_url`` lets the user view and manage the full set of connectors - required by the affected servers — including ones already set up — rather than being - scoped strictly to the ones that happen to be missing right now. + """Raised when an MCP server the user invoked is not yet connection-ready. + + The tooling gateway reports a per-server ``connectivityStatus`` of ``"Pending"`` when an MCP + server has downstream connections the user has not yet established. Discovery runs every turn. + + The agentframework extension gates **per server, non-blocking**: Ready servers are wired as + real tools and a Pending server is registered as a placeholder tool. Only when the model + actually invokes that placeholder (because the user's request needs the server) is the static + setup message — including ``missing_connections_url`` — surfaced; the rest of the turn runs + normally with the Ready tools. + + This exception is **not** raised by the non-blocking gate (Agent Framework swallows exceptions + raised inside a tool, and converts ``UserInputRequiredException`` into tool-result content, so + neither reaches the developer's turn handler). It is exported for developers who want to + implement their own *blocking* gating — e.g. inspect the discovered servers and raise this + before building the agent to abort the whole turn — and as the structured carrier of the same + message the placeholder returns. """ def __init__( self, - all_connections_url: Optional[str], + missing_connections_url: Optional[str], connectivity_status: Optional[str], server_names: List[str], ) -> None: - self.all_connections_url = all_connections_url + self.missing_connections_url = missing_connections_url self.connectivity_status = connectivity_status self.server_names = server_names - servers_text = ", ".join(server_names) if server_names else "(unknown)" super().__init__( - f"MCP servers [{servers_text}] require connection setup " - f"(connectivityStatus={connectivity_status}). " - f"Set up connections at: {all_connections_url}" + format_mcp_connections_required_message( + server_names=server_names, + connectivity_status=connectivity_status, + missing_connections_url=missing_connections_url, + ) ) diff --git a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py index 7f557885..0b97fa4c 100644 --- a/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py +++ b/libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py @@ -8,7 +8,13 @@ from datetime import datetime, timezone from typing import TYPE_CHECKING, List, Optional, Sequence -from agent_framework import RawAgent, Message, HistoryProvider, MCPStreamableHTTPTool +from agent_framework import ( + RawAgent, + Message, + HistoryProvider, + MCPStreamableHTTPTool, + FunctionTool, +) import httpx if TYPE_CHECKING: @@ -18,7 +24,7 @@ from microsoft_agents_a365.runtime import OperationResult from microsoft_agents_a365.runtime.utility import Utility -from microsoft_agents_a365.tooling.models import ChatHistoryMessage, ToolOptions +from microsoft_agents_a365.tooling.models import ChatHistoryMessage, MCPServerConfig, ToolOptions from microsoft_agents_a365.tooling.services.mcp_tool_server_configuration_service import ( McpToolServerConfigurationService, ) @@ -28,16 +34,19 @@ is_development_environment, ) -from ..exceptions import McpConnectionsRequiredError +from ..exceptions import format_mcp_connections_required_message # Default timeout for MCP server HTTP requests (in seconds) MCP_HTTP_CLIENT_TIMEOUT_SECONDS = 90.0 # Sentinel per-server status that means a server's downstream connections are -# already in place. Anything else (typically "Pending") triggers the -# connection-readiness gate, which raises McpConnectionsRequiredError before the -# agent is built so the turn handler can prompt the user to set up connections. +# already in place. Anything else (typically "Pending") means the server is not +# connection-ready: instead of wiring it as a real tool, the extension registers +# a placeholder tool that returns a static "set up your connections" message when +# the model invokes it. This gates per server without blocking the rest of the +# turn — Ready servers stay usable. None means the source predates the field +# (dev manifest / legacy gateway) and is treated as Ready. _CONNECTIVITY_READY = "Ready" @@ -131,54 +140,38 @@ async def add_tool_servers_to_agent( c.missing_connections_url, ) - # Connection-readiness gate. Discovery runs every turn; when the - # gateway flags any MCP server's downstream connection(s) as not yet - # set up (``connectivityStatus == "Pending"``), raise before building - # the agent so the developer's turn handler can catch the error, - # prompt the user with ``all_connections_url``, and return without - # running the model/tools. A later turn re-runs discovery and - # proceeds automatically once the connections are in place. - # - # The gate raises here — at agent-construction time — rather than - # from inside a tool call on purpose: Agent Framework's tool-call - # loop swallows exceptions raised inside a ``FunctionTool`` and - # reflects them to the model as an opaque error string, which would - # drop the setup URL before it ever reaches the user. - pending = [ - c - for c in server_configs - if c.connectivity_status is not None - and c.connectivity_status != _CONNECTIVITY_READY - ] - if pending: - pending_names = [c.mcp_server_name or c.mcp_server_unique_name for c in pending] - # Surface all_connections_url so the user can view and manage the - # full set of connectors required by the affected servers. Use the - # first pending server that provides one (they are environment-scoped - # and typically identical across servers). - all_connections_url = next( - (c.all_connections_url for c in pending if c.all_connections_url), - None, - ) - self._logger.info( - "MCP connection gate blocking turn: pending servers=%s, setup URL=%s", - pending_names, - all_connections_url, - ) - raise McpConnectionsRequiredError( - all_connections_url=all_connections_url, - connectivity_status=pending[0].connectivity_status, - server_names=pending_names, - ) - # Create the agent with all tools (initial + MCP tools) all_tools = list(initial_tools) - # Add each Ready server as an MCPStreamableHTTPTool instance. + # Connection-readiness gate (non-blocking, per server). Discovery runs + # every turn; the gateway flags each server's downstream connections via + # ``connectivityStatus``. A "Ready" (or legacy ``None``) server is wired + # as a real ``MCPStreamableHTTPTool``. A "Pending" server is instead + # registered as a placeholder tool that, when the model invokes it, + # returns a static "set up your connections" message (including the + # server's ``missing_connections_url``). This keeps the turn running with + # the Ready tools and only prompts the user about connection setup if the + # model actually needs the Pending server. A later turn re-runs discovery + # and wires the server for real once its connections are in place. for config in server_configs: # Use mcp_server_name if available (not None or empty), otherwise fall back to mcp_server_unique_name server_name = config.mcp_server_name or config.mcp_server_unique_name + if ( + config.connectivity_status is not None + and config.connectivity_status != _CONNECTIVITY_READY + ): + placeholder = self._build_pending_placeholder_tool(config) + all_tools.append(placeholder) + self._logger.info( + "MCP server '%s' is %s; registered connection-setup placeholder " + "instead of live tools (setup URL=%s)", + server_name, + config.connectivity_status, + config.missing_connections_url or config.all_connections_url, + ) + continue + try: # Merge base (non-auth) headers with per-server headers from list_tool_servers. # server.headers already contains the correct per-audience Authorization token. @@ -239,13 +232,54 @@ async def add_tool_servers_to_agent( self._logger.info(f"Agent created with {len(all_tools)} total tools") return agent - except McpConnectionsRequiredError: - # Connection-readiness gate — propagate intact to the turn handler. - raise except Exception as ex: self._logger.error(f"Failed to add tool servers to agent: {ex}") raise + def _build_pending_placeholder_tool(self, config: MCPServerConfig) -> FunctionTool: + """Build the placeholder tool registered in place of a not-yet-connected MCP server. + + The gateway reported this server's ``connectivityStatus`` as something other than + ``"Ready"`` (typically ``"Pending"``), meaning the user has not finished setting up the + downstream data connection(s) the server needs. The server's real sub-tools are not + available until those connections exist, so instead of wiring it as a live + ``MCPStreamableHTTPTool`` we expose a single stand-in tool named after the server. When + the model invokes it — because the user's request needs that server — the tool returns a + static message (including ``missing_connections_url``) that the model relays to the user. + + The message is **returned**, not raised: Agent Framework swallows exceptions raised inside + a tool (and converts ``UserInputRequiredException`` into tool-result content), so returning + the text is the only way to surface the URL through the model. ``max_invocations=1`` keeps + the model from calling the placeholder repeatedly within a turn. + + Args: + config: The discovered configuration for the Pending MCP server. + + Returns: + A ``FunctionTool`` standing in for the not-yet-connected server. + """ + server_name = config.mcp_server_name or config.mcp_server_unique_name + message = format_mcp_connections_required_message( + server_names=[server_name], + connectivity_status=config.connectivity_status, + missing_connections_url=config.missing_connections_url or config.all_connections_url, + ) + + def _connections_required() -> str: + return message + + return FunctionTool( + name=server_name, + description=( + f"Tools provided by the '{server_name}' MCP server. The required data " + "connection(s) for this server are not set up yet, so its tools cannot run. " + "Call this when the user's request needs this server, then relay the returned " + "message — including the setup URL — to the user verbatim." + ), + func=_connections_required, + max_invocations=1, + ) + def _convert_chat_messages_to_history( self, chat_messages: Sequence[Message], diff --git a/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py b/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py index c9f04005..99bc34c0 100644 --- a/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py +++ b/tests/tooling/extensions/agentframework/services/test_mcp_tool_registration_service.py @@ -743,17 +743,13 @@ async def test_cleanup_called_twice_after_creating_clients( class TestPendingServerGate: - """Tests for the connection-readiness gate in ``add_tool_servers_to_agent``. - - When the gateway flags any MCP server's downstream connections as - ``Pending``, ``add_tool_servers_to_agent`` must raise - ``McpConnectionsRequiredError`` (carrying ``all_connections_url``) *before* - building the agent, so the developer's turn handler can prompt the user to - set up connections. Raising at registration time — rather than from inside a - tool call — ensures the exception (and the setup URL) reaches the turn - handler, because Agent Framework's tool-call loop swallows exceptions raised - inside a ``FunctionTool``. When every server is Ready, the agent is built - normally with real ``MCPStreamableHTTPTool`` instances. + """Tests for the non-blocking, per-server connection gate in ``add_tool_servers_to_agent``. + + When the gateway flags an MCP server's downstream connections as ``Pending``, the service + registers a placeholder ``FunctionTool`` (named after the server) in place of a live + ``MCPStreamableHTTPTool``. The agent is still built and Ready servers stay usable; only if the + model invokes the placeholder does it return a static setup message (including + ``missing_connections_url``). The turn is never aborted. """ @pytest.fixture @@ -781,31 +777,29 @@ def mock_auth(self): return auth @staticmethod - def _config(name, status, all_url=None): + def _config(name, status, missing_url=None, all_url=None): c = Mock() c.mcp_server_name = name c.mcp_server_unique_name = name c.url = f"https://gw.example/{name}" c.headers = None c.connectivity_status = status - # The gate surfaces all_connections_url (view/manage all connectors - # required by the server, including ones already set up), not the - # narrower missing_connections_url. + c.missing_connections_url = missing_url c.all_connections_url = all_url - c.missing_connections_url = None return c @pytest.mark.asyncio @pytest.mark.unit - async def test_pending_server_raises_before_building_agent( + async def test_pending_server_registers_placeholder_and_builds_agent( self, service, mock_chat_client, mock_turn_context, mock_auth ): - """A Pending server raises McpConnectionsRequiredError; no agent is built.""" - from microsoft_agents_a365.tooling.extensions.agentframework import ( - McpConnectionsRequiredError, - ) + """A Pending server is wired as a placeholder tool; the agent is still built.""" + from agent_framework import FunctionTool - pending = self._config("mcp_Salesforce", "Pending", "https://make.example/all/salesforce") + pending = self._config( + "mcp_Salesforce", "Pending", missing_url="https://make.example/missing/salesforce" + ) + captured_tools = [] with ( patch.object( service._mcp_server_configuration_service, @@ -827,38 +821,87 @@ async def test_pending_server_raises_before_building_agent( return_value="test-agent-id", ), ): - with pytest.raises(McpConnectionsRequiredError) as exc_info: - await service.add_tool_servers_to_agent( - chat_client=mock_chat_client, - agent_instructions="Test", - initial_tools=[], - auth=mock_auth, - auth_handler_name="h", - turn_context=mock_turn_context, - auth_token="discovery-token", - ) - # The gate fires before any tool wiring or agent construction. + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # No live MCP tool / httpx client for the Pending server; agent still built. mock_httpx_client.assert_not_called() mock_mcp_tool.assert_not_called() - mock_raw_agent.assert_not_called() - # The exception surfaces the setup URL, status, and server name. - assert exc_info.value.all_connections_url == "https://make.example/all/salesforce" - assert exc_info.value.connectivity_status == "Pending" - assert exc_info.value.server_names == ["mcp_Salesforce"] - assert "https://make.example/all/salesforce" in str(exc_info.value) + mock_raw_agent.assert_called_once() + # Exactly one placeholder FunctionTool, named after the server. + placeholders = [t for t in captured_tools if isinstance(t, FunctionTool)] + assert len(placeholders) == 1 + placeholder = placeholders[0] + assert placeholder.name == "mcp_Salesforce" + # The placeholder is registered, not connected, so it isn't tracked for cleanup. + assert service._connected_servers == [] + # Invoking the placeholder returns the static message with the missing-connections URL. + message = placeholder.func() + assert "https://make.example/missing/salesforce" in message + assert "mcp_Salesforce" in message + # Capped so the model can't loop on it within a turn. + assert placeholder.max_invocations == 1 @pytest.mark.asyncio @pytest.mark.unit - async def test_mixed_ready_and_pending_raises( + async def test_placeholder_falls_back_to_all_connections_url( self, service, mock_chat_client, mock_turn_context, mock_auth ): - """Any Pending server blocks the whole turn, even alongside Ready servers.""" - from microsoft_agents_a365.tooling.extensions.agentframework import ( - McpConnectionsRequiredError, + """When the gateway omits missing_connections_url, the placeholder uses all_connections_url.""" + from agent_framework import FunctionTool + + pending = self._config( + "mcp_Zendesk", "Pending", missing_url=None, all_url="https://make.example/all/zendesk" ) + captured_tools = [] + with ( + patch.object( + service._mcp_server_configuration_service, + "list_tool_servers", + new_callable=AsyncMock, + return_value=[pending], + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" + ) as mock_raw_agent, + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.Utility.resolve_agent_identity", + return_value="test-agent-id", + ), + ): + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + placeholder = next(t for t in captured_tools if isinstance(t, FunctionTool)) + assert "https://make.example/all/zendesk" in placeholder.func() + + @pytest.mark.asyncio + @pytest.mark.unit + async def test_mixed_ready_and_pending_wires_both( + self, service, mock_chat_client, mock_turn_context, mock_auth + ): + """Ready servers become real tools; Pending servers become placeholders — turn proceeds.""" + from agent_framework import FunctionTool ready = self._config("mcp_Mail", "Ready") - pending = self._config("mcp_Salesforce", "Pending", "https://make.example/all/sf") + pending = self._config( + "mcp_Salesforce", "Pending", missing_url="https://make.example/missing/sf" + ) + captured_tools = [] with ( patch.object( service._mcp_server_configuration_service, @@ -866,6 +909,12 @@ async def test_mixed_ready_and_pending_raises( new_callable=AsyncMock, return_value=[ready, pending], ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.httpx.AsyncClient" + ), + patch( + "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.MCPStreamableHTTPTool" + ) as mock_mcp_tool, patch( "microsoft_agents_a365.tooling.extensions.agentframework.services.mcp_tool_registration_service.RawAgent" ) as mock_raw_agent, @@ -874,26 +923,32 @@ async def test_mixed_ready_and_pending_raises( return_value="test-agent-id", ), ): - with pytest.raises(McpConnectionsRequiredError) as exc_info: - await service.add_tool_servers_to_agent( - chat_client=mock_chat_client, - agent_instructions="Test", - initial_tools=[], - auth=mock_auth, - auth_handler_name="h", - turn_context=mock_turn_context, - auth_token="discovery-token", - ) - mock_raw_agent.assert_not_called() - assert exc_info.value.server_names == ["mcp_Salesforce"] - assert exc_info.value.all_connections_url == "https://make.example/all/sf" + mock_raw_agent.side_effect = lambda **kw: captured_tools.extend(kw["tools"]) or Mock() + await service.add_tool_servers_to_agent( + chat_client=mock_chat_client, + agent_instructions="Test", + initial_tools=[], + auth=mock_auth, + auth_handler_name="h", + turn_context=mock_turn_context, + auth_token="discovery-token", + ) + # Ready server wired once as a live MCP tool; agent built once. + assert mock_mcp_tool.call_count == 1 + mock_raw_agent.assert_called_once() + # One placeholder for the Pending server, and the live tool for the Ready one. + placeholders = [t for t in captured_tools if isinstance(t, FunctionTool)] + assert [p.name for p in placeholders] == ["mcp_Salesforce"] + assert len(captured_tools) == 2 @pytest.mark.asyncio @pytest.mark.unit async def test_all_ready_builds_agent_with_real_tools( self, service, mock_chat_client, mock_turn_context, mock_auth ): - """All-Ready (and legacy None) servers → real MCPStreamableHTTPTool; agent built.""" + """All-Ready (and legacy None) servers → real MCPStreamableHTTPTool; no placeholders.""" + from agent_framework import FunctionTool + ready = self._config("mcp_Mail", "Ready") legacy = self._config("mcp_Files", None) # legacy / no status captured_tools = [] @@ -928,14 +983,15 @@ async def test_all_ready_builds_agent_with_real_tools( turn_context=mock_turn_context, auth_token="discovery-token", ) - # Both servers wired as real MCP tools; agent constructed. + # Both servers wired as real MCP tools; agent constructed; no placeholders. assert mock_mcp_tool.call_count == 2 mock_raw_agent.assert_called_once() + assert [t for t in captured_tools if isinstance(t, FunctionTool)] == [] assert len(captured_tools) == 2 class TestExtensionExceptionExport: - """The connection-gating exception is owned and exported by this extension.""" + """The connection-gating exception/formatter are owned by this extension.""" def test_exception_importable_from_package_root(self): from microsoft_agents_a365.tooling.extensions.agentframework import ( @@ -950,16 +1006,30 @@ def test_exception_exposes_payload(self): ) err = McpConnectionsRequiredError( - all_connections_url="https://make.example/all", + missing_connections_url="https://make.example/missing", connectivity_status="Pending", server_names=["mcp_Salesforce", "mcp_Zendesk"], ) - assert err.all_connections_url == "https://make.example/all" + assert err.missing_connections_url == "https://make.example/missing" assert err.connectivity_status == "Pending" assert err.server_names == ["mcp_Salesforce", "mcp_Zendesk"] message = str(err) assert "mcp_Salesforce" in message - assert "https://make.example/all" in message + assert "https://make.example/missing" in message + + def test_formatter_handles_missing_url(self): + from microsoft_agents_a365.tooling.extensions.agentframework.exceptions import ( + format_mcp_connections_required_message, + ) + + message = format_mcp_connections_required_message( + server_names=["mcp_Salesforce"], + connectivity_status="Pending", + missing_connections_url=None, + ) + assert "mcp_Salesforce" in message + assert "http" not in message # no URL when none is supplied + assert "administrator" in message class TestMcpHttpClientTimeoutConstant: diff --git a/tests/tooling/test_mcp_server_configuration.py b/tests/tooling/test_mcp_server_configuration.py index 1949b1ee..60cad5dc 100644 --- a/tests/tooling/test_mcp_server_configuration.py +++ b/tests/tooling/test_mcp_server_configuration.py @@ -1025,10 +1025,9 @@ async def test_pending_servers_returned_without_raising(self, service): Connection gating has been removed from the core service. It is now the responsibility of framework-specific extensions (see e.g. - ``microsoft-agents-a365-tooling-extensions-agentframework``), which raise - ``McpConnectionsRequiredError`` before building the agent when a server is - Pending. Core simply returns the configs with their per-server connection - metadata intact. + ``microsoft-agents-a365-tooling-extensions-agentframework``), which register + a non-blocking placeholder tool for each Pending server. Core simply returns + the configs with their per-server connection metadata intact. """ payload = { "mcpServers": [