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/_fixtures/ByogMockCli/mocks/uip b/tests/tasks/uipath-agents/_fixtures/ByogMockCli/mocks/uip new file mode 100755 index 0000000000..f881f42bd1 --- /dev/null +++ b/tests/tasks/uipath-agents/_fixtures/ByogMockCli/mocks/uip @@ -0,0 +1,178 @@ +#!/usr/bin/env python3 +"""Selective `uip` shim for the BYOG guardrail tasks. + +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 + +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 any of the three tasks), hence no +`uip.cmd` twin -- same as the ixp 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..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 @@ -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 @@ -47,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 c3ce3525db..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 @@ -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 @@ -39,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 c6b19f4146..0b48e85b82 100644 --- a/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml +++ b/tests/tasks/uipath-platform/guardrails/delete_smoke.yaml @@ -78,16 +78,23 @@ 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. + + 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 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):