flightcheck: add TOPIC-020 — verify handoff agent id is not the shipped placeholder

[daeunJe0ng]

Implements the FlightCheck check requested in #152.

Checkpoint: TOPIC-020
Category: Agent Handoff
Priority: High
Conditional? Yes — fires only for ENABLED handoff topics (status == "Active"). Not-configured PVA, missing bot id, fetch errors, and no-enabled-handoff-topic all return no results.

What identifies a handoff topic: a topic contains a SetVariable node assigning Topic.HandoffAgentId. Identity is that variable, NOT the schemaName. Makers clone and rename the sample template (agent-handoff.md Step 1) or enable an out-of-the-box Handoff Accelerator, so real enabled topics do not carry the Agenthandoff-scenarioname name. Keying on the name would skip exactly the topics the maker enabled and misconfigured — the case #152 exists to catch.

What it validates: For each enabled handoff topic, resolve Topic.HandoffAgentId and confirm it is a concrete GPT id, not a shipped placeholder and not blank. Placeholder detection covers the whole family: the sample template ships "AgentIdentifier"; the OOTB accelerators (Workday / ServiceNow) ship system-prefixed variants such as "ServiceNowAgentIdentifier". Match is a case-insensitive ...AgentIdentifier suffix — real target ids are prefixed GUIDs (T_<guid>, P_<guid>) and never end that way. A placeholder or blank value makes the handoff fail at runtime.

Remediation (two paths): On failure the check offers both correct end states — set a valid target agent id (agent-handoff.md Step 4), OR disable the topic if the handoff was unintended (returns it to the OOTB default). Either resolves the failure.

API tier: validated
Mock source:

• tests/fixtures/cassettes/island_gateway_botcomponents.yaml (interaction 2, the POST botcomponents response). The captured handoff DialogComponent ships status="Inactive" and expressionText: "\"AgentIdentifier\"". Read via pva_client.get_dialog_components(bot_id), mirroring get_knowledge_sources.
• Inference note: status == "Active" meaning "enabled" is inferred; the captured cassette only holds the disabled template. Enabled shape is consistent with other live components but unproven pending a captured enabled cassette. Validated live instead (below).

Tests: tests/flightcheck/checks/test_agent_handoff.py — gating (no client / not configured / no bot id / no handoff topic), identity by SetVariable (a handoff-named topic without the SetVariable is ignored; a renamed clone with the placeholder fails), GOOD (concrete id → Passed), BAD (sample placeholder → Failed; accelerator-prefixed ServiceNowAgentIdentifier → Failed; blank → Failed), and two-path remediation text. DialogComponent dicts are built inline from the cassette shape.

Live E2E (EmployeeHub tenant): ran the real check across the accessible ServiceNow environments. Of the handoff topics found, 4 were enabled: a blank-target accelerator → Failed; an AgentIdentifier template → Failed; an AgentIdentifier template on a second env → Failed; a renamed accelerator with a concrete T_<guid> id → Passed. 2 of the 4 enabled topics had schemaNames without agenthandoff, which the previous schemaName gate would have skipped. No regression.

Local runs: 15 targeted (test_agent_handoff.py), 1011 full flightcheck suite — all passing; ruff clean on both changed files.

Fixes #152

[daeunJe0ng]

Code review — TOPIC-020 handoff agent id check

Verdict: Approve with nits

Reviewed the new check, the get_dialog_components client method, the CLI wiring, and the test module against the cardinal rules in flightcheck/AGENTS.md and tests/AGENTS.md.

What holds up well:

• get_dialog_components(bot_id) mirrors get_knowledge_sources exactly (same endpoint/headers/extraction) — reuses an already-registered API tier, no new grounding surface.
• Gating is correct and fails safe: returns [] when PVA is absent/unconfigured, bot id is missing, no enabled (Active) auto-handoff topic exists, or the client raises.
• CheckResult fields and the Status / Priority / Role enum values all match runner.py.
• Tests are cassette-grounded (validated tier, not the banned placeholder tier): every mock field traces to island_gateway_botcomponents.yaml, and only the two real captured values (status, expressionText) are varied. Gating / good / placeholder / blank / missing-SetVariable / multi-topic indexing are all covered.

One nit inline (non-blocking) on the FAILED-branch message. No blocking or should-fix bugs found.

[daeunJe0ng]

Addressed the code-review nit in 8135710: the FAILED result text now branches on the failure mode. A blank Topic.HandoffAgentId is reported as blank; the placeholder case still names AgentIdentifier. Strengthened the blank-value test to assert the accurate wording. Targeted tests pass (12 passed).

[apurvabanka]

Reviewed PR #218 — checked out the branch, read the check, the pva_client additions, the tests, the cassette fixture, and the CLI wiring; the 15 targeted tests pass locally. It's a solid, well-documented check. Wiring matches the existing graph_connector_kb pattern exactly, and identity-by-SetVariable (not schemaName) is the right call. Here are the gaps I'd flag, roughly by materiality:

1. The status == "Active" gate has no automated guard (self-admitted). Every "enabled" test manually flips the captured Inactive→Active; no cassette holds a real enabled topic. If Copilot Studio ever represents an enabled handoff topic differently (separate publish state, status absent on a draft component, etc.), run_handoff_topic_checks silently returns [] and #152 is never caught in production — a false negative with no test to catch the drift. The live E2E run mitigates this today, but nothing pins the behavior going forward. Worth a captured enabled cassette when one is available.

2. Non-literal HandoffAgentId assignments → false-positive FAILED. _resolved_agent_id only reads expressionText. The cassette itself shows sibling SetVariables using variableReference (e.g. System.Activity.Text) with no expressionText. If a maker sets the handoff id via a Power Fx variable/reference/formula instead of a string literal, expressionText is "" → the check reports "blank (no target agent id set)" and fails a correctly-configured topic. No test covers a variableReference-style value. This is the most likely real-world false positive.

3. get_dialog_components (43 new lines) has no automated test. The endpoint + botComponentChanges → component → $kind==DialogComponent filtering is exercised only through the fake PVA in the check tests; no cassette-backed test asserts the parser reads the real fixture. It mirrors get_knowledge_sources (also untested), so no regression — but the new parsing is unverified.

Minor:

• Placeholder match breadth: _is_placeholder_id fails any id ending in agentidentifier (case-insensitive). Safe against prefixed-GUID ids, but technically over-broad. Low risk.

• Duplicate POST: get_knowledge_sources and get_dialog_components hit the identical botcomponents endpoint separately, so a full run makes two identical calls. Inefficiency, not correctness.

• Doc gap: the new handoff scope isn't added to the cli.py module docstring "Scopes:" list (argparse choices still work since they derive from SCOPE_MAP).

• First-match only: the recursive search returns the first HandoffAgentId SetVariable; conditional branches assigning it elsewhere aren't considered. Edge case.

CC: @daeunJe0ng