flightcheck: fail loud on malformed DA connection-ref payloads (reader hardening)

[Dawn Jeong (daeunJe0ng)]

Summary

Hardens the shared FlightCheck DA connection-reference reader (_da_connection_refs.py) to fail loudly on malformed bot-components payloads, instead of silently skipping bad data.

This PR was originally scoped to re-point DV-CONN-001 to the Declarative Agent (DA) components API for a Workday SOAP connection check. That approach is dropped for two reasons raised in review:

1. It repurposed the established checkpoint ID DV-CONN-001 (a Dataverse connection-reference check consumed by CEA docs and validation-matrix.md), which breaks those consumers.
2. The Workday shared_workdaysoap connection reference is flow/solution-scoped (bound via the Dataverse flow user-connections contract, ref new_sharedworkdaysoap_ff0df). It does not surface in the bot-components connectionReferenceChanges layer this check read. A live sweep of 24 DA agents found 0 carrying it there, so the check would FAILED "not found" on every real DA GA agent. This is the same wrong-layer defect that closed flightcheck: re-point WD-WF-CAT-001 + WD-REF-001 to Declarative Agent components API #318 and flightcheck: exclude per-agent DA connection refs from WD-PKG-001 fingerprint #328.

DV-CONN-001 is therefore restored to its original Dataverse meaning. The only surviving change is the reader hardening below.

Work items: parent AB#7865450 (ESS DA GA FlightCheck).

What changed

• checks/_da_connection_refs.py: _bot_connection_references now raises ValueError on a non-dict change entry or a present-but-non-dict connectionReference, and tolerates an absent/null connectionReference (skips it). This makes malformed payloads fail loudly rather than passing through as silent empties.
• tests/flightcheck/checks/test_da_connection_refs.py: +3 tests covering non-dict change entry (raises), malformed individual reference (raises), and change-without-reference (tolerated).

Design rationale

• The reader is shared by ENV-004 (read_all_agents_connection_references), which wraps the read in except Exception (environment.py:537/560), so the new fail-loud ValueError` degrades to a WARNING row rather than crashing the run. No regression to existing consumers.
• Restoring DV-CONN-001 keeps CEA docs and the validation matrix accurate.

Testing

• python -m ruff check solutions/ess-maker-skills/scripts/flightcheck tests/flightcheck -> clean.
• python -m pytest tests/flightcheck -q -> 1211 passed.

Deferred

A correct-layer DA Workday-connection check (reading the flow/solution binding, not bot-components) is deferred, same disposition as #318/#328. Noted in team continuity; no tracking item opened yet.

[nkemms]

Verdict: do not merge.

Finding count: 1 MAJOR.

[MAJOR] F-1 — Routing check can false-pass on stray text
Owner: Dev | Confidence: High | Impact: High

File: solutions/ess-maker-skills/scripts/flightcheck/checks/workday_extension.py · Line: 580
if "BeginDialog" in text and _USER_CONTEXT_TOPIC_V2 in text:

This only checks that both strings occur somewhere in the file. A comment, dead code, or unrelated dialog block can satisfy the condition even when the OnRedirect/BeginDialog wiring points elsewhere, so a broken user-context redirect can still pass readiness.

Fix: parse the topic YAML and verify the OnRedirect branch's BeginDialog.dialog target is WorkdaySystemGetUserContextV2 instead of using a raw substring search.

[nkemms]

Updated review finding — contract-breaking blocker

• Critical — DV-CONN-001 is repurposed across architectures. Existing CEA setup guidance, the validation matrix, and S5.4 use DV-CONN-001 to mean the Dataverse shared_commondataserviceforapps connection reference. This PR unconditionally changes that same checkpoint ID to a DA Workday SOAP reference from AgentBuilder, silently removing the original validation for existing callers.

Preserve the current DV-CONN-001 contract and introduce a DA-specific checkpoint ID, or branch explicitly by architecture while retaining the established CEA result. A checkpoint identifier needs one stable meaning for its consumers.

[nkemms]

Re-reviewed the current head (ab398f1) against main. This remains blocked by checkpoint-contract and result-correctness issues:

• DV-CONN-001 still changes meaning without changing identity — solutions/ess-maker-skills/scripts/flightcheck/registry.py:524-533 and checks/workday_extension.py:378-417 replace the documented Dataverse connection-reference checkpoint with an AgentBuilder Workday SOAP check. Existing setup docs and the validation matrix still consume DV-CONN-001 under its Dataverse contract. Preserve that contract or introduce a new checkpoint ID and update all consumers together.
• A disconnected connection can pass — workday_extension.py:442-460 returns Passed after resolving a connection ID but does not verify its health/status. Require the active/connected state before passing.
• Reference selection is ambiguous and provider-agnostic — workday_extension.py:391-400 chooses the first connector ID ending in /apis/shared_workdaysoap; multiple matches are order-dependent and a different provider namespace can match. Require the full provider-qualified ID and surface ambiguity.
• Malformed reference entries become a confident absence result — checks/_da_connection_refs.py:20-25,75-95 silently skips malformed connectionReferenceChanges elements despite the stated fail-loud contract. Surface the malformed service payload rather than converting it to “not found.”

[nkemms]

Re-reviewed the current head (ab398f1). One critical compatibility blocker remains:

DV-CONN-001 has been changed from the established Dataverse connection-reference checkpoint to a Workday SOAP connection-reference check, while its existing consumers still require the original contract. The validation matrix and setup flow continue to define S5.4 as the Dataverse reference (..._92b66 / msdyn_sharedcommondataserviceforapps_workdayruntime) bound to an active connection.

Please preserve DV-CONN-001 for that existing contract and assign the AgentBuilder Workday SOAP check a new checkpoint ID, updating its consumers together.

[nkemms]

Follow-up PR: After checkpoint identity is stable, harden the new AgentBuilder check by handling multiple matching references explicitly, requiring the full provider-qualified connector identity, surfacing malformed reference entries, and verifying live connection health before returning Passed. These additional improvements are not the blocker identified above.

[Dawn Jeong (daeunJe0ng)]

Thanks for the thorough review, Nkem, and sorry for the missing ping. I reworked #304 substantially since your last pass. Here's how each point is addressed.

Critical — DV-CONN-001 is repurposed across architectures ... Preserve the current DV-CONN-001 contract and introduce a DA-specific checkpoint ID.

Fixed. DV-CONN-001 is fully restored to its original self-contained Dataverse connection-reference check (registry.py and workday_extension.py are now byte-identical to main). CEA setup guidance, the validation matrix, and S5.4 keep their existing contract. No consumer changes meaning.

I did not re-add the DA Workday SOAP check under a new checkpoint ID, and that's deliberate, see the next point.

Follow-up — harden the new AgentBuilder check ... verifying live connection health before returning Passed ... ambiguous reference selection ... require the full provider-qualified connector identity.

While reworking this I confirmed the DA Workday check was reading the wrong architectural layer. It read the bot-components API (connectionReferenceChanges), but the Workday shared_workdaysoap reference is flow/solution-scoped (bound via the Dataverse flow user-connections contract, new_sharedworkdaysoap_ff0df) and never surfaces there. A live sweep of 24 DA agents found 0 carrying it on that surface, so the check would FAILED "not found" on every real DA GA agent. That's the same wrong-layer defect that closed #318 and #328, so I pulled the check out rather than ship a guaranteed false-fail. The correct-layer version (reading the flow/solution binding) is deferred as separate work. Because that check is gone, the disconnected-can-pass, ambiguous-selection, and live-health items no longer apply to any shipped code here.

Malformed reference entries become a confident absence result (_da_connection_refs.py) ... Surface the malformed service payload rather than converting it to "not found."

Fixed, and this is the only surviving change in the PR. _bot_connection_references now raises ValueError on a non-dict change entry or a present-but-non-dict connectionReference, so a malformed components payload degrades the owning check to a WARNING instead of a confident "not found". An absent/null connectionReference (a genuine non-connection change) is still tolerated. Added 3 tests covering those cases. This reader is consumed live by ENV-004, which wraps the read in except Exception, so the fail-loud path surfaces as a WARNING row rather than a crash.

Net result: the PR is now two files (_da_connection_refs.py + its test). ruff is clean and all 1211 flightcheck tests pass. Re-review whenever you have a moment, and happy to walk through the wrong-layer finding if useful.