fix(tests): report a broken tenant as ERROR, not as an agent failure - #3090
Conversation
On 2026-09-04 two flow tasks were scored FAILURE for reasons that had
nothing to do with the agent:
skill-flow-outlook-trigger-inbox AADSTS50173 — the Outlook grant was
revoked on 2026-08-31
skill-flow-generic-dynamic-node the ServiceNow developer instance was
hibernating, so every metadata call 403'd
Both were root-caused as skill defects first. The 403 is several thousand
characters into the checker output, and nothing above it says the tenant
is down, so the reports read as "the agent wrote a display name where an
ID belongs" — which it did, but only because the lookup it was told to
use could not answer.
A `pre_run` failure lands the run as FinalStatus.ERROR rather than
FAILURE, and PreRunCommand's own docstring gives the reason: "the agent
should not run against a broken environment." This uses that.
preflight_connections.py takes connector keys, lists their connections
with --all-folders (without it an empty result is a false negative), and
exits non-zero unless at least one is Enabled. Wired into the two tasks
above.
Behaviour verified against six tenant states: all-enabled, hibernating,
mixed with one up, no connections in any folder, CLI non-zero, and a
Failure envelope.
Only those two tasks are wired. 23 others name a connector, but that list
comes from grepping prompts for `uipath-*` and it catches non-connectors
like uipath-rpa and uipath-platform — worth doing deliberately, not from
a crude match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @rockymadden's task in 2m 8s —— View job SummaryAdds a Change-by-Change Review1.
|
| Area | Status | Notes |
|---|---|---|
| Frontmatter | N/A | No skill frontmatter changed |
| E2E Tests | OK | Existing tasks unchanged; pre_run is additive |
| Skill Body | N/A | No skill body changed |
| References & Assets | N/A | No references or assets changed |
| Repo Hygiene | OK | No secrets, no cross-skill deps, $SKILLS_REPO_PATH consistent with codebase |
Issues for Manual Review
- Confirm that
uip is connections list <key> --all-folders --output jsonreturns the expected{"Result": "Success", "Data": [...]}envelope in the CI runner's tenant. The script's parsing depends on this shape. - The PR scopes to only 2 of 23 connector-dependent tasks. The PR description explains why (the grep catches non-connectors), but the other connector tasks will still report FAILURE on tenant issues. Worth a follow-up pass with a curated connector list.
Conclusion
Clean, well-motivated change. The script handles edge cases correctly, the YAML additions are minimal and correctly placed, and the PR is well-scoped to the two tasks with evidence. The missing unit test file is the only gap — the established _shared/test_*.py pattern makes this a natural addition. Approve with a suggestion to add test_preflight_connections.py in a follow-up or this PR.
|
Claude finished @rockymadden's task in 2m 52s —— View job Coder-eval task lint (advisory)2 task YAMLs modified; 0 Critical, 0 High, 0 Medium, 1 Low, 1 OK. ( Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge. Evidence of passing run❌ High — PR body does not claim the modified tasks have been run through coder-eval and passed. "775 pytest pass" covers the preflight script's unit tests, not a coder-eval run of the two wired tasks. Since the change is additive (
|
There was a problem hiding this comment.
🟡 Changes recommended
The preflight can accept unhealthy or wrong-folder connections, use stale results, and lacks committed regression tests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds tenant-connection preflight checks so two Flow evaluations report infrastructure outages as ERROR instead of agent failures.
Changes:
- Adds a shared Integration Service connection checker.
- Wires it into the Outlook and ServiceNow Flow tasks.
- Provides actionable tenant-remediation output.
File summaries
| File | Description |
|---|---|
_shared/preflight_connections.py |
Checks connection state before evaluation. |
outlook_trigger_inbox.yaml |
Adds Outlook preflight. |
generic_dynamic_node.yaml |
Adds ServiceNow preflight. |
Review details
Suppressed comments (1)
tests/tasks/uipath-maestro-flow/_shared/preflight_connections.py:60
- Do not report success from list state alone. Integration Service discovery requires
uip is connections ping <id>after selecting an Enabled candidate to establish current health; otherwise this preflight can printOKwithout testing the revoked grant/provider availability it was introduced to detect. Ping the eligible candidates and pass only when at least one health check succeeds and reports Enabled.
print(f"OK: {key} — {len(enabled)}/{len(conns)} connection(s) Enabled")
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import sys | ||
|
|
||
|
|
||
| def _connections(key: str) -> list[dict]: |
|
|
||
| def _connections(key: str) -> list[dict]: | ||
| proc = subprocess.run( | ||
| ["uip", "is", "connections", "list", key, "--all-folders", "--output", "json"], |
| enabled = [c for c in conns if c.get("State") == "Enabled"] | ||
| if not enabled: | ||
| states = ", ".join(f"{c.get('Name')}={c.get('State')}" for c in conns) | ||
| broken.append(f"{key}: no Enabled connection ({states})") |
…ero-shot flow task (#3088) In the 2026-09-04 nightly, 5 of 8 `skill-flow-*` tasks built and validated a flow, reported success, and never executed it. The checker then ran `flow debug` and found a null End-node output mapping, a faulted script, and an empty result. `flow validate` had passed on all of them. Every one of those prompts contained "Do NOT ask for approval, confirmation, or feedback". That phrasing forbids asking. It does not say nobody is there to ask, so an agent can honor it and still stop at a consent gate waiting for a reply that never arrives. Measured across the 128-task suite: 0 tasks said the run was headless, 51 of the 119 non-simulated tasks said nothing about autonomy at all, and the 68 that did were spread across 9 wording variants — the ninth found by the guard added here, not by the sweep. Task prompts, not an experiment config: flow tasks run under nightly.yaml via daily.sh in coder_eval_uipath, smoke.yaml on every PR, default.yaml locally, and whatever a dispatch selects. coder_eval has no pattern-scoped defaults, so a config carrying this would either miss those runners or reach the 319 simulated tasks of every other skill. Keeping it in the prompt makes it travel with the task and work everywhere today, with no cross-repo change. - One canonical paragraph on all 119 zero-shot tasks. It states that nobody is present, that the task's implied actions are authorized including tenant writes, and the two things to hold back on: do not delete or overwrite what this run did not create, and exhaust the documented resolution path before giving up on a lookup. - _shared/test_headless_preamble.py enforces presence, identical wording, absence on the 9 simulated tasks, and that no superseded variant comes back. Each guard verified by breaking it. - test-task-template.yaml points authors at the canonical block instead of telling them to hand-write autonomy language. Skill side, both true with a user watching: - `flow debug` consent comes from the mandate. A request to build something that does X is a request for it to work. Debug also overwrites the Studio Web solution behind the local .uipx SolutionId, so never debug a solution this run did not scaffold. - "Publish to Studio Web" is no longer marked `(default)` in either What's next dropdown — rule #5's non-interactive fallback takes the marked option, which would have auto-published to a tenant. Both menus also stopped gating Debug on consent, which contradicted rule #2. Modifies Critical Rules 2 and 5, per CONTRIBUTING. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): cut the docstring, repair a dead assertion, cache the file reads Self-review of the new guard. The module docstring was 24 lines restating the commit message and PR body — the nightly counts, the variant tally, the history. Two facts there are load-bearing for anyone editing the file: why the text is in the prompt rather than a config, and why this parses with regex instead of PyYAML. The rest went. Prose is now 14 of 83 lines, from 30 of 101. The presence check built a `needles` list from every line of the canonical block, then discarded it except for the first sentence, and ran `all()` over the resulting one-element list. It only ever checked "This run is headless." — which is the right cheap marker, since the wording test covers the full text, but the code said something else. Now it says what it does. Four tests each re-read every file in the suite; `_tasks()` is cached. All four guards re-proven after the rewrite: removing the preamble, rewording it, leaking it into a simulated task, and reintroducing a superseded variant each fail the expected test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(tests): leave the task template alone Out of scope. The template still tells authors to hand-write autonomy language, which the canonical preamble supersedes, but changing it is a separate call from fixing the flow suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): the task's own instructions outrank the shared preamble Several tasks say "Do NOT run or debug the flow — the grader executes it with the seeded inputs", and one (skill-flow-eval-no-auto-upload) deliberately asserts a refusal. The preamble says the actions a task implies are authorized, which sits against those without saying which wins. Added the precedence sentence to the canonical block and all 119 tasks. The test constant moved in the same commit, so the guard and the tasks cannot disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: address Copilot review — a 10th variant, an unconditional menu, unbounded consent A 10th autonomy variant survived the sweep: five billing tasks carried "Build the complete flow / without stopping to ask me." The guard advertised catching superseded variants and did not match that wording, so it passed with the duplication in place. Removed from the five, and the matcher now covers it — verified by reintroducing the phrase and watching the test fail. devcon_expense_approval's "Build the complete flow as a UiPath Flow project called ..." is task text and is untouched. Both What's next menus were unconditional, so a request that already named the next step ("run debug and iterate") still stopped for a redundant selection — the exact gate this work removes elsewhere. An instruction in the original request is now the selection; the menu shows only when the next step was unspecified. Rule #2's mandate said an ordinary build request authorizes a real run. That is too broad for side effects reaching someone who is not the user: a phone call, an email to a real person. Those now need the run asked for explicitly, whoever is watching, and rule #2 points at the outbound-call case in inline-voice-agent/impl.md, whose own gate this never touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): report a broken tenant as ERROR, not as an agent failure (#3090) On 2026-09-04 two flow tasks were scored FAILURE for reasons that had nothing to do with the agent: skill-flow-outlook-trigger-inbox AADSTS50173 — the Outlook grant was revoked on 2026-08-31 skill-flow-generic-dynamic-node the ServiceNow developer instance was hibernating, so every metadata call 403'd Both were root-caused as skill defects first. The 403 is several thousand characters into the checker output, and nothing above it says the tenant is down, so the reports read as "the agent wrote a display name where an ID belongs" — which it did, but only because the lookup it was told to use could not answer. A `pre_run` failure lands the run as FinalStatus.ERROR rather than FAILURE, and PreRunCommand's own docstring gives the reason: "the agent should not run against a broken environment." This uses that. preflight_connections.py takes connector keys, lists their connections with --all-folders (without it an empty result is a false negative), and exits non-zero unless at least one is Enabled. Wired into the two tasks above. Behaviour verified against six tenant states: all-enabled, hibernating, mixed with one up, no connections in any folder, CLI non-zero, and a Failure envelope. Only those two tasks are wired. 23 others name a connector, but that list comes from grepping prompts for `uipath-*` and it catches non-connectors like uipath-rpa and uipath-platform — worth doing deliberately, not from a crude match. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stacked on #3088 — both touch
outlook_trigger_inbox.yamlandgeneric_dynamic_node.yaml, so this is based on that branch rather thanmain. Retarget tomainonce #3088 lands.Problem
On 2026-09-04 two flow tasks were scored FAILURE for reasons that had nothing to do with the agent:
skill-flow-outlook-trigger-inboxAADSTS50173— the Outlook grant was revoked on 2026-08-31skill-flow-generic-dynamic-nodeBoth got root-caused as skill defects first. The 403 sits several thousand characters into the checker output and nothing above it says the tenant is down, so the reports read as "the agent wrote a display name where an ID belongs" — which it did, but only because the lookup it was told to use could not answer.
That is 2 of the 8 failures in that nightly reporting the wrong thing, and it cost real time before anyone read far enough to find the 403.
Fix
A
pre_runfailure lands the run asFinalStatus.ERRORrather thanFAILURE, andPreRunCommand's own docstring gives the reason:_shared/preflight_connections.pytakes connector keys, lists their connections with--all-folders(without it an empty result is a false negative), and exits non-zero unless at least one is Enabled. On failure it says plainly that this is an environment problem and what to do:Verification
Behavior checked against six tenant states, each asserted:
State: Failed)Result: Failureenvelope775 pytest pass. The two wired tasks keep their prompt and criteria unchanged — only
pre_runis added.Scope
Only the two tasks with evidence. 23 others name a connector in their prompt, but that list comes from grepping for
uipath-*and it catches non-connectors likeuipath-rpaanduipath-platform. Widening it is worth doing deliberately, not from a crude match — left as a follow-up.What this does not do
It does not make either task pass. Both still need a working tenant. It makes the report tell the truth, so the next person triaging a nightly does not spend an hour proving the skill is fine.
🤖 Generated with Claude Code