From 67168d3311e3e12adf9a063200acacdbe0bf08aa Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Wed, 23 Sep 2026 16:13:17 -0700 Subject: [PATCH 1/6] 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, From dba6714282a38a20ab2cb831600a4c44edc26cc4 Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Tue, 22 Sep 2026 14:50:24 -0700 Subject: [PATCH 2/6] flightcheck: re-point DV-CONN-001 to Declarative Agent components API (7852506) Read the Workday SOAP connection reference from the Declarative Agent minimalBots components API (runner.agentbuilder.fetch_components) instead of the Custom Agent Dataverse connectionreferences query, so DV-CONN-001 works against DA-GA agents that no longer expose Dataverse. Verdict simplifies to found + bound (connectionId present); the Dataverse-only statuscode and multiple-ref branches are dropped since the components shape carries neither. BAP owner echo is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../flightcheck/checks/workday_extension.py | 195 +++++++---------- .../scripts/flightcheck/registry.py | 9 +- .../checks/test_workday_extension.py | 197 ++++++------------ tests/flightcheck/test_registry.py | 12 +- 4 files changed, 149 insertions(+), 264 deletions(-) diff --git a/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py b/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py index 9f023bc01..0dc741a17 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py @@ -20,11 +20,12 @@ (never assert a verdict from an unconfirmed API response shape) — this checkpoint echoes the observed ``connectionParametersSet.name`` for the operator to confirm, rather than PASS/FAIL on a guessed value. - * ``DV-CONN-001`` (S5.4) — the Dataverse connection reference the extension - pack ships (``…_92b66``, connector ``shared_commondataserviceforapps``) is - bound to an **active** connection, and its owner is echoed so the operator - can confirm it is their **own** account. Programmatic PASS/FAIL on a - documented-tier Dataverse ``connectionreferences`` read. + * ``DV-CONN-001`` (S5.4) — the Workday SOAP connection reference reported by + the Declarative Agent minimalBots components API is bound (``connectionId`` + present), and its owner is echoed so the operator can confirm it is their + **own** account. Programmatic PASS/FAIL on the validated minimalBots + components read (same endpoint + ``connectionReferenceChanges`` shape as the + shipped native ``DA-CONN-001`` check). * ``WD-REST-001`` (S5.5) — the captured ``restBaseUrl`` is present and **trimmed to** ``/api``. Pure-config check, no client. * ``WD-REST-002`` (S5.7) — the agent's ``user-context-setup.mcs.yml`` topic @@ -43,7 +44,7 @@ whole run. * **One CheckResult per checkpoint** (principle 7). * **No guessed API shapes** — the two API-backed checks read documented fields - only (Dataverse ``connectionid`` / ``statuscode``; BAP + only (minimalBots ``connectionReferenceChanges`` connector/connection ids; BAP ``connectionParametersSet.name`` / ``createdBy``), and degrade gracefully when a client is unavailable. * **Every** ``CheckResult`` declares ``roles=`` (enforced by @@ -52,18 +53,11 @@ from __future__ import annotations -import os import re -import sys from pathlib import Path from ..runner import CheckResult, Priority, Role, Status -# scripts/auth.py is on sys.path via cli.py at runtime (tests add it too); this -# mirrors checks/environment.py's top-level import so query_all is patchable as -# flightcheck.checks.workday_extension.query_all. -from auth import query_all # noqa: E402 - DOC_BASE = ( "https://learn.microsoft.com/en-us/copilot/microsoft-365/" "employee-self-service" @@ -91,12 +85,9 @@ _WORKDAY_RUNTIME_REF_LOGICAL_NAME = ( "msdyn_sharedworkdaysoap_workdayruntime" ) -# The Dataverse connection reference the simplified pack ships. -_DATAVERSE_CONNECTOR_SUFFIX = "/apis/shared_commondataserviceforapps" -_DATAVERSE_REF_SUFFIX = "92b66" -_DATAVERSE_RUNTIME_REF_LOGICAL_NAME = ( - "msdyn_sharedcommondataserviceforapps_workdayruntime" -) +# The Workday SOAP connection reference the Declarative Agent reports via the +# minimalBots components API (connector ``shared_workdaysoap``). +_WORKDAY_CONNECTOR_SUFFIX = "/apis/shared_workdaysoap" _REF_SUFFIX_RE = re.compile(r"_([0-9a-f]{5})$") # ---- Local user-context topic (WD-REST-002) ---- @@ -108,7 +99,7 @@ "Workday connection authentication type is Microsoft Entra ID Integrated" ) _DV_CONN_DESC = ( - "Dataverse connection reference bound to an active connection you own" + "Workday SOAP connection reference bound to a connection you own" ) _REST_URL_DESC = "Workday REST base URL present and trimmed to '/api'" _REDIRECT_DESC = ( @@ -155,14 +146,6 @@ def _is_workday_auth_ref(logical_name) -> bool: ) -def _is_dataverse_runtime_ref(logical_name) -> bool: - normalized = str(logical_name or "").casefold() - return ( - _ref_suffix(logical_name) == _DATAVERSE_REF_SUFFIX - or normalized == _DATAVERSE_RUNTIME_REF_LOGICAL_NAME.casefold() - ) - - def _host_of(url: str) -> str: """Return the host portion of an ``https://host/…`` URL for display.""" match = re.match(r"https?://([^/]+)", str(url).strip()) @@ -192,26 +175,40 @@ def _resolve_owner(props: dict) -> str: def _query_connection_references(runner): - """Return all Dataverse ``connectionreferences`` rows, or ``None`` when the - Dataverse token/endpoint is not available. - - Documented-tier read (Dataverse Web API v9.2) — no cassette required; tests - stub ``query_all``. + """Return the agent's connection references from the Declarative Agent + minimalBots components API, normalized to the row shape + ``_check_dv_connection`` consumes, or ``None`` when the AgentBuilder client + or the active-agent ``botId`` is unavailable. + + Validated-tier read (minimalBots ``POST …/components``). The same endpoint + and ``connectionReferenceChanges`` shape already back the shipped native + ``DA-CONN-001`` check (``checks/native_agent.py``); see + ``tests/fixtures/cassettes/INDEX.md`` and ``tests/mocks/ + agentbuilder_connectivity.py``. Any ``AgentBuilderHTTPError`` propagates so + the dispatcher degrades this checkpoint to a WARNING (fail loudly). """ - env_url = getattr(runner, "env_url", None) - dv_token = getattr(runner, "dv_token", None) - if not env_url or not dv_token: + 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 - # Belt-and-suspenders: keep scripts/ importable even if the module was - # imported before cli.py put it on the path. - sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..")) - return query_all( - env_url, - dv_token, - "connectionreferences", - "connectionreferenceid,connectionreferencelogicalname," - "connectionreferencedisplayname,connectorid,connectionid,statuscode", - ) + changeset = client.fetch_components(agent_id) or {} + changes = changeset.get("connectionReferenceChanges") + if not isinstance(changes, list): + return [] + refs = [] + for change in changes: + ref = (change or {}).get("connectionReference") or {} + refs.append( + { + "connectionreferencelogicalname": ref.get( + "connectionReferenceLogicalName" + ), + "connectorid": ref.get("connectorId"), + "connectionid": ref.get("connectionId"), + } + ) + return refs def _get_connections(runner): @@ -366,7 +363,7 @@ def _check_connection_auth(runner) -> list[CheckResult]: # ───────────────────────────────────────────────────────────────────── -# DV-CONN-001 — Dataverse connection reference binding (S5.4, PASS/FAIL). +# DV-CONN-001 — Workday SOAP connection reference binding (S5.4, PASS/FAIL). # ───────────────────────────────────────────────────────────────────── @@ -378,67 +375,44 @@ def _check_dv_connection(runner) -> list[CheckResult]: priority=Priority.HIGH.value, status=Status.SKIPPED.value, description=_DV_CONN_DESC, result=( - "Dataverse token not available — skipping the Dataverse " - "connection-reference check." + "AgentBuilder client or active-agent botId not available — " + "skipping the Workday connection-reference check." ), )] - dv_refs = [ - r - for r in refs - if str(r.get("connectorid") or "").lower().endswith( - _DATAVERSE_CONNECTOR_SUFFIX - ) - and _is_dataverse_runtime_ref( - r.get("connectionreferencelogicalname") - ) - ] - if len(dv_refs) > 1: - names = ", ".join( - sorted( - str(ref.get("connectionreferencelogicalname") or "(unnamed)") - for ref in dv_refs - ) - ) - return [CheckResult(roles=_MAKER_ROLES, - checkpoint_id="DV-CONN-001", category=_CATEGORY, - priority=Priority.HIGH.value, status=Status.WARNING.value, - description=_DV_CONN_DESC, - result=( - "Multiple ESS Dataverse connection references match the " - f"runtime and legacy package fingerprints: {names}. " - "FlightCheck cannot determine which reference is active." - ), - remediation=( - "Remove obsolete Workday package references, then rerun " - "FlightCheck against the remaining Dataverse binding." - ), - doc_link=_DOC_SIMPLIFIED, - )] - dv_ref = dv_refs[0] if dv_refs else None + wd_ref = next( + ( + r + for r in refs + if str(r.get("connectorid") or "") + .lower() + .endswith(_WORKDAY_CONNECTOR_SUFFIX) + ), + None, + ) - if dv_ref is None: + if wd_ref is None: return [CheckResult(roles=_MAKER_ROLES, checkpoint_id="DV-CONN-001", category=_CATEGORY, - priority=Priority.HIGH.value, status=Status.NOT_CONFIGURED.value, + priority=Priority.HIGH.value, status=Status.FAILED.value, description=_DV_CONN_DESC, result=( - "The ESS Dataverse connection reference " - f"(\u2026_{_DATAVERSE_REF_SUFFIX}, connector " - "shared_commondataserviceforapps) was not found in this " - "environment." + "The ESS Workday SOAP connection reference (connector " + "shared_workdaysoap) was not found in the Declarative Agent " + "components payload." ), remediation=( - "Install/repair the Workday extension pack so its Dataverse " - "connection reference is created, then bind it to a Dataverse " - "connection you own." + "Install or repair the Workday extension pack so its Workday " + "SOAP connection reference is created, then bind it to a " + "Workday connection you own." ), doc_link=_DOC_SIMPLIFIED, )] - dv_ref_name = str(dv_ref.get("connectionreferencelogicalname")) - connection_id = dv_ref.get("connectionid") - statuscode = dv_ref.get("statuscode") + wd_ref_name = str( + wd_ref.get("connectionreferencelogicalname") or "(unnamed)" + ) + connection_id = wd_ref.get("connectionid") if not connection_id: return [CheckResult(roles=_MAKER_ROLES, @@ -446,31 +420,13 @@ def _check_dv_connection(runner) -> list[CheckResult]: priority=Priority.HIGH.value, status=Status.FAILED.value, description=_DV_CONN_DESC, result=( - "The ESS Dataverse connection reference " - f"({dv_ref_name}) is unbound " - "(connectionid=null)." - ), - remediation=( - "In Power Platform / Copilot Studio, bind the Dataverse " - "connection reference to an active Dataverse connection owned " - "by your own account." - ), - doc_link=_DOC_SIMPLIFIED, - )] - - if statuscode != 1: - return [CheckResult(roles=_MAKER_ROLES, - checkpoint_id="DV-CONN-001", category=_CATEGORY, - priority=Priority.HIGH.value, status=Status.FAILED.value, - description=_DV_CONN_DESC, - result=( - "The ESS Dataverse connection reference " - f"({dv_ref_name}) is bound but inactive " - f"(statuscode={statuscode})." + "The ESS Workday SOAP connection reference " + f"({wd_ref_name}) is unbound (connectionId=null)." ), remediation=( - "Re-authenticate or re-bind the Dataverse connection so its " - "status is active, using an account you own." + "In Power Platform / Copilot Studio, bind the Workday SOAP " + "connection reference to an active Workday connection owned by " + "your own account." ), doc_link=_DOC_SIMPLIFIED, )] @@ -490,9 +446,8 @@ def _check_dv_connection(runner) -> list[CheckResult]: priority=Priority.HIGH.value, status=Status.PASSED.value, description=_DV_CONN_DESC, result=( - "The ESS Dataverse connection reference " - f"({dv_ref_name}) is bound to an active " - "connection." + owner_note + "The ESS Workday SOAP connection reference " + f"({wd_ref_name}) is bound to a connection." + owner_note ), doc_link=_DOC_SIMPLIFIED, )] diff --git a/solutions/ess-maker-skills/scripts/flightcheck/registry.py b/solutions/ess-maker-skills/scripts/flightcheck/registry.py index 6040d49ff..b5badccca 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/registry.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/registry.py @@ -521,15 +521,16 @@ class ResolvedPlan: priority=Priority.HIGH.value, roles=(Role.ESS_MAKER.value,), ), - # DV-CONN-001 — self-contained Dataverse read (its own connectionreferences - # query) plus a best-effort BAP owner echo. + # DV-CONN-001 — reads the Workday SOAP connection reference from the + # Declarative Agent minimalBots components API (AGENTBUILDER), plus a + # best-effort BAP owner echo (PP_ADMIN). CheckpointSpec( key="DV-CONN-001", category_fn=run_workday_extension_checks, category_label="Workday Extension", - clients=frozenset({DATAVERSE, PP_ADMIN}), + clients=frozenset({AGENTBUILDER, PP_ADMIN}), requires_config=True, - requires_dataverse_endpoint=True, + requires_dataverse_endpoint=False, priority=Priority.HIGH.value, roles=(Role.ESS_MAKER.value,), ), diff --git a/tests/flightcheck/checks/test_workday_extension.py b/tests/flightcheck/checks/test_workday_extension.py index 0e641716b..2910bb762 100644 --- a/tests/flightcheck/checks/test_workday_extension.py +++ b/tests/flightcheck/checks/test_workday_extension.py @@ -10,9 +10,9 @@ connection, degrades gracefully when it does not. Cached-ref read + a best-effort Power Platform admin owner echo — no cassette required (the admin connections listing is the ``validated`` pp_admin mock). - * DV-CONN-001 — PASS/FAIL/NOT_CONFIGURED/SKIPPED over a documented-tier - Dataverse ``connectionreferences`` read (stubbed with ``responses``); owner - echo via the ``validated`` pp_admin mock. + * DV-CONN-001 — PASS/FAIL/SKIPPED over the validated minimalBots components + read (Workday SOAP connection reference; faked ``runner.agentbuilder``); + owner echo via the ``validated`` pp_admin mock. * WD-REST-001 — pure-config check (restBaseUrl trimmed to '/api'). * WD-REST-002 — pure local-file check (user-context redirect topic); SKIPPED on the legacy install path. @@ -28,22 +28,18 @@ from dataclasses import dataclass, field from typing import Any -import responses - from tests.conftest import require_validated_mock +from tests.mocks import agentbuilder_connectivity as ab from tests.mocks import dataverse as dv from tests.mocks import pp_admin as pp +require_validated_mock(ab) require_validated_mock(dv) require_validated_mock(pp) from flightcheck.checks import workday_extension as wx # noqa: E402 from flightcheck.runner import Priority, Role, Status # noqa: E402 -_DV_CONNECTOR_ID = ( - "/providers/Microsoft.PowerApps/apis/shared_commondataserviceforapps" -) - # ───────────────────────────────────────────────────────────────────── # Minimal runner. The emitters read only these attributes; anything the @@ -62,12 +58,24 @@ def get_connections(self, _env_id: str): return self._connections +class _FakeAgentBuilder: + """Stand-in for FlightCheckRunner.agentbuilder. Only ``fetch_components`` + is consumed (DV-CONN-001's connection-reference read).""" + + def __init__(self, components: dict[str, Any]): + self._components = components + + def fetch_components(self, _agent_id: str): + return self._components + + @dataclass class _Runner: config: Any = field(default_factory=dict) env_url: str | None = None dv_token: str | None = None pp_admin: Any = None + agentbuilder: Any = None env_id: str | None = None _workday_connection_refs: list[dict[str, Any]] = field(default_factory=list) @@ -88,28 +96,6 @@ def _by_id(results): return {r.checkpoint_id: r for r in results} -def _dv_ref(*, connection_id, statuscode=1): - """A Dataverse connection reference matching the extension pack's shipped - ref (connector shared_commondataserviceforapps, logical-name suffix - 92b66).""" - return dv.connection_ref( - logical_name="msdyn_sharedcommondataserviceforapps_92b66", - display_name="Microsoft Dataverse", - connector_id=_DV_CONNECTOR_ID, - connection_id=connection_id, - statuscode=statuscode, - ) - - -def _register_refs(base_url: str, refs: list[dict[str, Any]]) -> None: - responses.add( - method="GET", - url=f"{base_url}/api/data/v9.2/connectionreferences", - json=dv.collection(refs), - status=200, - ) - - # ───────────────────────────────────────────────────────────────────── # WD-CONN-AUTH-001 — always MANUAL echo (S5.3). # ───────────────────────────────────────────────────────────────────── @@ -253,147 +239,88 @@ def test_never_passes_regardless_of_state(self): # ───────────────────────────────────────────────────────────────────── -# DV-CONN-001 — Dataverse connection binding (S5.4, PASS/FAIL). +# DV-CONN-001 — Workday SOAP connection binding (S5.4, PASS/FAIL). # ───────────────────────────────────────────────────────────────────── +def _runner_with_refs(references, *, pp_admin=None, env_id=None): + """A runner whose faked ``agentbuilder.fetch_components`` returns the given + connection references and whose config names an active agent (botId).""" + components = ab.components_with_references(references=references) + return _Runner( + config={"agent": {"botId": ab.MOCK_AGENT_ID}}, + agentbuilder=_FakeAgentBuilder(components), + pp_admin=pp_admin, + env_id=env_id, + ) + + class TestDataverseConnection: - @responses.activate - def test_bound_active_with_owner_echo_passes( - self, fake_dataverse_url, fake_token - ): - _register_refs( - fake_dataverse_url, - [_dv_ref(connection_id="dv-conn-active", statuscode=1)], - ) + def test_bound_with_owner_echo_passes(self): owner_conn = pp.connection( - name="dv-conn-active", - api_name="shared_commondataserviceforapps", + name="wd-conn-active", + api_name="shared_workdaysoap", extra_properties={"accountName": "maker@contoso.com"}, ) - runner = _Runner( - env_url=fake_dataverse_url, - dv_token=fake_token, + runner = _runner_with_refs( + [ab.workday_connection_reference(connection_id="wd-conn-active")], pp_admin=_FakePPAdmin([owner_conn]), env_id="env-1", ) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] assert r.status == Status.PASSED.value - assert "bound to an active" in r.result + assert "bound to a connection" in r.result assert "maker@contoso.com" in r.result assert "your own account" in r.result - @responses.activate - def test_passes_without_pp_admin_notes_owner_unreadable( - self, fake_dataverse_url, fake_token - ): - _register_refs( - fake_dataverse_url, - [_dv_ref(connection_id="dv-conn-active", statuscode=1)], + def test_passes_without_pp_admin_notes_owner_unreadable(self): + runner = _runner_with_refs( + [ab.workday_connection_reference(connection_id="wd-conn-active")], ) - runner = _Runner(env_url=fake_dataverse_url, dv_token=fake_token) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] assert r.status == Status.PASSED.value assert "owner could not be read" in r.result assert "your own account" in r.result - @responses.activate - def test_runtime_dataverse_reference_passes( - self, fake_dataverse_url, fake_token - ): - runtime_ref = dv.workday_connection_refs_runtime()[1] - _register_refs(fake_dataverse_url, [runtime_ref]) - runner = _Runner( - env_url=fake_dataverse_url, - dv_token=fake_token, - ) - - r = _by_id( - wx.run_workday_extension_checks(runner) - )["DV-CONN-001"] - - assert r.status == Status.PASSED.value - assert ( - "msdyn_sharedcommondataserviceforapps_workdayruntime" - in r.result + def test_unbound_fails(self): + runner = _runner_with_refs( + [ab.workday_connection_reference(connection_id=None)], ) - - @responses.activate - def test_mixed_runtime_and_legacy_dataverse_refs_warn( - self, fake_dataverse_url, fake_token - ): - runtime_ref = dv.workday_connection_refs_runtime()[1] - _register_refs( - fake_dataverse_url, - [_dv_ref(connection_id="legacy-dv"), runtime_ref], - ) - runner = _Runner( - env_url=fake_dataverse_url, - dv_token=fake_token, - ) - - r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] - - assert r.status == Status.WARNING.value - assert "Multiple ESS Dataverse connection references" in r.result - assert "Remove obsolete Workday package references" in r.remediation - - @responses.activate - def test_unbound_fails(self, fake_dataverse_url, fake_token): - _register_refs( - fake_dataverse_url, [_dv_ref(connection_id=None, statuscode=1)] - ) - runner = _Runner(env_url=fake_dataverse_url, dv_token=fake_token) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] assert r.status == Status.FAILED.value assert "unbound" in r.result - assert "connectionid=null" in r.result - assert "bind the Dataverse connection reference" in r.remediation - - @responses.activate - def test_inactive_statuscode_fails(self, fake_dataverse_url, fake_token): - _register_refs( - fake_dataverse_url, - [_dv_ref(connection_id="dv-conn-inactive", statuscode=2)], - ) - runner = _Runner(env_url=fake_dataverse_url, dv_token=fake_token) + assert "connectionId=null" in r.result + assert "bind the Workday SOAP connection reference" in r.remediation + + def test_workday_ref_absent_fails(self): + # Only the default ServiceNow ref present — no Workday SOAP ref. + runner = _runner_with_refs(None) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] assert r.status == Status.FAILED.value - assert "inactive" in r.result - assert "statuscode=2" in r.result - assert "Re-authenticate or re-bind" in r.remediation - - @responses.activate - def test_missing_ref_not_configured(self, fake_dataverse_url, fake_token): - # Only a Workday ref present — no Dataverse (92b66) ref. - _register_refs( - fake_dataverse_url, - [ - dv.connection_ref( - logical_name="new_sharedworkdaysoap_ff0df", - display_name="OAuthUser", - connector_id=dv.WORKDAY_SOAP_CONNECTOR_ID, - connection_id="wd-conn-1", - ) - ], - ) - runner = _Runner(env_url=fake_dataverse_url, dv_token=fake_token) + assert "was not found" in r.result + assert "shared_workdaysoap" in r.result + assert "Install or repair the Workday extension pack" in r.remediation + + def test_no_agentbuilder_client_skips(self): + runner = _Runner(config={"agent": {"botId": ab.MOCK_AGENT_ID}}) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] - assert r.status == Status.NOT_CONFIGURED.value - assert "was not found in this environment" in r.result - assert "Install/repair the Workday extension pack" in r.remediation + assert r.status == Status.SKIPPED.value + assert "not available" in r.result - def test_no_dv_token_skips(self): - runner = _Runner(env_url="https://x.crm.dynamics.com", dv_token="") + def test_no_active_agent_botid_skips(self): + runner = _Runner( + config={}, + agentbuilder=_FakeAgentBuilder(ab.components_with_references()), + ) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] assert r.status == Status.SKIPPED.value - assert "Dataverse token not available" in r.result + assert "not available" in r.result # ───────────────────────────────────────────────────────────────────── diff --git a/tests/flightcheck/test_registry.py b/tests/flightcheck/test_registry.py index e8882af57..ca696abe2 100644 --- a/tests/flightcheck/test_registry.py +++ b/tests/flightcheck/test_registry.py @@ -294,7 +294,7 @@ class TestWorkdayExtensionCheckpoints: """skill-5 mints five checkpoints, all sharing checks/workday_extension.run_workday_extension_checks, category "Workday Extension". Two are always-MANUAL echoes/attestations, three are - programmatic (one Dataverse read + two pure-local).""" + programmatic (one minimalBots components read + two pure-local).""" _ALL = ( "WD-CONN-AUTH-001", @@ -329,10 +329,12 @@ def test_conn_auth_exact_beats_wd_conn_family(self): assert registry.resolve("WD-CONN-AUTH-001").key == "WD-CONN-AUTH-001" assert registry.resolve("WD-CONN-AUTH-001").is_family is False - def test_dv_conn_spec_declares_dataverse_and_pp_admin(self): + def test_dv_conn_spec_declares_agentbuilder_and_pp_admin(self): spec = registry.resolve("DV-CONN-001") - assert spec.clients == frozenset({registry.DATAVERSE, registry.PP_ADMIN}) - assert spec.requires_dataverse_endpoint is True + assert spec.clients == frozenset( + {registry.AGENTBUILDER, registry.PP_ADMIN} + ) + assert spec.requires_dataverse_endpoint is False assert spec.prereqs == () assert Role.ESS_MAKER.value in spec.roles @@ -354,7 +356,7 @@ def test_net_check_is_clientless_and_ppadmin_gated(self): def test_dv_conn_plan_unions_clients(self): plan = registry.transitive_requirements("DV-CONN-001") - assert registry.DATAVERSE in plan.clients + assert registry.AGENTBUILDER in plan.clients assert registry.PP_ADMIN in plan.clients def test_all_five_are_listable(self): From 671de3677b9f684b4d36259e47d9af34aaa43226 Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Tue, 22 Sep 2026 15:07:25 -0700 Subject: [PATCH 3/6] flightcheck: fail loudly on malformed DV-CONN-001 changeset (review fix) Distinguish a genuinely-absent connectionReferenceChanges (missing key -> no references -> FAILED not-found) from a present-but-non-list payload (a shape we do not understand -> raise ValueError so the dispatcher degrades DV-CONN-001 to a WARNING). Mirrors native_agent._connection_references, the shipped precedent this check re-uses; the prior code collapsed both to [] and could report a confident "reference not found" on an unparseable 200 response. Adds a malformed-changeset test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../flightcheck/checks/workday_extension.py | 14 +++++++++++--- .../flightcheck/checks/test_workday_extension.py | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py b/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py index 0dc741a17..8ac4a47b1 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py @@ -184,8 +184,12 @@ def _query_connection_references(runner): and ``connectionReferenceChanges`` shape already back the shipped native ``DA-CONN-001`` check (``checks/native_agent.py``); see ``tests/fixtures/cassettes/INDEX.md`` and ``tests/mocks/ - agentbuilder_connectivity.py``. Any ``AgentBuilderHTTPError`` propagates so - the dispatcher degrades this checkpoint to a WARNING (fail loudly). + agentbuilder_connectivity.py``. Fails loudly (lets the dispatcher degrade + this checkpoint to a WARNING) rather than overclaiming: an + ``AgentBuilderHTTPError`` propagates, and a 200 payload whose + ``connectionReferenceChanges`` is present but not a list raises + ``ValueError`` (mirrors ``native_agent._connection_references``). A missing + changeset is treated as "no references" (genuine absence), not an error. """ client = getattr(runner, "agentbuilder", None) config = getattr(runner, "config", None) or {} @@ -194,8 +198,12 @@ def _query_connection_references(runner): return None changeset = client.fetch_components(agent_id) or {} changes = changeset.get("connectionReferenceChanges") - if not isinstance(changes, list): + if changes is None: return [] + if not isinstance(changes, list): + raise ValueError( + "Component fetch returned invalid connectionReferenceChanges." + ) refs = [] for change in changes: ref = (change or {}).get("connectionReference") or {} diff --git a/tests/flightcheck/checks/test_workday_extension.py b/tests/flightcheck/checks/test_workday_extension.py index 2910bb762..202372eb5 100644 --- a/tests/flightcheck/checks/test_workday_extension.py +++ b/tests/flightcheck/checks/test_workday_extension.py @@ -322,6 +322,22 @@ def test_no_active_agent_botid_skips(self): assert r.status == Status.SKIPPED.value assert "not available" in r.result + def test_malformed_changeset_degrades_to_warning(self): + # A 200 payload whose connectionReferenceChanges is present but not a + # list is a shape we do not understand: fail loudly (dispatcher WARNING) + # rather than reporting a confident "reference not found" FAILED. + runner = _Runner( + config={"agent": {"botId": ab.MOCK_AGENT_ID}}, + agentbuilder=_FakeAgentBuilder( + {"connectionReferenceChanges": {"unexpected": "dict"}} + ), + ) + r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] + + assert r.status == Status.WARNING.value + assert "Unable to run DV-CONN-001" in r.result + assert "DV-CONN-001" in r.remediation + # ───────────────────────────────────────────────────────────────────── # WD-REST-001 — REST base URL trimmed to /api (S5.5). From ab398f13739619cce437148ffa78ac1eab056061 Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Tue, 22 Sep 2026 23:20:14 -0700 Subject: [PATCH 4/6] tests: cover DV-CONN-001 ServiceNow-present true-negative (was empty-list) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8246cd2c-37d0-4000-8fe3-fa9b082669e0 --- tests/flightcheck/checks/test_workday_extension.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/flightcheck/checks/test_workday_extension.py b/tests/flightcheck/checks/test_workday_extension.py index 202372eb5..3a4b2dfa1 100644 --- a/tests/flightcheck/checks/test_workday_extension.py +++ b/tests/flightcheck/checks/test_workday_extension.py @@ -296,8 +296,15 @@ def test_unbound_fails(self): assert "bind the Workday SOAP connection reference" in r.remediation def test_workday_ref_absent_fails(self): - # Only the default ServiceNow ref present — no Workday SOAP ref. - runner = _runner_with_refs(None) + # Only a ServiceNow ref is present - no Workday SOAP ref. + runner = _runner_with_refs( + [ + ab.connection_reference_change( + connector="shared_service-now", + connection_id="sn-1", + ) + ] + ) r = _by_id(wx.run_workday_extension_checks(runner))["DV-CONN-001"] assert r.status == Status.FAILED.value From baddd4823200d4f8f3010e38b6d7b9f6e2d0d1f8 Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Tue, 22 Sep 2026 20:58:50 -0700 Subject: [PATCH 5/6] flightcheck: re-point publishing checks to DA ALM APIs (AB#7852507, AB#7852508) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8246cd2c-37d0-4000-8fe3-fa9b082669e0 --- .../scripts/flightcheck/checks/publishing.py | 444 ++++++++++++++++-- .../scripts/flightcheck/cli.py | 9 +- .../scripts/flightcheck/registry.py | 22 + tests/flightcheck/checks/test_publishing.py | 283 ++++++++++- tests/flightcheck/test_cli.py | 3 +- tests/flightcheck/test_registry.py | 16 + tests/mocks/agentbuilder_connectivity.py | 43 ++ 7 files changed, 783 insertions(+), 37 deletions(-) diff --git a/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py b/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py index 5adb2e1a8..c3d5ea476 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py @@ -4,21 +4,28 @@ """ ESS FlightCheck — Publishing & QA Validation (PUB-xxx, QA-xxx) -These checks are organizational/process gates that the kit cannot +Most checks are organizational/process gates that the kit cannot verify by reading an API (test sets live in Copilot Studio behind the -Analytics surface; managed-solution exports happen in the Power Apps -maker; UAT sign-off lives in the operator's change-management -system; M365 admin approval lives in the Microsoft 365 admin center). +Analytics surface; UAT sign-off lives in the operator's +change-management system; M365 admin approval lives in the Microsoft +365 admin center). PUB-001 and PUB-002 use the Copilot Studio +AgentBuilder ALM APIs when the required client is available. -Each check therefore emits ``Status.MANUAL`` — meaning "the kit has -nothing to check; the operator must confirm this themselves" — and -the remediation provides the concrete steps + best available deep -link for that specific action. +Rows that the kit cannot verify still emit ``Status.MANUAL`` — meaning +"the operator must confirm this themselves" — and the remediation provides +the concrete steps plus best available deep link for that specific action. Bucketing: MANUAL routes to the "Needs manual verification" section -of the FlightCheck report. These checks never fail readiness. +of the FlightCheck report. PUB-001/PUB-002 can pass or fail readiness +when their API probes run. """ +import tempfile +import zipfile +from pathlib import Path + +from agentbuilder import AgentBuilderHTTPError + from ..runner import CheckResult, Role, Status DOC_BASE = "https://learn.microsoft.com/en-us/copilot/microsoft-365/employee-self-service" @@ -26,6 +33,7 @@ M365_INTEGRATED_APPS_URL = ( "https://admin.microsoft.com/Adminportal/Home#/Settings/IntegratedApps" ) +ALM_NOT_OPTED_IN_CODE = "4003" def _studio_agent_url(runner) -> str | None: @@ -61,6 +69,382 @@ def _maker_solutions_url(runner) -> str | None: return f"https://make.powerapps.com/environments/{env_id}/solutions" +def _configured_bot_id(runner) -> str | None: + config = getattr(runner, "config", None) or {} + for agent in config.get("agents", []) or []: + bot_id = agent.get("botId") + if bot_id: + return str(bot_id) + bot_id = (config.get("agent") or {}).get("botId") + return str(bot_id) if bot_id else None + + +def _api_result( + *, + checkpoint_id: str, + row: dict, + status: Status, + result: str, + remediation: str, +) -> CheckResult: + return CheckResult( + checkpoint_id=checkpoint_id, + category="Publishing", + priority=row["p"], + status=status.value, + description=row["desc"], + result=result, + remediation=remediation, + doc_link=row["doc_link"], + roles=row["roles"], + ) + + +def _agentbuilder_unavailable(checkpoint_id: str, row: dict) -> CheckResult: + fallback = ( + f" Manual fallback: {row['remediation']}" + if row.get("remediation") + else "" + ) + return _api_result( + checkpoint_id=checkpoint_id, + row=row, + status=Status.SKIPPED, + result="Copilot Studio AgentBuilder ALM client is unavailable for this run.", + remediation=( + "Re-run FlightCheck in a scope that authenticates the Copilot Studio " + "AgentBuilder Power Platform API client, and make sure the " + f"environment ID can be resolved.{fallback}" + ), + ) + + +def _bot_id_missing(checkpoint_id: str, row: dict) -> CheckResult: + return _api_result( + checkpoint_id=checkpoint_id, + row=row, + status=Status.SKIPPED, + result="No configured agent botId was found in .local/config.json.", + remediation=( + "Run /setup or update .local/config.json so the active ESS agent has " + "a botId, then re-run FlightCheck." + ), + ) + + +def _is_alm_not_opted_in(error: Exception) -> bool: + if not isinstance(error, AgentBuilderHTTPError): + return False + code = str(error.error_code or "").strip() + if code == ALM_NOT_OPTED_IN_CODE: + return True + response = error.response + if response is None: + return False + try: + body = response.json() + except ValueError: + return False + return ALM_NOT_OPTED_IN_CODE in str(body) + + +def _invalid_archive_reason(path: Path) -> str | None: + try: + with zipfile.ZipFile(path, "r") as archive: + names = archive.namelist() + if not names: + return "the package archive has no entries" + for name in names: + entry = Path(name) + if entry.is_absolute() or ".." in entry.parts: + return f"the package archive contains unsafe entry {name!r}" + first_bad = archive.testzip() + if first_bad is not None: + return f"CRC validation failed for {first_bad!r}" + except zipfile.BadZipFile: + return "the response is not a valid zip archive" + return None + + +def _check_pub_001_export(runner, row: dict) -> CheckResult: + client = getattr(runner, "agentbuilder", None) + if client is None: + return _agentbuilder_unavailable("PUB-001", row) + + bot_id = _configured_bot_id(runner) + if not bot_id: + return _bot_id_missing("PUB-001", row) + + with tempfile.TemporaryDirectory(prefix="flightcheck-pub001-") as tmp: + package_path = Path(tmp) / "agent.zip" + try: + client.export_package(bot_id, package_path) + except Exception as exc: # noqa: BLE001 - report as a verdict row + if _is_alm_not_opted_in(exc): + return _api_result( + checkpoint_id="PUB-001", + row=row, + status=Status.FAILED, + result=( + f"AgentBuilder ALM export returned {ALM_NOT_OPTED_IN_CODE} " + f"for configured agent {bot_id}; the agent is not enrolled " + "in ALM." + ), + remediation=( + "Open the agent in Copilot Studio, go to Settings > ALM, " + "enroll the agent, then re-run PUB-001." + ), + ) + return _api_result( + checkpoint_id="PUB-001", + row=row, + status=Status.WARNING, + result=( + f"AgentBuilder ALM export failed for configured agent {bot_id}: " + f"{exc}" + ), + remediation=( + "Confirm the signed-in maker can export this agent through " + "Copilot Studio ALM, then re-run PUB-001." + ), + ) + + if not package_path.exists() or package_path.stat().st_size == 0: + return _api_result( + checkpoint_id="PUB-001", + row=row, + status=Status.FAILED, + result=( + f"AgentBuilder ALM export returned an empty package for {bot_id}." + ), + remediation=( + "Open the agent in Copilot Studio and confirm it can be " + "exported. If export still returns no bytes, fix the " + "agent/package issue before promotion." + ), + ) + + invalid_reason = _invalid_archive_reason(package_path) + if invalid_reason is not None: + return _api_result( + checkpoint_id="PUB-001", + row=row, + status=Status.FAILED, + result=( + f"AgentBuilder ALM export returned {package_path.stat().st_size} " + f"bytes for {bot_id}, but {invalid_reason}." + ), + remediation=( + "Re-run export from Copilot Studio. The promotion artifact " + "must be a readable .zip package whose central directory and " + "CRC checks pass." + ), + ) + + return _api_result( + checkpoint_id="PUB-001", + row=row, + status=Status.PASSED, + result=( + f"AgentBuilder ALM export returned a valid zip package for {bot_id} " + f"({package_path.stat().st_size} bytes)." + ), + remediation="", + ) + + +def _pub_002_requires_opt_in(row: dict) -> CheckResult: + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.SKIPPED, + result=( + "PUB-002 did not run because the AgentBuilder ALM import probe was " + "not explicitly enabled. FlightCheck stayed read-only and did not " + "create anything." + ), + remediation=( + "Run PUB-002 only against a throwaway environment where ALM import " + "is safe, then enable the import probe in the caller. Manual " + f"fallback: {row['remediation']}" + ), + ) + + +def _check_pub_002_import(runner, row: dict) -> CheckResult: + if not bool(getattr(runner, "alm_import_probe", False)): + return _pub_002_requires_opt_in(row) + + client = getattr(runner, "agentbuilder", None) + if client is None: + return _agentbuilder_unavailable("PUB-002", row) + + bot_id = _configured_bot_id(runner) + if not bot_id: + return _bot_id_missing("PUB-002", row) + + with tempfile.TemporaryDirectory(prefix="flightcheck-pub002-") as tmp: + package_path = Path(tmp) / "agent.zip" + try: + client.export_package(bot_id, package_path) + except Exception as exc: # noqa: BLE001 - report as a verdict row + if _is_alm_not_opted_in(exc): + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result=( + f"AgentBuilder ALM export returned {ALM_NOT_OPTED_IN_CODE} " + f"for configured agent {bot_id}; PUB-002 could not obtain " + "an import package." + ), + remediation=( + "Open the source agent in Copilot Studio, go to " + "Settings > ALM, enroll the agent, then re-run PUB-002 " + "against a throwaway environment." + ), + ) + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.WARNING, + result=( + f"AgentBuilder ALM export failed before import for {bot_id}: " + f"{exc}" + ), + remediation=( + "Fix PUB-001 first. PUB-002 needs the source agent's exported " + ".zip package before it can test import." + ), + ) + + invalid_reason = _invalid_archive_reason(package_path) + if invalid_reason is not None: + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result=( + f"PUB-002 could not use the exported ALM package because " + f"{invalid_reason}." + ), + remediation=( + "Fix PUB-001 first. PUB-002 imports the same exported .zip " + "package and requires the archive validation to pass." + ), + ) + + try: + outcome = client.import_package(package_path) + except Exception as exc: # noqa: BLE001 - report as a verdict row + if _is_alm_not_opted_in(exc): + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result=( + f"AgentBuilder ALM import returned {ALM_NOT_OPTED_IN_CODE}; " + "the target agent/environment is not enrolled in ALM." + ), + remediation=( + "Use a throwaway environment with ALM enabled, then " + "re-run PUB-002. Do not run the import probe against a " + "production environment." + ), + ) + if isinstance(exc, AgentBuilderHTTPError) and exc.status_code == 409: + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result=( + "AgentBuilder ALM import returned HTTP 409. The package " + "appears to conflict with an agent/schema already present " + "in the target environment." + ), + remediation=( + "Run PUB-002 in a throwaway environment that does not " + "already contain this exported agent, or clear the " + "conflicting test import before retrying." + ), + ) + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.WARNING, + result=f"AgentBuilder ALM import failed: {exc}", + remediation=( + "Confirm the maker has import permission and the target " + "throwaway environment has AgentBuilder ALM enabled." + ), + ) + + if not isinstance(outcome, dict): + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result="AgentBuilder ALM import returned a non-object response.", + remediation=( + "Retry the import after confirming the AgentBuilder ALM API " + "is returning the documented import response shape." + ), + ) + + if outcome.get("responseStatus") != "valid": + reason = str(outcome.get("reason") or "unknown") + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result=f"AgentBuilder ALM import returned invalid response: {reason}.", + remediation=( + "Treat the import as failed. The API must return a valid " + "imported agent identity before promotion." + ), + ) + + imported = outcome.get("result") + if not isinstance(imported, dict): + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result="AgentBuilder ALM import did not return imported agent details.", + remediation=( + "Treat the import as failed. The API must return cdsBotId " + "and schemaName for the imported agent." + ), + ) + imported_bot_id = str(imported.get("cdsBotId") or "").strip() + schema_name = str(imported.get("schemaName") or "").strip() + if not imported_bot_id or not schema_name: + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.FAILED, + result=( + "AgentBuilder ALM import did not return both cdsBotId and " + "schemaName for the imported agent." + ), + remediation=( + "Treat the import as failed. The API must return the " + "imported agent identity before promotion." + ), + ) + + return _api_result( + checkpoint_id="PUB-002", + row=row, + status=Status.PASSED, + result=( + f"AgentBuilder ALM import created agent {imported_bot_id} " + f"with schema {schema_name}." + ), + remediation="", + ) + + def _qa_remediation(runner, action: str, doc_anchor: str) -> str: """Build a QA-* remediation that points at Copilot Studio Analytics when the deep link is available, falling back to documentation.""" @@ -259,24 +643,30 @@ def _build_checks(runner) -> list[dict]: def run_publishing_checks(runner) -> list[CheckResult]: - """Return the publishing/QA checklist as MANUAL results. + """Return publishing/QA checks, using AgentBuilder ALM where available. - None of these checks reads an API — they're organizational gates - or actions on portals the kit doesn't traverse. Emitting them as - MANUAL (not NOT_CONFIGURED) keeps the report honest: nothing is - misconfigured, the operator just has work the kit can't witness. + PUB-001 validates export without mutating the environment. PUB-002 is + explicitly gated because import creates an agent in the target environment. """ - return [ - CheckResult( - checkpoint_id=c["id"], - category="Publishing", - priority=c["p"], - status=Status.MANUAL.value, - description=c["desc"], - result=c["result"], - remediation=c["remediation"], - doc_link=c["doc_link"], - roles=c["roles"], + results: list[CheckResult] = [] + for c in _build_checks(runner): + if c["id"] == "PUB-001": + results.append(_check_pub_001_export(runner, c)) + continue + if c["id"] == "PUB-002": + results.append(_check_pub_002_import(runner, c)) + continue + results.append( + CheckResult( + checkpoint_id=c["id"], + category="Publishing", + priority=c["p"], + status=Status.MANUAL.value, + description=c["desc"], + result=c["result"], + remediation=c["remediation"], + doc_link=c["doc_link"], + roles=c["roles"], + ) ) - for c in _build_checks(runner) - ] + return results diff --git a/solutions/ess-maker-skills/scripts/flightcheck/cli.py b/solutions/ess-maker-skills/scripts/flightcheck/cli.py index 31b2feeaa..c620a95c6 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/cli.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/cli.py @@ -144,10 +144,12 @@ ("Native Agent", run_native_agent_checks), ("Environment", run_capacity_check), ("Local Files", run_local_file_checks), + ("Publishing", run_publishing_checks), ], "environment": [("Environment", run_capacity_check)], "servicenow": [("Native Agent", run_native_agent_checks)], "workday": [("Native Agent", run_native_agent_checks)], + "publishing": [("Publishing", run_publishing_checks)], } NATIVE_CONNECTOR_FILTERS = { "servicenow": ("shared_service-now",), @@ -1295,7 +1297,12 @@ def main(): ) sys.exit(1) - needs_agent_readiness = args.scope in {"full", "servicenow", "workday"} + needs_agent_readiness = args.scope in { + "full", + "servicenow", + "workday", + "publishing", + } if needs_agent_readiness: environment_host = str( config.get("powerPlatformApiEndpoint") or "" diff --git a/solutions/ess-maker-skills/scripts/flightcheck/registry.py b/solutions/ess-maker-skills/scripts/flightcheck/registry.py index b5badccca..1a92977bc 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/registry.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/registry.py @@ -50,6 +50,7 @@ run_preferred_solution_check, ) from flightcheck.checks.native_agent import run_native_agent_checks +from flightcheck.checks.publishing import run_publishing_checks from flightcheck.checks.external_systems import run_external_systems_checks from flightcheck.checks.solution import run_solution_checks from flightcheck.checks.workday import run_workday_checks @@ -236,6 +237,26 @@ class ResolvedPlan: ), is_family=True, ), + CheckpointSpec( + key="PUB-001", + category_fn=run_publishing_checks, + category_label="Publishing", + clients=frozenset({AGENTBUILDER}), + requires_config=True, + requires_dataverse_endpoint=False, + priority=Priority.CRITICAL.value, + roles=(Role.ESS_MAKER.value,), + ), + CheckpointSpec( + key="PUB-002", + category_fn=run_publishing_checks, + category_label="Publishing", + clients=frozenset({AGENTBUILDER}), + requires_config=True, + requires_dataverse_endpoint=False, + priority=Priority.CRITICAL.value, + roles=(Role.ESS_MAKER.value, Role.POWER_PLATFORM_ADMIN.value), + ), CheckpointSpec( key="ENV-009", category_fn=run_preferred_solution_check, @@ -645,6 +666,7 @@ class ResolvedPlan: "WD-REST", "WD-NET", "DV-CONN", + "PUB", "TOPIC-TRIGGER", "TOPIC-INTEGRATION", ) diff --git a/tests/flightcheck/checks/test_publishing.py b/tests/flightcheck/checks/test_publishing.py index a3191633a..56ecf7aaf 100644 --- a/tests/flightcheck/checks/test_publishing.py +++ b/tests/flightcheck/checks/test_publishing.py @@ -28,6 +28,12 @@ from types import SimpleNamespace import pytest +import requests + +from tests.conftest import require_validated_mock +from tests.mocks import agentbuilder_connectivity as ab + +require_validated_mock(ab) @pytest.fixture(autouse=True) @@ -45,12 +51,74 @@ def _scripts_on_path(): pass -def _runner(env_id: str | None = "env-abc", bot_id: str | None = "bot-xyz"): +class _FakeAgentBuilder: + def __init__( + self, + *, + export_bytes: bytes | None = None, + export_error: Exception | None = None, + import_result: dict | None = None, + import_error: Exception | None = None, + ) -> None: + self.export_bytes = ( + ab.export_package_bytes() if export_bytes is None else export_bytes + ) + self.export_error = export_error + self.import_result = import_result or ab.import_package_result() + self.import_error = import_error + self.export_calls: list[tuple[str, Path]] = [] + self.import_calls: list[Path] = [] + + def export_package(self, agent_id: str, destination: Path) -> None: + self.export_calls.append((agent_id, destination)) + if self.export_error is not None: + raise self.export_error + destination.write_bytes(self.export_bytes) + + def import_package(self, package_path: Path) -> dict: + self.import_calls.append(package_path) + if self.import_error is not None: + raise self.import_error + return self.import_result + + +def _agentbuilder_http_error( + *, + status_code: int, + error_code: str, + body: dict | None = None, +): + from agentbuilder import AgentBuilderHTTPError + + response = requests.Response() + response.status_code = status_code + response._content = b"{}" if body is None else str(body).encode() + response.headers["x-ms-request-id"] = "request-1" + return AgentBuilderHTTPError( + "Native ALM test", + status_code, + error_code=error_code, + request_id="request-1", + response=response, + ) + + +def _runner( + env_id: str | None = "env-abc", + bot_id: str | None = "bot-xyz", + agentbuilder=None, + alm_import_probe: bool = False, +): """Minimal runner stub exposing the two attributes publishing.py reads.""" config: dict = {} if bot_id: config["agents"] = [{"slug": "esshr", "botId": bot_id}] - return SimpleNamespace(env_id=env_id, config=config) + return SimpleNamespace( + env_id=env_id, + config=config, + agentbuilder=agentbuilder, + alm_import_probe=alm_import_probe, + ) def _results_by_id(runner) -> dict: @@ -70,17 +138,34 @@ def test_all_eight_checks_emitted(): } -def test_every_check_is_manual_not_notconfigured(): - """The user's principle: nothing here is misconfigured. Don't - label it ``NotConfigured`` — that wrongly implies setup is missing.""" +def test_non_api_checks_are_manual_not_notconfigured(): + """Rows the kit still cannot verify remain manual, not NotConfigured.""" from flightcheck.runner import Status - for r in _results_by_id(_runner()).values(): + by_id = _results_by_id(_runner()) + for check_id, r in by_id.items(): + if check_id in {"PUB-001", "PUB-002"}: + continue assert r.status == Status.MANUAL.value, ( f"{r.checkpoint_id} regressed to status={r.status!r}; " "publishing/QA gates are manual, not NotConfigured." ) +def test_pub_api_checks_skip_without_agentbuilder_client(): + from flightcheck.runner import Status + + by_id = _results_by_id(_runner(agentbuilder=None)) + + assert by_id["PUB-001"].status == Status.SKIPPED.value + assert "AgentBuilder ALM client is unavailable" in by_id["PUB-001"].result + assert "authenticates the Copilot Studio AgentBuilder" in by_id[ + "PUB-001" + ].remediation + assert by_id["PUB-002"].status == Status.SKIPPED.value + assert "import probe was not explicitly enabled" in by_id["PUB-002"].result + assert "throwaway environment" in by_id["PUB-002"].remediation + + def test_every_check_has_concrete_result_text(): """No row may carry the old "Manual verification required" stub.""" for r in _results_by_id(_runner()).values(): @@ -148,7 +233,7 @@ def test_qa_checks_point_at_evaluations_doc(): def test_pub_001_links_to_maker_solutions_for_export(): - by_id = _results_by_id(_runner(env_id="env-abc")) + by_id = _results_by_id(_runner(env_id="env-abc", agentbuilder=None)) text = by_id["PUB-001"].remediation assert "https://make.powerapps.com/environments/env-abc/solutions" in text, ( f"PUB-001 must deep-link to the maker Solutions list so the " @@ -159,7 +244,7 @@ def test_pub_001_links_to_maker_solutions_for_export(): def test_pub_002_describes_target_environment_import(): - text = _results_by_id(_runner())["PUB-002"].remediation + text = _results_by_id(_runner(agentbuilder=None))["PUB-002"].remediation # The action happens in a *different* environment than the kit # was pointed at, so we can't deep-link — but we must say where. assert "test environment" in text.lower() @@ -194,3 +279,185 @@ def test_pub_011_is_informational_with_no_action_at_publish_time(): ) # And still link to where to check status if rollout drags. assert "Integrated apps" in text + + +# ----------------------------------------------------------- PUB API checks + + +def test_pub_001_passes_when_export_returns_valid_archive(): + from flightcheck.runner import Status + + client = _FakeAgentBuilder() + by_id = _results_by_id(_runner(agentbuilder=client)) + + row = by_id["PUB-001"] + assert row.status == Status.PASSED.value + assert "valid zip package" in row.result + assert "bot-xyz" in row.result + assert row.remediation == "" + assert client.export_calls[0][0] == "bot-xyz" + + +def test_pub_001_fails_when_export_archive_is_corrupt(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner(agentbuilder=_FakeAgentBuilder(export_bytes=b"PK\x03\x04junk")) + ) + + row = by_id["PUB-001"] + assert row.status == Status.FAILED.value + assert "not a valid zip archive" in row.result + assert "central directory and CRC checks pass" in row.remediation + + +def test_pub_001_fails_when_export_returns_4003_not_opted_in(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner( + agentbuilder=_FakeAgentBuilder( + export_error=_agentbuilder_http_error( + status_code=400, + error_code="4003", + ) + ) + ) + ) + + row = by_id["PUB-001"] + assert row.status == Status.FAILED.value + assert "not enrolled in ALM" in row.result + assert "Settings > ALM" in row.remediation + + +def test_pub_001_warns_on_export_http_error(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner( + agentbuilder=_FakeAgentBuilder( + export_error=_agentbuilder_http_error( + status_code=500, + error_code="ExportFailed", + ) + ) + ) + ) + + row = by_id["PUB-001"] + assert row.status == Status.WARNING.value + assert "ALM export failed" in row.result + assert "re-run PUB-001" in row.remediation + + +def test_pub_001_skips_without_bot_id(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner(bot_id=None, agentbuilder=_FakeAgentBuilder()) + ) + + row = by_id["PUB-001"] + assert row.status == Status.SKIPPED.value + assert "No configured agent botId" in row.result + assert ".local/config.json" in row.remediation + + +def test_pub_002_passes_when_import_returns_valid_identity(): + from flightcheck.runner import Status + + client = _FakeAgentBuilder() + by_id = _results_by_id( + _runner(agentbuilder=client, alm_import_probe=True) + ) + + row = by_id["PUB-002"] + assert row.status == Status.PASSED.value + assert "ALM import created agent" in row.result + assert "gptagent_mockemployeeselfservice_imported" in row.result + assert row.remediation == "" + assert client.import_calls + + +def test_pub_002_fails_on_same_environment_import_conflict(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner( + agentbuilder=_FakeAgentBuilder( + import_error=_agentbuilder_http_error( + status_code=409, + error_code="DuplicateItemError", + ) + ), + alm_import_probe=True, + ) + ) + + row = by_id["PUB-002"] + assert row.status == Status.FAILED.value + assert "HTTP 409" in row.result + assert "already contain this exported agent" in row.remediation + + +def test_pub_002_fails_when_import_returns_4003_not_opted_in(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner( + agentbuilder=_FakeAgentBuilder( + import_error=_agentbuilder_http_error( + status_code=400, + error_code="4003", + ) + ), + alm_import_probe=True, + ) + ) + + row = by_id["PUB-002"] + assert row.status == Status.FAILED.value + assert "not enrolled in ALM" in row.result + assert "throwaway environment" in row.remediation + + +def test_pub_002_warns_on_import_http_error(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner( + agentbuilder=_FakeAgentBuilder( + import_error=_agentbuilder_http_error( + status_code=500, + error_code="ImportFailed", + ) + ), + alm_import_probe=True, + ) + ) + + row = by_id["PUB-002"] + assert row.status == Status.WARNING.value + assert "ALM import failed" in row.result + assert "import permission" in row.remediation + + +def test_pub_002_skips_without_client_or_bot_id(): + from flightcheck.runner import Status + + no_client = _results_by_id(_runner(alm_import_probe=True))["PUB-002"] + no_bot = _results_by_id( + _runner( + bot_id=None, + agentbuilder=_FakeAgentBuilder(), + alm_import_probe=True, + ) + )["PUB-002"] + + assert no_client.status == Status.SKIPPED.value + assert "AgentBuilder ALM client is unavailable" in no_client.result + assert "authenticates the Copilot Studio AgentBuilder" in no_client.remediation + assert no_bot.status == Status.SKIPPED.value + assert "No configured agent botId" in no_bot.result + assert ".local/config.json" in no_bot.remediation diff --git a/tests/flightcheck/test_cli.py b/tests/flightcheck/test_cli.py index 3508aa24d..e54adf12e 100644 --- a/tests/flightcheck/test_cli.py +++ b/tests/flightcheck/test_cli.py @@ -273,7 +273,7 @@ class TestAgentBuilderNativeScopes: [ ( "full", - ["Native Agent", "Environment", "Local Files"], + ["Native Agent", "Environment", "Local Files", "Publishing"], True, True, None, @@ -293,6 +293,7 @@ class TestAgentBuilderNativeScopes: False, ("shared_workdaysoap",), ), + ("publishing", ["Publishing"], True, False, None), ], ) def test_native_no_dataverse_scope_uses_only_native_clients( diff --git a/tests/flightcheck/test_registry.py b/tests/flightcheck/test_registry.py index ca696abe2..0417c53d3 100644 --- a/tests/flightcheck/test_registry.py +++ b/tests/flightcheck/test_registry.py @@ -365,6 +365,22 @@ def test_all_five_are_listable(self): assert cp in keys +class TestPublishingCheckpoints: + def test_pub_checks_resolve_to_publishing_category_with_agentbuilder(self): + for cp in ("PUB-001", "PUB-002"): + spec = registry.resolve(cp) + assert spec is not None and spec.key == cp + assert spec.category_label == "Publishing" + assert spec.clients == frozenset({registry.AGENTBUILDER}) + assert spec.requires_dataverse_endpoint is False + assert spec.priority == Priority.CRITICAL.value + assert Role.ESS_MAKER.value in spec.roles + + def test_pub_checks_are_listable(self): + keys = {spec.key for spec in registry.list_checkpoints()} + assert {"PUB-001", "PUB-002"} <= keys + + class TestTopicCheckpoints: """skill-6 mints two FAMILY checkpoints (one row per new/custom topic), both sharing checks/topics.run_topic_checks, category "Workday Topics". diff --git a/tests/mocks/agentbuilder_connectivity.py b/tests/mocks/agentbuilder_connectivity.py index f666f928c..70380df4c 100644 --- a/tests/mocks/agentbuilder_connectivity.py +++ b/tests/mocks/agentbuilder_connectivity.py @@ -5,7 +5,9 @@ from __future__ import annotations +import io import json +import zipfile from typing import Any, Iterable import responses @@ -188,6 +190,47 @@ def components_with_references( return payload +def export_package_bytes( + *, + filename: str = "agent/manifest.json", + content: bytes = b'{"schemaName":"gptagent_mockemployeeselfservice"}', +) -> bytes: + """Real in-memory zip archive for AgentBuilder ALM export tests. + + Cited consumers: + - solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py + + Source (validated): + AgentBuilder ALM export is implemented by + AgentBuilderClient.export_package, which streams the response from + POST /copilotstudio/minimalBots/alm/{agent_id}/export into a caller-owned + .zip path. The route, method, and binary package contract are pinned by + tests/scripts/test_agentbuilder.py::test_realm_discovery_configuration_and_export_use_native_alm_requests. + The FlightCheck check validates the archive by reading the zip central + directory and running testzip() CRC validation, so this builder returns + an actual zipfile archive rather than a magic-byte stub. + """ + stream = io.BytesIO() + with zipfile.ZipFile(stream, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr(filename, content) + return stream.getvalue() + + +def import_package_result( + *, + agent_id: str = MOCK_AGENT_ID, + schema_name: str = "gptagent_mockemployeeselfservice_imported", +) -> dict[str, Any]: + """Successful AgentBuilderClient.import_package result shape.""" + return { + "responseStatus": "valid", + "result": { + "cdsBotId": agent_id, + "schemaName": schema_name, + }, + } + + def connection( *, connection_id: str = MOCK_CONNECTION_ID, From 283a2483d42b81617c3efe623fc0dd0512238201 Mon Sep 17 00:00:00 2001 From: Dawn Jeong Date: Tue, 22 Sep 2026 23:25:09 -0700 Subject: [PATCH 6/6] flightcheck: reword PUB rows to DA ALM framing and add import-probe safety-gate test (PUB-001/PUB-002) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8246cd2c-37d0-4000-8fe3-fa9b082669e0 --- .../scripts/flightcheck/checks/publishing.py | 44 ++++++++--------- tests/flightcheck/checks/test_publishing.py | 49 ++++++++++++++++--- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py b/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py index c3d5ea476..74a231628 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/checks/publishing.py @@ -156,7 +156,11 @@ def _invalid_archive_reason(path: Path) -> str | None: return "the package archive has no entries" for name in names: entry = Path(name) - if entry.is_absolute() or ".." in entry.parts: + if ( + name.startswith(("/", "\\")) + or entry.is_absolute() + or ".." in entry.parts + ): return f"the package archive contains unsafe entry {name!r}" first_bad = archive.testzip() if first_bad is not None: @@ -467,7 +471,6 @@ def _build_checks(runner) -> list[dict]: """Per-check authored content. Constructed at call-time so deep links can incorporate the runner's environment / agent IDs.""" studio = _studio_agent_url(runner) - solutions = _maker_solutions_url(runner) publish_doc = f"{DOC_BASE}/publish" deploy_doc = f"{DOC_BASE}/deploy-overview-alm" evaluations_doc = f"{DOC_BASE}/evaluations" @@ -475,10 +478,6 @@ def _build_checks(runner) -> list[dict]: # Studio link as a markdown fragment ready to splice into prose, # or the literal phrase "Copilot Studio" when no deep link exists. studio_md = f"[Copilot Studio]({studio})" if studio else "Copilot Studio" - solutions_md = ( - f"[Power Apps → Solutions]({solutions})" - if solutions else "Power Apps → Solutions" - ) return [ { @@ -546,18 +545,17 @@ def _build_checks(runner) -> list[dict]: "id": "PUB-001", "p": "Critical", "roles": [Role.ESS_MAKER.value], - "desc": "Export your customization solution as a managed solution", + "desc": "Export the agent's ALM package as a .zip from AgentBuilder", "result": ( - "The kit can't inspect maker-portal solution exports — " - "confirm a managed (.zip) export exists for promotion to test/UAT/prod." + "The kit can't inspect AgentBuilder ALM package downloads — " + "confirm the agent's ALM package .zip exists for promotion to test/UAT/prod." ), "remediation": ( - f"In {solutions_md} → select the solution that contains your " - f"agent customizations → ⋯ → **Export solution** → **Publish** " - f"(publish all customizations first) → **Next** → choose " - f"**Managed** → **Export** → **Download**. Keep the .zip — " - f"it's the artifact you import into test/UAT/prod. See the " - f"[publish guide]({publish_doc}) for the full deployment flow." + f"In {studio_md}, open the agent → **Settings** → **ALM**. " + f"Enroll the agent if prompted, then export and download the " + f"agent's ALM package .zip. Keep the .zip — it's the artifact " + f"you import into test/UAT/prod. See the [publish guide]" + f"({publish_doc}) for the full deployment flow." ), "doc_link": publish_doc, }, @@ -565,18 +563,18 @@ def _build_checks(runner) -> list[dict]: "id": "PUB-002", "p": "Critical", "roles": [Role.ESS_MAKER.value, Role.POWER_PLATFORM_ADMIN.value], - "desc": "Import the managed solution into a test environment", + "desc": "Import the agent's ALM package into a test environment", "result": ( "The kit only sees the configured environment — " - "confirm the managed solution was imported into a non-production environment and smoke-tested." + "confirm the agent's ALM package was imported into a non-production environment and smoke-tested." ), "remediation": ( - "Switch to your test environment in the Power Apps maker → " - "**Solutions** → **Import solution** → upload the managed .zip " - "from PUB-001 → install any prompted dependencies (the ESS " - "agent itself plus any connector solutions) → open the agent " - f"and smoke-test a handful of representative prompts. See the " - f"[publish guide]({publish_doc}) for the full deployment flow." + "Switch to your test environment in Copilot Studio, open " + "**Settings** → **ALM**, then import the agent's ALM package " + ".zip from PUB-001. Install any prompted dependencies, open " + "the imported agent, and smoke-test a handful of representative " + f"prompts. See the [publish guide]({publish_doc}) for the full " + f"deployment flow." ), "doc_link": publish_doc, }, diff --git a/tests/flightcheck/checks/test_publishing.py b/tests/flightcheck/checks/test_publishing.py index 56ecf7aaf..ea96d7f22 100644 --- a/tests/flightcheck/checks/test_publishing.py +++ b/tests/flightcheck/checks/test_publishing.py @@ -23,7 +23,9 @@ from __future__ import annotations +import io import sys +import zipfile from pathlib import Path from types import SimpleNamespace @@ -126,6 +128,13 @@ def _results_by_id(runner) -> dict: return {r.checkpoint_id: r for r in run_publishing_checks(runner)} +def _zip_bytes(entry_name: str) -> bytes: + archive_bytes = io.BytesIO() + with zipfile.ZipFile(archive_bytes, "w") as archive: + archive.writestr(entry_name, "content") + return archive_bytes.getvalue() + + # --------------------------------------------------------------- shape @@ -232,15 +241,15 @@ def test_qa_checks_point_at_evaluations_doc(): # ----------------------------------------------------------- PUB deep links -def test_pub_001_links_to_maker_solutions_for_export(): +def test_pub_001_links_to_studio_alm_for_export(): by_id = _results_by_id(_runner(env_id="env-abc", agentbuilder=None)) text = by_id["PUB-001"].remediation - assert "https://make.powerapps.com/environments/env-abc/solutions" in text, ( - f"PUB-001 must deep-link to the maker Solutions list so the " - f"operator can find Export → Managed; got: {text!r}" + assert "https://copilotstudio.microsoft.com/environments/env-abc/" in text, ( + f"PUB-001 must deep-link to Copilot Studio so the operator can " + f"export the agent ALM package; got: {text!r}" ) # And the operator is told what to actually click. - assert "Export solution" in text and "Managed" in text + assert "Settings" in text and "ALM" in text and "package .zip" in text def test_pub_002_describes_target_environment_import(): @@ -248,7 +257,7 @@ def test_pub_002_describes_target_environment_import(): # The action happens in a *different* environment than the kit # was pointed at, so we can't deep-link — but we must say where. assert "test environment" in text.lower() - assert "Import solution" in text + assert "ALM package" in text and "Copilot Studio" in text def test_pub_003_is_explicitly_organizational(): @@ -311,6 +320,19 @@ def test_pub_001_fails_when_export_archive_is_corrupt(): assert "central directory and CRC checks pass" in row.remediation +def test_pub_001_fails_when_export_archive_has_posix_absolute_entry(): + from flightcheck.runner import Status + + by_id = _results_by_id( + _runner(agentbuilder=_FakeAgentBuilder(export_bytes=_zip_bytes("/evil"))) + ) + + row = by_id["PUB-001"] + assert row.status == Status.FAILED.value + assert "unsafe entry '/evil'" in row.result + assert "readable .zip package" in row.remediation + + def test_pub_001_fails_when_export_returns_4003_not_opted_in(): from flightcheck.runner import Status @@ -380,6 +402,21 @@ def test_pub_002_passes_when_import_returns_valid_identity(): assert client.import_calls +def test_pub_002_skips_import_probe_when_not_opted_in_with_client_present(): + from flightcheck.runner import Status + + client = _FakeAgentBuilder() + by_id = _results_by_id( + _runner(agentbuilder=client, alm_import_probe=False) + ) + + row = by_id["PUB-002"] + assert row.status == Status.SKIPPED.value + assert "import probe was not explicitly enabled" in row.result + assert "throwaway environment" in row.remediation + assert not client.import_calls + + def test_pub_002_fails_on_same_environment_import_conflict(): from flightcheck.runner import Status