From 67168d3311e3e12adf9a063200acacdbe0bf08aa Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Wed, 23 Sep 2026 16:13:17 -0700 Subject: [PATCH] flightcheck: add canonical DA connection-reference reader + contract test (AB#7852506, AB#7852495, AB#7852511) Extract the shared minimalBots components connection-reference reader into checks/_da_connection_refs.py as the single source that DV-CONN-001 (active agent), ENV-004 (environment-wide, de-duped by logical name) and the Workday shared-parameter checks (WD-ENV-001/WD-REST-001) all read through, so the per-check re-point PRs stop adding divergent copies of it. The module is the superset of PR #317's reader plus a public read_all_agents_connection_references() that de-dupes by connection-reference logical name for ENV-004. Brings the validated/documented mock builders the checks need and a direct pure-logic contract test (18 cases) covering normalization, None-gating (SKIP), fail-loudly ValueError, JSON-string sharedConnectionParameters parsing, and the Workday shared-parameter sweep. Foundation for the 6 DA re-point PRs to rebase onto (single shared reader, no add/add collision). Full flightcheck suite: 1142 passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e25cf992-09c9-49b8-9971-2a19cb2fcb05 --- .../flightcheck/checks/_da_connection_refs.py | 245 ++++++++++++++++++ .../checks/test_da_connection_refs.py | 205 +++++++++++++++ tests/mocks/agentbuilder_connectivity.py | 112 ++++++++ 3 files changed, 562 insertions(+) create mode 100644 solutions/ess-maker-skills/scripts/flightcheck/checks/_da_connection_refs.py create mode 100644 tests/flightcheck/checks/test_da_connection_refs.py diff --git a/solutions/ess-maker-skills/scripts/flightcheck/checks/_da_connection_refs.py b/solutions/ess-maker-skills/scripts/flightcheck/checks/_da_connection_refs.py new file mode 100644 index 000000000..3e40167da --- /dev/null +++ b/solutions/ess-maker-skills/scripts/flightcheck/checks/_da_connection_refs.py @@ -0,0 +1,245 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Canonical reader for Declarative Agent connection references (minimalBots +components API), shared so the DA connection checks cannot drift apart. + +Consumers: + * ``DV-CONN-001`` (checks/workday_extension.py) -> the single active agent's + Workday SOAP reference, via ``read_active_agent_connection_references``. + * ``ENV-004`` (checks/environment.py) -> every configured agent's references, + environment-wide and de-duped by logical name, via + ``read_all_agents_connection_references``. + * The Workday shared-parameter checks (checks/workday.py) -> per-agent Workday + ``sharedConnectionParameters`` via ``workday_shared_connection_parameters``. + +Read shape: ``POST .../components`` -> ``connectionReferenceChanges`` (cassette +``agentbuilder_readiness.yaml``, the same endpoint + shape the shipped native +``DA-CONN-001`` check consumes). + +Fail-loudly contract: + * a missing ``connectionReferenceChanges`` key means genuine absence -> ``[]``; + * a present-but-malformed shape raises ``ValueError`` so the owning check + degrades to a WARNING rather than reporting a confident but wrong verdict; + * a read that cannot be attempted at all (no AgentBuilder client, or no + configured agent botId) returns ``None`` so the caller SKIPs. +""" + +from __future__ import annotations + +import json +from typing import Any + + +WORKDAY_SOAP_CONNECTOR_SUFFIX = "/apis/shared_workdaysoap" + + +def agent_bot_ids(config: dict[str, Any]) -> list[str]: + """Return configured bot IDs from multi-agent and single-agent config.""" + bot_ids: list[str] = [] + for agent in config.get("agents", []) or []: + bid = (agent or {}).get("botId") + if isinstance(bid, str) and bid.strip(): + bot_ids.append(bid.strip()) + single = (config.get("agent") or {}).get("botId") + if isinstance(single, str) and single.strip(): + bot_ids.append(single.strip()) + + seen: set[str] = set() + ordered: list[str] = [] + for bot_id in bot_ids: + folded = bot_id.casefold() + if folded not in seen: + seen.add(folded) + ordered.append(bot_id) + return ordered + + +def _bot_connection_references(client, bot_id: str) -> list[dict[str, Any]]: + """Fetch + normalize one agent's connection references from the minimalBots + components API. + + Raises ``ValueError`` for a malformed ``connectionReferenceChanges`` shape + so the owning check reports a WARNING instead of overclaiming. + """ + changeset = client.fetch_components(bot_id) or {} + changes = changeset.get("connectionReferenceChanges") + if changes is None: + return [] + if not isinstance(changes, list): + raise ValueError( + "Component fetch returned invalid connectionReferenceChanges." + ) + + refs: list[dict[str, Any]] = [] + for change in changes: + item = ( + change.get("connectionReference") + if isinstance(change, dict) + else None + ) + if not isinstance(item, dict): + continue + refs.append( + { + "botid": bot_id, + "connectionreferencelogicalname": item.get( + "connectionReferenceLogicalName" + ), + "connectorid": item.get("connectorId"), + "connectionid": item.get("connectionId"), + "sharedconnectionparameters": item.get( + "sharedConnectionParameters" + ), + } + ) + return refs + + +def read_active_agent_connection_references(runner) -> list[dict[str, Any]] | None: + """The single active agent's DA connection references (config + ``agent.botId``), or ``None`` when the AgentBuilder client or the + active-agent botId is unavailable. + + Used by ``DV-CONN-001`` (checks/workday_extension.py), which validates the + Workday SOAP connection reference on the agent under check. Scoping this to + the active agent — not every configured agent — keeps the check from + reporting on a Workday reference that belongs to a different agent. + Raises ``ValueError`` for malformed components payloads. + """ + client = getattr(runner, "agentbuilder", None) + config = getattr(runner, "config", None) or {} + agent_id = (config.get("agent") or {}).get("botId") + if client is None or not agent_id: + return None + return _bot_connection_references(client, agent_id) + + +def _all_agents_connection_references(runner) -> list[dict[str, Any]] | None: + """Every configured agent's DA connection references (multi-agent and + single-agent config), preserving per-agent rows, or ``None`` when the + AgentBuilder client is unavailable or no agent botId is configured. + + Used by the Workday shared-parameter sweep, which must inspect each + configured agent's own Workday reference rather than only the active one. + Raises ``ValueError`` for malformed components payloads. + """ + client = getattr(runner, "agentbuilder", None) + config = getattr(runner, "config", None) or {} + bot_ids = agent_bot_ids(config) + if client is None or not bot_ids: + return None + + refs: list[dict[str, Any]] = [] + for bot_id in bot_ids: + refs.extend(_bot_connection_references(client, bot_id)) + return refs + + +def read_all_agents_connection_references( + runner, +) -> list[dict[str, Any]] | None: + """Every configured agent's references, de-duped by connection-reference + logical name (first occurrence wins, order preserved), or ``None`` when the + AgentBuilder client is unavailable or no agent botId is configured. + + Used by ``ENV-004`` (checks/environment.py), which is environment-wide + across every agent under check and reports one row per distinct logical + name. The per-agent (non-de-duped) view is ``_all_agents_connection_references``, + which the Workday shared-parameter sweep uses instead. + """ + refs = _all_agents_connection_references(runner) + if refs is None: + return None + seen: set[str] = set() + deduped: list[dict[str, Any]] = [] + for ref in refs: + key = (ref.get("connectionreferencelogicalname") or "").casefold() + if key and key in seen: + continue + if key: + seen.add(key) + deduped.append(ref) + return deduped + + +def _shared_parameter_value(raw_value: Any) -> str: + if isinstance(raw_value, dict): + raw_value = raw_value.get("value") + if raw_value is None: + return "" + return str(raw_value).strip() + + +def shared_connection_parameter_values(ref: dict[str, Any]) -> dict[str, str]: + """Return ``sharedConnectionParameters.values`` as a string map. + + A present-but-malformed shape raises ``ValueError`` because the components + payload no longer matches the validated contract. + """ + params = ref.get("sharedconnectionparameters") + if params is None: + return {} + # Live AgentBuilder returns sharedConnectionParameters as a JSON string, + # not a nested object (observed on a live connection reference), so parse + # the string before validating the shape. + if isinstance(params, str): + text = params.strip() + if not text: + return {} + try: + params = json.loads(text) + except ValueError as exc: + raise ValueError( + "Component fetch returned invalid sharedConnectionParameters." + ) from exc + if not isinstance(params, dict): + raise ValueError( + "Component fetch returned invalid sharedConnectionParameters." + ) + raw_values = params.get("values") + if raw_values is None: + return {} + if not isinstance(raw_values, dict): + raise ValueError( + "Component fetch returned invalid sharedConnectionParameters.values." + ) + return { + str(key): value + for key, raw_value in raw_values.items() + if isinstance(key, str) + if (value := _shared_parameter_value(raw_value)) + } + + +def workday_shared_connection_parameters( + runner, +) -> tuple[dict[str, str] | None, str]: + """Return Workday ``sharedConnectionParameters.values`` from components. + + ``values is None`` means the check could not run because AgentBuilder or a + botId is unavailable. ``values == {}`` means the check ran and observed a + missing Workday reference or missing shared parameters. + """ + refs = _all_agents_connection_references(runner) + if refs is None: + return None, ( + "AgentBuilder client or a configured agent botId not available" + ) + + found_workday_ref = False + for ref in refs: + connector_id = str(ref.get("connectorid") or "").casefold().rstrip("/") + if connector_id.endswith(WORKDAY_SOAP_CONNECTOR_SUFFIX): + found_workday_ref = True + values = shared_connection_parameter_values(ref) + if values: + return values, "" + + if found_workday_ref: + return {}, ( + "Workday connection reference is missing " + "sharedConnectionParameters.values" + ) + + return {}, "Workday connection reference was not found" diff --git a/tests/flightcheck/checks/test_da_connection_refs.py b/tests/flightcheck/checks/test_da_connection_refs.py new file mode 100644 index 000000000..e3b4c42e7 --- /dev/null +++ b/tests/flightcheck/checks/test_da_connection_refs.py @@ -0,0 +1,205 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Contract tests for the canonical Declarative Agent connection-reference +reader (``checks/_da_connection_refs.py``), the single shared module that +``DV-CONN-001`` (active agent), ``ENV-004`` (environment-wide, de-duped) and the +Workday shared-parameter checks all read through. + +These are pure-logic tests: a duck-typed fake AgentBuilder client returns +minimalBots components payloads built from +``tests.mocks.agentbuilder_connectivity`` (``MOCK_STATUS == "validated"``), so +no network replay or cassette is needed (same inline-fake approach as +``test_agent_handoff.py``). Every component shape traces to the validated +``components()`` builder or its documented ``sharedConnectionParameters`` +variant; none is invented here. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from flightcheck.checks import _da_connection_refs as reader +from tests.mocks import agentbuilder_connectivity as ab + + +class _FakeClient: + """AgentBuilder stand-in: ``fetch_components(bot_id)`` returns the payload + registered for that bot id (empty ``{}`` when none is registered).""" + + def __init__(self, payload_by_bot: dict[str, dict[str, Any]]): + self._payload_by_bot = payload_by_bot + + def fetch_components(self, bot_id: str) -> dict[str, Any]: + return self._payload_by_bot.get(bot_id, {}) + + +class _FakeRunner: + def __init__(self, client: _FakeClient | None, config: dict[str, Any]): + self.agentbuilder = client + self.config = config + + +# -------------------------------------------------------------------------- +# agent_bot_ids +# -------------------------------------------------------------------------- + +def test_agent_bot_ids_unions_multi_and_single_and_dedups_casefold(): + config = { + "agents": [{"botId": "Agent-A"}, {"botId": "Agent-B"}, {"botId": "agent-a"}], + "agent": {"botId": "Agent-B"}, + } + # agent-a folds onto Agent-A (dup); single Agent-B folds onto Agent-B (dup). + assert reader.agent_bot_ids(config) == ["Agent-A", "Agent-B"] + + +def test_agent_bot_ids_single_only(): + assert reader.agent_bot_ids({"agent": {"botId": "SOLO"}}) == ["SOLO"] + + +def test_agent_bot_ids_ignores_blank_and_non_string(): + config = {"agents": [{"botId": ""}, {"botId": None}, {"botId": " X "}]} + assert reader.agent_bot_ids(config) == ["X"] + + +# -------------------------------------------------------------------------- +# read_active_agent_connection_references (DV-CONN-001 surface) +# -------------------------------------------------------------------------- + +def test_read_active_none_when_no_client(): + runner = _FakeRunner(None, {"agent": {"botId": "BOT"}}) + assert reader.read_active_agent_connection_references(runner) is None + + +def test_read_active_none_when_no_active_bot_id(): + # Only multi-agent config; no config["agent"].botId -> active read SKIPs. + runner = _FakeRunner(_FakeClient({}), {"agents": [{"botId": "BOT"}]}) + assert reader.read_active_agent_connection_references(runner) is None + + +def test_read_active_normalizes_workday_row(): + payload = ab.components_with_references( + references=[ab.workday_connection_reference(connection_id="wd-conn-1")] + ) + runner = _FakeRunner(_FakeClient({"BOT": payload}), {"agent": {"botId": "BOT"}}) + rows = reader.read_active_agent_connection_references(runner) + assert len(rows) == 1 + row = rows[0] + assert row["botid"] == "BOT" + assert row["connectorid"].endswith("/apis/shared_workdaysoap") + assert row["connectionid"] == "wd-conn-1" + + +def test_read_active_missing_change_set_is_empty_not_none(): + runner = _FakeRunner(_FakeClient({"BOT": {}}), {"agent": {"botId": "BOT"}}) + assert reader.read_active_agent_connection_references(runner) == [] + + +def test_read_active_malformed_change_set_raises(): + runner = _FakeRunner( + _FakeClient({"BOT": {"connectionReferenceChanges": "not-a-list"}}), + {"agent": {"botId": "BOT"}}, + ) + with pytest.raises(ValueError): + reader.read_active_agent_connection_references(runner) + + +# -------------------------------------------------------------------------- +# read_all_agents_connection_references (ENV-004 surface: env-wide, de-duped) +# -------------------------------------------------------------------------- + +def test_read_all_none_when_no_client(): + runner = _FakeRunner(None, {"agents": [{"botId": "A"}]}) + assert reader.read_all_agents_connection_references(runner) is None + + +def test_read_all_dedups_by_logical_name_first_wins(): + ref_a = ab.workday_connection_reference( + connection_id="c1", logical_name="ns.shared_workdaysoap" + ) + ref_b = ab.workday_connection_reference( + connection_id="c2", logical_name="ns.shared_workdaysoap" + ) + client = _FakeClient( + { + "A": ab.components_with_references(references=[ref_a]), + "B": ab.components_with_references(references=[ref_b]), + } + ) + runner = _FakeRunner(client, {"agents": [{"botId": "A"}, {"botId": "B"}]}) + rows = reader.read_all_agents_connection_references(runner) + assert len(rows) == 1 + assert rows[0]["connectionid"] == "c1" + + +# -------------------------------------------------------------------------- +# shared_connection_parameter_values +# -------------------------------------------------------------------------- + +def test_scp_values_parsed_from_json_string(): + ref = { + "sharedconnectionparameters": ab.shared_connection_parameters_json_string( + rest_base_uri="https://wd.example.com/ccx/api" + ) + } + values = reader.shared_connection_parameter_values(ref) + assert values["restBaseUri"] == "https://wd.example.com/ccx/api" + + +def test_scp_values_parsed_from_nested_object(): + ref = {"sharedconnectionparameters": ab.shared_connection_parameters(tenant_name="mocktenant")} + values = reader.shared_connection_parameter_values(ref) + assert values["tenantName"] == "mocktenant" + + +def test_scp_values_missing_is_empty_map(): + assert reader.shared_connection_parameter_values({}) == {} + + +def test_scp_values_malformed_json_string_raises(): + ref = {"sharedconnectionparameters": "{not valid json"} + with pytest.raises(ValueError): + reader.shared_connection_parameter_values(ref) + + +# -------------------------------------------------------------------------- +# workday_shared_connection_parameters (WD-ENV-001 / WD-REST-001 surface) +# -------------------------------------------------------------------------- + +def test_wscp_found_with_values(): + ref = ab.workday_connection_reference( + shared_connection_parameters=ab.shared_connection_parameters_json_string() + ) + payload = ab.components_with_references(references=[ref]) + runner = _FakeRunner(_FakeClient({"BOT": payload}), {"agent": {"botId": "BOT"}}) + values, message = reader.workday_shared_connection_parameters(runner) + assert message == "" + assert values["tenantName"] == "mocktenant" + + +def test_wscp_found_but_missing_values(): + ref = ab.workday_connection_reference(shared_connection_parameters=None) + payload = ab.components_with_references(references=[ref]) + runner = _FakeRunner(_FakeClient({"BOT": payload}), {"agent": {"botId": "BOT"}}) + values, message = reader.workday_shared_connection_parameters(runner) + assert values == {} + assert "missing" in message.lower() + + +def test_wscp_workday_reference_not_found(): + # components() carries only the ServiceNow reference, no Workday one. + runner = _FakeRunner( + _FakeClient({"BOT": ab.components()}), {"agent": {"botId": "BOT"}} + ) + values, message = reader.workday_shared_connection_parameters(runner) + assert values == {} + assert "not found" in message.lower() + + +def test_wscp_none_when_client_unavailable(): + runner = _FakeRunner(None, {"agent": {"botId": "BOT"}}) + values, message = reader.workday_shared_connection_parameters(runner) + assert values is None + assert message diff --git a/tests/mocks/agentbuilder_connectivity.py b/tests/mocks/agentbuilder_connectivity.py index 9fb6c123d..f666f928c 100644 --- a/tests/mocks/agentbuilder_connectivity.py +++ b/tests/mocks/agentbuilder_connectivity.py @@ -5,6 +5,7 @@ from __future__ import annotations +import json from typing import Any, Iterable import responses @@ -20,6 +21,7 @@ MOCK_AGENT_ID = "00000000-0000-0000-0000-000000002222" MOCK_FAMILY_ID = "00000000-0000-0000-0000-000000003333" MOCK_CONNECTION_ID = "mock-servicenow-connection" +MOCK_WORKDAY_CONNECTION_ID = "mock-workday-connection" MOCK_AGENTBUILDER_BASE = ( "https://00000000000000000000000000000000." "0.environment.api.test.powerplatform.com" @@ -76,6 +78,116 @@ def components() -> dict[str, Any]: } +def connection_reference_change( + *, + connector: str, + connection_id: str | None, + logical_name: str | None = None, + shared_connection_parameters: dict[str, Any] | None = None, +) -> dict[str, Any]: + """One ``connectionReferenceChanges`` entry in the validated minimalBots + components shape (cassette ``agentbuilder_readiness.yaml``; the same shape + the shipped native ``DA-CONN-001`` check consumes). Only the ``connectorId`` + value varies from the captured ServiceNow reference, so this is + same-endpoint value variance and needs no new cassette (see + ``scripts/flightcheck/AGENTS.md``). ``connection_id=None`` models an unbound + reference. + """ + reference = { + "connectionReferenceLogicalName": ( + logical_name + or f"gptagent_mockemployeeselfservice.{connector}" + ), + "connectorId": ( + f"/providers/Microsoft.PowerApps/apis/{connector}" + ), + "connectionId": connection_id, + } + if shared_connection_parameters is not None: + reference["sharedConnectionParameters"] = ( + shared_connection_parameters + ) + return { + "changeType": "Insert", + "connectionReference": reference, + } + + +def workday_connection_reference( + *, + connection_id: str | None = MOCK_WORKDAY_CONNECTION_ID, + logical_name: str | None = None, + shared_connection_parameters: dict[str, Any] | None = None, +) -> dict[str, Any]: + """The Workday SOAP (``shared_workdaysoap``) connection-reference variant + that ``DV-CONN-001`` filters on.""" + return connection_reference_change( + connector="shared_workdaysoap", + connection_id=connection_id, + logical_name=logical_name, + shared_connection_parameters=shared_connection_parameters, + ) + + +def shared_connection_parameters( + *, + rest_base_uri: str | None = "https://wd.example.com/ccx/api", + tenant_name: str | None = "mocktenant", + resource_uri: str | None = "https://wd.example.com", + token_uri: str | None = "https://wd.example.com/ccx/oauth2/mocktenant/token", + client_id: str | None = "mock-client-id", +) -> dict[str, Any]: + """Workday ``sharedConnectionParameters`` from documented sources. + + Source (documented): + ``tools/ess-ca-to-da/reference/hr/agent.yml`` captures a real + ServiceNow ``sharedConnectionParameters`` entry using the nested + ``values..value`` wrapper shape. The Workday-specific fields mirror + the public Workday connector definition documented at + ``https://learn.microsoft.com/connectors/workdaysoap/``: + ``restBaseUri``, ``tenantName``, ``token:ResourceUri``, + ``token:WorkdayTokenUri``, and ``token:WorkdayClientId``. These + Workday-specific keys are not yet captured from a live AgentBuilder + components response. + """ + values: dict[str, dict[str, str]] = {} + for key, value in ( + ("restBaseUri", rest_base_uri), + ("tenantName", tenant_name), + ("token:ResourceUri", resource_uri), + ("token:WorkdayTokenUri", token_uri), + ("token:WorkdayClientId", client_id), + ): + if value is not None: + values[key] = {"value": value} + return {"values": values} + + +def shared_connection_parameters_json_string(**kwargs: Any) -> str: + """``sharedConnectionParameters`` as the JSON string the live AgentBuilder + components response returns, rather than a nested object. + + Source (documented): a live ServiceNow connection reference encodes + ``sharedConnectionParameters`` as a JSON string, so checks must parse it + before reading ``values``. + """ + return json.dumps(shared_connection_parameters(**kwargs)) + + +def components_with_references( + *, + references: Iterable[dict[str, Any]] | None = None, +) -> dict[str, Any]: + """``components()`` with its ``connectionReferenceChanges`` replaced by the + given references (``None`` -> an empty list, modelling an agent with no + connection references).""" + payload = components() + payload["connectionReferenceChanges"] = ( + [] if references is None else list(references) + ) + return payload + + def connection( *, connection_id: str = MOCK_CONNECTION_ID,