From d85c30b19da97772372b58cf81adf16927d3b7cd Mon Sep 17 00:00:00 2001 From: Andrei Petraru Date: Thu, 10 Sep 2026 16:32:09 +0300 Subject: [PATCH 1/4] fix(uipath-review): read the newest review-history entry, not the oldest `check_review_history.py` took `history[-1]`, and its docstring claimed the CLI "appends". The CLI's own help says the opposite: uip agent review-history add -- "Keeps the newest 25 entries, most recent first." So `history[0]` is the newest and `history[-1]` is the oldest. With a single entry the two coincide, which is why this shipped green; it bites the moment a reviewer records twice. Run 2026-09-10_04-18-49, `skill-review-agents-lowcode-guardrail-pii-missing`: the reviewer recorded a provisional grade C, finished the judgment analysis, then recorded the corrected final grade D. The file the CLI wrote was [ {"grade": "D", "runAt": "...04:36:37Z"}, <- newest, matches the report {"grade": "C", "runAt": "...04:29:48Z"} ] <- oldest and the checker compared the report's final grade D against the C, failing a correct artifact. Verified against that exact artifact: FAIL before, PASS after, and a newest entry that genuinely disagrees still FAILs. Co-Authored-By: Claude Opus 5 (1M context) --- .../_shared/check_review_history.py | 23 +++++++++++-------- .../_shared/test_check_review_history.py | 23 ++++++++++++++++--- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/tests/tasks/uipath-review/_shared/check_review_history.py b/tests/tasks/uipath-review/_shared/check_review_history.py index f3008ae7c0..9df48a23b0 100644 --- a/tests/tasks/uipath-review/_shared/check_review_history.py +++ b/tests/tasks/uipath-review/_shared/check_review_history.py @@ -6,11 +6,15 @@ uip agent review-history add "" --errors --warnings --output json -which appends `{grade, runAt, errors, warnings}` to -`/review-history.json`. This checker grades the OUTCOME (the file -the CLI wrote) rather than command telemetry, for the reasons documented in -`check_review_cli_provenance.py`: batched shell scripts make `command_executed` -criteria blind to both the call and its exit code. +which writes `{grade, runAt, errors, warnings}` to +`/review-history.json`. Per the CLI's own help -- "Keeps the newest +25 entries, most recent first" -- the array is ordered NEWEST FIRST, so the entry +to grade is `history[0]`, not `history[-1]`. That distinction is invisible while +a project has a single entry and bites the moment a reviewer records twice (e.g. +recording a provisional grade, then correcting it). This checker grades the +OUTCOME (the file the CLI wrote) rather than command telemetry, for the reasons +documented in `check_review_cli_provenance.py`: batched shell scripts make +`command_executed` criteria blind to both the call and its exit code. The recorded grade must equal the report's `**Final grade: **` footer -- Step 6 persists the Step 4.5 final grade, not some other letter. @@ -107,13 +111,14 @@ def main() -> None: sys.exit(f"FAIL: {history_path} is not valid JSON: {error}") if not isinstance(history, list) or not history: sys.exit(f"FAIL: {history_path} must be a non-empty JSON array of review entries") - entry = history[-1] + # Newest first, per the CLI's help text -- NOT append order. + entry = history[0] if not isinstance(entry, dict): - sys.exit(f"FAIL: last review-history entry must be a JSON object, got {entry!r}") + sys.exit(f"FAIL: newest review-history entry must be a JSON object, got {entry!r}") grade = entry.get("grade") if grade not in GRADES: - sys.exit(f"FAIL: last review-history entry has invalid grade {grade!r}") + sys.exit(f"FAIL: newest review-history entry has invalid grade {grade!r}") if grade != final_grade: sys.exit( f"FAIL: recorded grade {grade} does not match the report's final grade " @@ -123,7 +128,7 @@ def main() -> None: warnings = _int_field(entry, "warnings") run_at = entry.get("runAt") if not isinstance(run_at, str) or not run_at.strip(): - sys.exit(f"FAIL: last review-history entry has no `runAt` timestamp, got {run_at!r}") + sys.exit(f"FAIL: newest review-history entry has no `runAt` timestamp, got {run_at!r}") print( f"OK: {history_path} records grade {grade} (errors={errors}, warnings={warnings}, " diff --git a/tests/tasks/uipath-review/_shared/test_check_review_history.py b/tests/tasks/uipath-review/_shared/test_check_review_history.py index c423fdec66..161392b08e 100644 --- a/tests/tasks/uipath-review/_shared/test_check_review_history.py +++ b/tests/tasks/uipath-review/_shared/test_check_review_history.py @@ -74,11 +74,28 @@ def test_passes_when_recorded_grade_matches_final_grade(tmp_path): assert "records grade C" in r.stdout -def test_last_entry_wins_when_history_has_multiple_entries(tmp_path): - older = {**ENTRY, "grade": "F", "runAt": "2026-09-01T00:00:00.000Z"} - _write_history(tmp_path, [older, ENTRY]) +def test_newest_entry_wins_when_history_has_multiple_entries(tmp_path): + """`uip agent review-history add` writes "most recent first", so the entry to + grade is `history[0]`. A reviewer who records a provisional grade and then + corrects it leaves the superseded one behind at the END of the array; reading + that one failed a correct artifact (run 2026-09-10_04-18-49). + """ + superseded = {**ENTRY, "grade": "F", "runAt": "2026-09-01T00:00:00.000Z"} + _write_history(tmp_path, [ENTRY, superseded]) r = run(tmp_path) assert r.returncode == 0, r.stderr + assert "records grade C" in r.stdout + + +def test_fails_when_the_newest_entry_is_the_one_that_disagrees(tmp_path): + """The reverse ordering must still fail -- reading `history[0]` is not a way + of finding *some* entry that matches the report. + """ + stale_but_matching = {**ENTRY, "runAt": "2026-09-01T00:00:00.000Z"} + _write_history(tmp_path, [{**ENTRY, "grade": "F"}, stale_but_matching]) + r = run(tmp_path) + assert r.returncode != 0 + assert "recorded grade F does not match" in r.stderr def test_fails_when_recorded_grade_differs_from_final_grade(tmp_path): From 02668f6575ae873e7b47a1df390682c933c6eb89 Mon Sep 17 00:00:00 2001 From: Andrei Petraru Date: Thu, 10 Sep 2026 18:29:15 +0300 Subject: [PATCH 2/4] fix(tests): mock BYOG discovery for the coded guardrail tasks `byog_middleware` and `byog_decorator` asked the agent to pin a bring-your-own guardrail named `byog-smoke-agent-pin`, and relied on that configuration existing on the shared smoke tenant. Neither task has a `pre_run` that creates it; the descriptions simply asserted "the tenant already has a BYOG configuration registered (admin-side)". The fixture drifted away and the tasks became a test of tenant state: 2026-08-24 tenant held byog-smoke-pii, byog-harmful-content, cli-harmful-content-1 -- none named byog-smoke-agent-pin 2026-09-10 both discovery calls returned Data: [] uip agent guardrails list --byo -> {"Code":"GuardrailDefinitionsList","Data":[]} uip guardrails byo-configurations list -> {"Code":"ByoGuardrailConfigurationsList","Data":[]} The agent then correctly refused to fabricate a validator name and changed nothing, scoring 0.00 (run 2026-09-10_04-17-44) -- a fixture outage rendered as a skill regression. `byog_decorator` carried the same defect, hidden because it was skipped that run and carried forward as a green 1.00. Seeding per-run is not an option: `byo-configurations create` probes the connection server-side with no skip flag, and the smoke tenant has no guardrail-capable connection. That is why the low-code sibling `byog_pinning` already mocks discovery; this applies the same treatment to the coded half of the family, from one shared shim. The shim serves BOTH verbs a coded agent walks -- `agent guardrails list [--byo]` and `guardrails byo-configurations list` -- where byog_pinning's serves only the first. Verified necessary: the decorator run hits each exactly once. Without `--byo`, `list` also returns the built-in pii_detection entry, so the agent must still disambiguate via IsByo/ByoValidatorName. `--help` and every other verb go to the real CLI. Grading is untouched: both checkers are pure AST analysis of graph.py and never contacted a tenant. Only discovery needed one. Validated locally with codex + gpt-5.6-terra (the harness that failed), `--driver tempdir`: byog_middleware SUCCESS 1.000, byog_decorator SUCCESS 1.000. Co-Authored-By: Claude Opus 5 (1M context) --- .../_fixtures/ByogMockCli/mocks/uip | 176 ++++++++++++++++++ .../byog_decorator/byog_decorator.yaml | 13 +- .../byog_middleware/byog_middleware.yaml | 13 +- 3 files changed, 196 insertions(+), 6 deletions(-) create mode 100755 tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip diff --git a/tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip b/tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip new file mode 100755 index 0000000000..cdb606f15f --- /dev/null +++ b/tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +"""Selective `uip` shim for the CODED BYOG guardrail tasks. + +Shared by `byog_middleware` and `byog_decorator`. Serves the two read-only +discovery paths a coded agent walks before it can pin a bring-your-own +guardrail, both answering with the configuration the prompts name +(`byog-smoke-agent-pin`, pii_detection): + + * `uip agent guardrails list [--byo]` -> GuardrailDefinitionsList + * `uip guardrails byo-configurations list` -> ByoGuardrailConfigurationsList + +Without `--byo`, `agent guardrails list` also returns the BUILT-IN +`pii_detection` entry sharing the same `Validator` name, so the agent must +disambiguate via `IsByo`/`ByoValidatorName` exactly as the skill teaches +rather than grabbing the first PII row. The two views agree on ids and +connector metadata, so cross-checking one against the other is consistent. + +Every other invocation exec's the REAL `uip` found later on PATH, and +`--help` is handed to the real CLI so flag questions get real answers +(a canned payload there reads as "that flag does not exist"). + +Why mock: the graded behavior is code wiring in `graph.py`, and both +checkers are pure AST analysis that never touch a tenant -- only the +agent's DISCOVERY step needed one. It could not keep depending on the +shared smoke tenant. `byo-configurations create` probes the connection +server-side with no skip flag, so a configuration cannot be seeded +per-run, and the tenant has no guardrail-capable connection to bind +(same reasoning as the byog_pinning mock, which fixed the low-code half +of this family). Left live, these tasks were graded on whether an +external tenant happened to hold a fixture: on 2026-08-24 the tenant +held three configurations, none of them `byog-smoke-agent-pin`; by +2026-09-10 it held none at all, and both discovery calls returned +`Data: []`. The agent then correctly refused to fabricate a validator +name and changed nothing, scoring 0.00 -- a fixture outage rendered as a +skill regression. + +`mock_path_dirs: [mocks]` PATH-prepends this script's directory inside the +sandbox. Linux smoke only (no `windows` tag on either task), hence no +`uip.cmd` twin -- same as the ixp, byog_pinning and platform guardrails mocks. +""" + +import json +import os +import shutil +import sys + +MOCK_DIR = os.path.dirname(os.path.abspath(__file__)) + +VALIDATOR_NAME = "byog-smoke-agent-pin" +CONNECTION_ID = "18fb337c-29b7-4162-a9e8-0c05b01cf4df" +CONFIGURATION_ID = "e5723bb8-fbc2-4317-c7d7-08de803bc010" +CONNECTOR_NAME = "Azure AI Content Safety" +CONNECTOR_KEY = "uipath-azure-contentsafety" + +PII_PARAMETERS = [ + { + "Type": "enum-list", + "Id": "entities", + "Required": True, + "DefaultValue": ["Email", "PhoneNumber"], + "Options": [ + "Email", + "PhoneNumber", + "Person", + "Address", + "USSocialSecurityNumber", + ], + "DisplayName": "Entities", + "Description": "PII entity types to detect.", + }, + { + "Type": "map-enum", + "Id": "entityThresholds", + "Required": True, + "DefaultValue": {"Email": 0.5, "PhoneNumber": 0.5}, + "KeySource": "entities", + "Min": 0, + "Max": 1, + "DisplayName": "Per-entity threshold", + "Description": "Confidence threshold (0-1) per selected entity.", + }, +] + +BYO_ENTRY = { + "Validator": "pii_detection", + "IsByo": True, + "Status": "Available", + "AllowedScopes": ["Agent", "Llm", "Tool"], + "GuardrailStages": { + "Agent": ["PreExecution", "PostExecution"], + "Llm": ["PreExecution", "PostExecution"], + "Tool": ["PreExecution", "PostExecution"], + }, + "DisplayName": "PII Detection", + "Description": ( + "Detects personally identifiable information. Served by the " + "tenant's external bring-your-own guardrail provider." + ), + "ByoValidatorName": VALIDATOR_NAME, + "ByoConnectionId": CONNECTION_ID, + "ByoConfigurationId": CONFIGURATION_ID, + "ByoConnectorName": CONNECTOR_NAME, + "ByoConnectorKey": CONNECTOR_KEY, + "FolderKey": "627fe423-5c73-464a-abff-41fdaad6ac19", + "Parameters": PII_PARAMETERS, +} + +BUILTIN_ENTRY = { + "Validator": "pii_detection", + "IsByo": False, + "Status": "Available", + "AllowedScopes": ["Agent", "Llm", "Tool"], + "GuardrailStages": { + "Agent": ["PreExecution", "PostExecution"], + "Llm": ["PreExecution", "PostExecution"], + "Tool": ["PreExecution", "PostExecution"], + }, + "DisplayName": "PII Detection", + "Description": ( + "Detects personally identifiable information using Azure " + "Cognitive Services (UiPath built-in)." + ), + "Parameters": PII_PARAMETERS, +} + +# The admin-side view of the same record. Field names mirror the CLI's own +# documented examples for `guardrails byo-configurations`, so the fields the +# skill teaches are the fields that come back. +CONFIGURATION = { + "Id": CONFIGURATION_ID, + "ConnectionId": CONNECTION_ID, + "ValidatorName": VALIDATOR_NAME, + "ValidatorType": "pii_detection", + "FallbackOnUiPath": False, + "Enabled": True, + "CreatedAt": "2026-07-02T11:40:00Z", + "UpdatedAt": None, + "ConnectorKey": CONNECTOR_KEY, + "ConnectorName": CONNECTOR_NAME, + "ConnectionName": f"{CONNECTOR_NAME} - Prod", + "ValidConnection": True, +} + + +def real_uip(): + path = os.environ.get("PATH", "") + parts = [p for p in path.split(os.pathsep) if p and os.path.abspath(p) != MOCK_DIR] + return shutil.which("uip", path=os.pathsep.join(parts)) + + +def emit(code, data): + print(json.dumps({"Result": "Success", "Code": code, "Data": data}, indent=2)) + return 0 + + +def main(): + args = sys.argv[1:] + literal = [a for a in args if not a.startswith("-")] + + # A help request is a question about flags, not about tenant data. + if "--help" not in args and "-h" not in args: + if literal[:3] == ["agent", "guardrails", "list"]: + data = [BYO_ENTRY] if "--byo" in args else [BUILTIN_ENTRY, BYO_ENTRY] + return emit("GuardrailDefinitionsList", data) + if literal[:3] == ["guardrails", "byo-configurations", "list"]: + return emit("ByoGuardrailConfigurationsList", [CONFIGURATION]) + + real = real_uip() + if not real: + sys.stderr.write("uip (shim): real uip CLI not found on PATH\n") + return 127 + os.execv(real, [real] + args) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml b/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml index 4a5c53fc5e..6e90ba21ae 100644 --- a/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml +++ b/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml @@ -2,9 +2,10 @@ task_id: skill-agent-guardrail-coded-byog-decorator description: > Decorator-style bring-your-own guardrail (BYOG) for a coded agent. A simple LangGraph customer support agent is seeded with no guardrails; it already has a - create_support_agent() factory. The tenant has a BYOG configuration registered - (admin-side); the user asks to route the agent's PII checks through it using the - decorator style. Verifies the agent stacks @guardrail with ByoValidator — pinned + create_support_agent() factory. A mocked `uip` serves the BYOG configuration the + prompt names, so discovery answers the same way on every run; the user asks to + route the agent's PII checks through it using the decorator style. + Verifies the agent stacks @guardrail with ByoValidator — pinned to the configuration by its validator name, blocking on violations — over a real factory function rather than reaching for the built-in PII validator. tags: [uipath-agents, e2e, coded, mode:build, lifecycle:edit, guardrail] @@ -21,7 +22,13 @@ agent: disallowed_tools: ["Task"] sandbox: + # Discovery is mocked, so the task no longer depends on a fixture living on the + # shared smoke tenant. See _fixtures/ByogMockCli/mocks/uip for why that + # dependency was untenable and what the shim serves. + mock_path_dirs: [mocks] template_sources: + - type: template_dir + path: ../_fixtures/ByogMockCli - type: template_dir path: ../_fixtures/SimpleCodedAgent diff --git a/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml b/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml index c3ce3525db..5a7152fc1a 100644 --- a/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml +++ b/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml @@ -1,9 +1,10 @@ task_id: skill-agent-guardrail-coded-byog-middleware description: > Middleware-style bring-your-own guardrail (BYOG) for a coded agent. A simple - LangGraph customer support agent is seeded with no guardrails. The tenant already - has a BYOG configuration registered (admin-side); the user asks to route the - agent's PII checks through it. Verifies the agent wires + LangGraph customer support agent is seeded with no guardrails. A mocked `uip` + serves the BYOG configuration the prompt names, so discovery answers the same + way on every run; the user asks to route the agent's PII checks through it. + Verifies the agent wires UiPathByoGuardrailMiddleware — pinned to the configuration by validator_name, spread into create_agent(), with a block action — rather than reaching for the built-in PII validator. @@ -21,7 +22,13 @@ agent: disallowed_tools: ["Task"] sandbox: + # Discovery is mocked, so the task no longer depends on a fixture living on the + # shared smoke tenant. See _fixtures/ByogMockCli/mocks/uip for why that + # dependency was untenable and what the shim serves. + mock_path_dirs: [mocks] template_sources: + - type: template_dir + path: ../_fixtures/ByogMockCli - type: template_dir path: ../_fixtures/SimpleCodedAgent From 2ebfac0c574749cffb4a6e53ccec5312e1ff79fc Mon Sep 17 00:00:00 2001 From: Andrei Petraru Date: Thu, 10 Sep 2026 18:29:31 +0300 Subject: [PATCH 3/4] fix(tests): grade byog-delete on the breakage claim the prompt asks for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `llm_judge` criterion required the response to convey two things for 1.0: that referencing guardrails will break, AND that they need updating or repointing. The prompt only asks the first — "Remove it, and tell me what happens to any agent still pointing at it." Nothing asks what to do about them. Run 2026-09-10_04-17-44 hit exactly that gap. The agent answered precisely: Any agent still referencing `ByoValidator("byog-legacy-harmful")` (or its low-code equivalent) will fail its guardrail check at runtime because the validator name no longer resolves. The 0.0 condition plainly did not apply, and the 1.0 condition was not fully met, so the judge returned 0.9 — a value the rubric never defines. Against `pass_threshold: 1.0` that failed the task at a weighted 0.984, with the other four criteria all at 1.0. Two changes: drop the remediation conjunct so the criterion grades what the user asked (naming a fix stays a bonus, never a deduction), and state the scale explicitly. A binary rubric paired with a 1.0 threshold is only sound if the judge cannot return an intermediate value. Not exercised locally: `experiments/default.yaml` routes the judge through LiteLLM with `api_base: CODEX_BASE_URL` / `api_key: CODEX_API_KEY`, so every judge-bearing task ERRORs here for want of those two vars, in either harness. `coder-eval plan` validates the config; the rubric text is otherwise unverified until CI. Co-Authored-By: Claude Opus 5 (1M context) --- .../uipath-platform/guardrails/delete_smoke.yaml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml b/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml index c6b19f4146..f8d48955f1 100644 --- a/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml +++ b/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml @@ -78,16 +78,22 @@ success_criteria: (`byoValidatorName`) or coded (`ByoValidator("byog-legacy-harmful")`) — that still references the deleted configuration by that validator name will fail at runtime, because the platform can no longer resolve the name - to a connection. The fix is to remove or repoint those guardrails (or - re-create a configuration under the same ValidatorName). + to a connection. Score 1.0 if the response conveys that agents/guardrails still referencing the configuration will now break or fail (at runtime, on their next run, when the guardrail is evaluated — any equivalent - wording), and that they need updating or repointing. + wording). Score 0.0 if it claims nothing breaks, that the guardrails silently fall back to the UiPath built-in check, that they keep working, or if it says nothing at all about the effect on referencing agents. + + Grade ONLY the breakage claim above. The user asked what happens, not what + to do about it, so naming a remediation (remove, repoint, re-create under + the same ValidatorName) is a bonus and its absence is NOT a deduction. + + Return exactly 1.0 or 0.0. This rubric has no partial credit, and the + criterion's pass_threshold is 1.0 — an intermediate score fails the task. weight: 1.5 pass_threshold: 1.0 From 2add28f58537692f84cd1e37515b5a0021e38b49 Mon Sep 17 00:00:00 2001 From: Andrei Petraru Date: Sat, 12 Sep 2026 13:11:26 +0300 Subject: [PATCH 4/4] fix(tests): address review findings on the guardrail nightly fixes Follow-up to the three fixes in this PR, from @rockymadden's review. `tests/tasks/uipath-review/` ran in no CI job (major). test-helpers.yml has pytest jobs for maestro-bpmn, maestro-flow, maestro-case, agents, planner and admin/_shared -- not review. So its three suites, including the ordering guard this PR just wrote, only ever ran by hand. That is the same failure mode as the bug itself: #3123 shipped a checker whose test encoded the wrong ordering model and nothing caught it. Adds `pytest-review-check` mirroring the sibling jobs, plus the `docs/REQUIRED-CHECKS.md` row the contract guard requires. The rubric scale sentence handed the judge a reason to round up. "the criterion's pass_threshold is 1.0 -- an intermediate score fails the task" tells the judge the CONSEQUENCE of its score, so a genuinely borderline judge has one option that "fails the task" and one that does not -- trading this PR's false negative for a false positive. Replaced with the wording three uipath-admin audit rubrics already use, which states the scale and gives a tiebreak procedure instead: "Score EXACTLY 1.0 or EXACTLY 0.0 ... If the case feels borderline, decide which of the two descriptions above fits better." The same latent defect was live in two siblings: create_configuration_smoke and probe_abort_smoke are both prose "Score 1.0 if / Score 0.0 if" with `pass_threshold: 1.0` and no scale statement, so both could return the same 0.9 that failed byog-delete. Both now carry the scale line. Nothing graded the discovery the BYOG mock exists to serve. Both checkers are pure AST over graph.py and the prompt already names `byog-smoke-agent-pin`, so they passed whether the agent discovered the configuration or copied the string out of the prompt -- leaving the mock as ungraded infrastructure, and coded Rule 18 (read ByoValidatorName from discovery, never from memory) untested. Adds the `byog_pinning`-style `command_executed` criterion to byog_middleware and byog_decorator, matching either discovery verb. The shim was a second copy of byog_pinning's payload, and the twin was the weaker one -- it serves only `agent guardrails list`, so a low-code agent following the ByoConfigurationId cross-reference fell through to the real CLI and got `Data: []`, the exact outage this PR fixes on the coded half; it also answers `--help` with a canned payload. Hoisted to `tests/tasks/uipath-agents/_fixtures/ByogMockCli` and pointed all three tasks at it, deleting byog_pinning's local copy. One payload, so a CLI field rename is a one-file edit. Validation: - byog_pinning SUCCESS 1.000 on the shared fixture, all four criteria including its own `uip agent guardrails list` discovery criterion. - Shim verified inside `skills-image:latest` under a PATH prepend: resolves to the shim, both verbs answer, passthrough to the real uip works. A full docker-driver task run is still unverified -- the local image bakes coder_eval 0.8.8 and rejects a 0.12.0 task config -- so the reviewer's open item is closed at the mechanism, not end to end. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-helpers.yml | 16 +++ docs/REQUIRED-CHECKS.md | 1 + .../_fixtures/ByogMockCli/mocks/uip | 16 ++- .../byog_decorator/byog_decorator.yaml | 10 +- .../byog_middleware/byog_middleware.yaml | 10 +- .../guardrails/byog_pinning/byog_pinning.yaml | 4 +- .../byog_pinning/check_byog_pinning.py | 2 +- .../byog_pinning/mock_template/mocks/uip | 133 ------------------ .../create_configuration_smoke.yaml | 3 + .../guardrails/delete_smoke.yaml | 5 +- .../guardrails/probe_abort_smoke.yaml | 3 + 11 files changed, 56 insertions(+), 147 deletions(-) rename tests/tasks/uipath-agents/{coded/guardrails => }/_fixtures/ByogMockCli/mocks/uip (90%) delete mode 100755 tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/mock_template/mocks/uip diff --git a/.github/workflows/test-helpers.yml b/.github/workflows/test-helpers.yml index 374bd47438..49ef2ff3b5 100644 --- a/.github/workflows/test-helpers.yml +++ b/.github/workflows/test-helpers.yml @@ -123,6 +123,22 @@ jobs: - name: Run pytest run: pytest tests/tasks/uipath-agents/ -v + pytest-review-check: + runs-on: uipath-ubuntu-latest + name: uipath-review checker unit tests + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: '3.13' + + - name: Install pytest + run: pip install pytest + + - name: Run pytest + run: pytest tests/tasks/uipath-review/ -v + runtime-payload-casing-guard: runs-on: uipath-ubuntu-latest name: runtime-payload key-casing contract guard diff --git a/docs/REQUIRED-CHECKS.md b/docs/REQUIRED-CHECKS.md index b7a510daf1..4df5f8f03a 100644 --- a/docs/REQUIRED-CHECKS.md +++ b/docs/REQUIRED-CHECKS.md @@ -156,6 +156,7 @@ This table is machine-read. `scripts/parse-required-checks.py` is its only parse | `maestro-bpmn checker unit tests` | `test-helpers.yml` | | `maestro-case checker unit tests` | `test-helpers.yml` | | `uipath-agents checker unit tests` | `test-helpers.yml` | +| `uipath-review checker unit tests` | `test-helpers.yml` | | `uipath-planner checker unit tests` | `test-helpers.yml` | | `uipath-admin verify negative controls` | `test-helpers.yml` | | `runtime-payload key-casing contract guard` | `test-helpers.yml` | diff --git a/tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip b/tests/tasks/uipath-agents/_fixtures/ByogMockCli/mocks/uip similarity index 90% rename from tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip rename to tests/tasks/uipath-agents/_fixtures/ByogMockCli/mocks/uip index cdb606f15f..f881f42bd1 100755 --- a/tests/tasks/uipath-agents/coded/guardrails/_fixtures/ByogMockCli/mocks/uip +++ b/tests/tasks/uipath-agents/_fixtures/ByogMockCli/mocks/uip @@ -1,10 +1,12 @@ #!/usr/bin/env python3 -"""Selective `uip` shim for the CODED BYOG guardrail tasks. +"""Selective `uip` shim for the BYOG guardrail tasks. -Shared by `byog_middleware` and `byog_decorator`. Serves the two read-only -discovery paths a coded agent walks before it can pin a bring-your-own -guardrail, both answering with the configuration the prompts name -(`byog-smoke-agent-pin`, pii_detection): +Shared by `coded/guardrails/byog_middleware`, `coded/guardrails/byog_decorator` +and `lowcode/guardrails/byog_pinning` -- one payload, so a CLI field rename is a +one-file edit and the three tasks cannot drift apart. Serves the two read-only +discovery paths an agent walks before it can pin a bring-your-own guardrail, +both answering with the configuration the prompts name (`byog-smoke-agent-pin`, +pii_detection): * `uip agent guardrails list [--byo]` -> GuardrailDefinitionsList * `uip guardrails byo-configurations list` -> ByoGuardrailConfigurationsList @@ -35,8 +37,8 @@ name and changed nothing, scoring 0.00 -- a fixture outage rendered as a skill regression. `mock_path_dirs: [mocks]` PATH-prepends this script's directory inside the -sandbox. Linux smoke only (no `windows` tag on either task), hence no -`uip.cmd` twin -- same as the ixp, byog_pinning and platform guardrails mocks. +sandbox. Linux smoke only (no `windows` tag on any of the three tasks), hence no +`uip.cmd` twin -- same as the ixp and platform guardrails mocks. """ import json diff --git a/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml b/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml index 6e90ba21ae..126c2c4ba3 100644 --- a/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml +++ b/tests/tasks/uipath-agents/coded/guardrails/byog_decorator/byog_decorator.yaml @@ -28,7 +28,7 @@ sandbox: mock_path_dirs: [mocks] template_sources: - type: template_dir - path: ../_fixtures/ByogMockCli + path: ../../../_fixtures/ByogMockCli - type: template_dir path: ../_fixtures/SimpleCodedAgent @@ -54,6 +54,14 @@ success_criteria: weight: 1.5 pass_threshold: 1.0 + - type: command_executed + description: "Agent discovered the BYO configuration from the CLI rather than copying the name out of the prompt" + tool_name: "Bash" + command_pattern: '(uip|\$UIP)\s+(agent\s+guardrails\s+list|guardrails\s+byo-configurations\s+list)' + min_count: 1 + weight: 2.0 + pass_threshold: 1.0 + - type: run_command description: "BYOG decorator guardrail correctly added to graph.py" command: "python3 $TASK_DIR/check_byog_decorator.py" diff --git a/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml b/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml index 5a7152fc1a..2799463c6b 100644 --- a/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml +++ b/tests/tasks/uipath-agents/coded/guardrails/byog_middleware/byog_middleware.yaml @@ -28,7 +28,7 @@ sandbox: mock_path_dirs: [mocks] template_sources: - type: template_dir - path: ../_fixtures/ByogMockCli + path: ../../../_fixtures/ByogMockCli - type: template_dir path: ../_fixtures/SimpleCodedAgent @@ -46,6 +46,14 @@ initial_prompt: | end-to-end in a single pass. success_criteria: + - type: command_executed + description: "Agent discovered the BYO configuration from the CLI rather than copying the name out of the prompt" + tool_name: "Bash" + command_pattern: '(uip|\$UIP)\s+(agent\s+guardrails\s+list|guardrails\s+byo-configurations\s+list)' + min_count: 1 + weight: 2.0 + pass_threshold: 1.0 + - type: run_command description: "BYOG middleware guardrail correctly added to graph.py" command: "python3 $TASK_DIR/check_byog_middleware.py" diff --git a/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/byog_pinning.yaml b/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/byog_pinning.yaml index 8f02948a65..398e614548 100644 --- a/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/byog_pinning.yaml +++ b/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/byog_pinning.yaml @@ -2,7 +2,7 @@ task_id: skill-agent-guardrail-byog-pinning description: > BYO guardrail pinning on an existing agent. The sandbox is pre-seeded with the Web Research Briefing solution (shared _fixtures copy, - guardrails empty), and a selective `uip` shim (mock_template/mocks/uip) + guardrails empty), and a selective `uip` shim (../../../_fixtures/ByogMockCli/mocks/uip) serves the discovery call — `uip agent guardrails list` returns a BYO pii_detection entry named byog-smoke-agent-pin alongside the built-in entry sharing the same Validator name — while every other uip command @@ -23,7 +23,7 @@ run_limits: sandbox: mock_path_dirs: [mocks] template_sources: - - {type: template_dir, path: mock_template} + - {type: template_dir, path: ../../../_fixtures/ByogMockCli} - {type: template_dir, path: ../_fixtures/WebResearchBriefingSolution} initial_prompt: | diff --git a/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/check_byog_pinning.py b/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/check_byog_pinning.py index f6a3e51db0..e97fcd7ec8 100644 --- a/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/check_byog_pinning.py +++ b/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/check_byog_pinning.py @@ -3,7 +3,7 @@ Validates that the agent authored a builtInValidator guardrail for pii_detection in agent.json that is pinned to the BYO configuration the -mocked discovery served (see mock_template/mocks/uip): +mocked discovery served (see ../../../_fixtures/ByogMockCli/mocks/uip): - guardrails array exists and is non-empty - At least one guardrail has $guardrailType == "builtInValidator" diff --git a/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/mock_template/mocks/uip b/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/mock_template/mocks/uip deleted file mode 100755 index eb17c6e8f7..0000000000 --- a/tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/mock_template/mocks/uip +++ /dev/null @@ -1,133 +0,0 @@ -#!/usr/bin/env python3 -"""Selective `uip` shim for the byog_pinning smoke task. - -Intercepts exactly one command — `uip agent guardrails list` — and serves a -canned tenant response containing the BYO guardrail entry the task's prompt -references (`byog-smoke-agent-pin`, pii_detection), alongside the built-in -`pii_detection` entry sharing the same `Validator` name, so the agent must -disambiguate via `IsByo`/`ByoValidatorName` exactly as the skill teaches. -`--byo` filters to the BYO entry only, mirroring the real CLI. - -Every other invocation exec's the REAL `uip` found later on PATH — -`agent init` / `refresh` / `validate` are local operations that must -actually run for the task's artifacts and checks to exist. - -Why mock discovery at all: the task's scope is the skill's authoring -behavior (pin an already-configured BYO guardrail in agent.json), not -tenant lifecycle. A real BYOG configuration can no longer be seeded -per-run (`byo-configurations create` probes the connection server-side, -no skip flag), and the shared smoke tenant has no guardrail-capable -connection — mocking the read-only discovery call removes that tenant -dependency entirely while leaving the graded behavior untouched. - -`mock_path_dirs: [mocks]` PATH-prepends this script's directory inside the -sandbox. Linux smoke only (no `windows` tag on the task), hence no -`uip.cmd` twin — same as the ixp mock. -""" - -import json -import os -import shutil -import sys - -MOCK_DIR = os.path.dirname(os.path.abspath(__file__)) - -BYO_ENTRY = { - "Validator": "pii_detection", - "IsByo": True, - "Status": "Available", - "AllowedScopes": ["Agent", "Llm", "Tool"], - "GuardrailStages": { - "Agent": ["PreExecution", "PostExecution"], - "Llm": ["PreExecution", "PostExecution"], - "Tool": ["PreExecution", "PostExecution"], - }, - "DisplayName": "PII Detection", - "Description": ( - "Detects personally identifiable information. Served by the " - "tenant's external bring-your-own guardrail provider." - ), - "ByoValidatorName": "byog-smoke-agent-pin", - "ByoConnectionId": "18fb337c-29b7-4162-a9e8-0c05b01cf4df", - "ByoConfigurationId": "e5723bb8-fbc2-4317-c7d7-08de803bc010", - "ByoConnectorName": "Azure AI Content Safety", - "ByoConnectorKey": "uipath-azure-contentsafety", - "FolderKey": "627fe423-5c73-464a-abff-41fdaad6ac19", - "Parameters": [ - { - "Type": "enum-list", - "Id": "entities", - "Required": True, - "DefaultValue": ["Email", "PhoneNumber"], - "Options": [ - "Email", - "PhoneNumber", - "Person", - "Address", - "USSocialSecurityNumber", - ], - "DisplayName": "Entities", - "Description": "PII entity types to detect.", - }, - { - "Type": "map-enum", - "Id": "entityThresholds", - "Required": True, - "DefaultValue": {"Email": 0.5, "PhoneNumber": 0.5}, - "KeySource": "entities", - "Min": 0, - "Max": 1, - "DisplayName": "Per-entity threshold", - "Description": "Confidence threshold (0-1) per selected entity.", - }, - ], -} - -BUILTIN_ENTRY = { - "Validator": "pii_detection", - "IsByo": False, - "Status": "Available", - "AllowedScopes": ["Agent", "Llm", "Tool"], - "GuardrailStages": { - "Agent": ["PreExecution", "PostExecution"], - "Llm": ["PreExecution", "PostExecution"], - "Tool": ["PreExecution", "PostExecution"], - }, - "DisplayName": "PII Detection", - "Description": ( - "Detects personally identifiable information using Azure " - "Cognitive Services (UiPath built-in)." - ), - "Parameters": BYO_ENTRY["Parameters"], -} - - -def real_uip(): - path = os.environ.get("PATH", "") - parts = [ - p for p in path.split(os.pathsep) - if p and os.path.abspath(p) != MOCK_DIR - ] - return shutil.which("uip", path=os.pathsep.join(parts)) - - -def main(): - args = sys.argv[1:] - literal = [a for a in args if not a.startswith("-")] - if literal[:3] == ["agent", "guardrails", "list"]: - data = [BYO_ENTRY] if "--byo" in args else [BUILTIN_ENTRY, BYO_ENTRY] - print(json.dumps({ - "Result": "Success", - "Code": "GuardrailDefinitionsList", - "Data": data, - }, indent=2)) - return 0 - real = real_uip() - if not real: - sys.stderr.write("uip (shim): real uip CLI not found on PATH\n") - return 127 - os.execv(real, [real] + args) - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/tests/tasks/uipath-platform/guardrails/create_configuration_smoke.yaml b/tests/tasks/uipath-platform/guardrails/create_configuration_smoke.yaml index 935acb9d3e..6da58bd56f 100644 --- a/tests/tasks/uipath-platform/guardrails/create_configuration_smoke.yaml +++ b/tests/tasks/uipath-platform/guardrails/create_configuration_smoke.yaml @@ -110,5 +110,8 @@ success_criteria: saved WITHOUT server-side validation (for example telling the user to go and verify the connection themselves, or calling the saved configuration unverified), or says nothing about validation at all. + Score EXACTLY 1.0 or EXACTLY 0.0 — there is no partial credit and no + intermediate value. If the case feels borderline, decide which of the two + descriptions above fits better and return that score. weight: 2.0 pass_threshold: 1.0 diff --git a/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml b/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml index f8d48955f1..0b48e85b82 100644 --- a/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml +++ b/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml @@ -93,7 +93,8 @@ success_criteria: to do about it, so naming a remediation (remove, repoint, re-create under the same ValidatorName) is a bonus and its absence is NOT a deduction. - Return exactly 1.0 or 0.0. This rubric has no partial credit, and the - criterion's pass_threshold is 1.0 — an intermediate score fails the task. + Score EXACTLY 1.0 or EXACTLY 0.0 — there is no partial credit and no + intermediate value. If the case feels borderline, decide which of the two + descriptions above fits better and return that score. weight: 1.5 pass_threshold: 1.0 diff --git a/tests/tasks/uipath-platform/guardrails/probe_abort_smoke.yaml b/tests/tasks/uipath-platform/guardrails/probe_abort_smoke.yaml index 9849ace04b..568d89216f 100644 --- a/tests/tasks/uipath-platform/guardrails/probe_abort_smoke.yaml +++ b/tests/tasks/uipath-platform/guardrails/probe_abort_smoke.yaml @@ -113,5 +113,8 @@ success_criteria: (auth, permissions, the feature flag, a transient error) rather than the connection not serving the validator; or if it never says whether the configuration exists. + Score EXACTLY 1.0 or EXACTLY 0.0 — there is no partial credit and no + intermediate value. If the case feels borderline, decide which of the two + descriptions above fits better and return that score. weight: 2.0 pass_threshold: 1.0