fix(tests): three false-negative guardrail failures from the 2026-09-10 nightlies

[apetraru-uipath]

Three unrelated fixes, each traced to a task that scored a false negative in the 2026-09-10 guardrail nightlies. In all three the agent behaved correctly and the harness or the fixture was wrong.

Task	Was	Cause
skill-review-agents-lowcode-guardrail-pii-missing	0.875	checker read the oldest review-history entry
skill-agent-guardrail-coded-byog-middleware	0.00	tenant fixture gone; discovery returned empty
skill-platform-guardrails-byog-delete	0.984 → FAILURE	judge rubric demanded more than the prompt asks

---

1. check_review_history.py read the oldest entry

It graded 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.

With a single entry the two coincide, which is why this shipped green in #3123. It bites the moment a reviewer records twice. In run 2026-09-10_04-18-49 the reviewer recorded a provisional C, finished its analysis, then recorded the corrected D — reasoning about the ordering explicitly:

"Since the CLI keeps history entries (most recent first) rather than overwriting, I'll add a corrected entry so the latest one reflects the accurate result."

[ {"grade": "D", "runAt": "...04:36:37Z"},    // newest — matches the report
  {"grade": "C", "runAt": "...04:29:48Z"} ]   // oldest — superseded

The report's final grade is D. The checker compared it against the C.

Validated by replaying that exact artifact: pre-fix FAIL: recorded grade C does not match … D; post-fix PASS on grade D; with the newest entry tampered to B, still FAIL. tests/tasks/uipath-review/_shared/ → 42 passed.

test_last_entry_wins_when_history_has_multiple_entries encoded the wrong model (it wrote [older, newest]). Corrected, renamed, and paired with a case asserting the reverse ordering still fails — so reading history[0] cannot degrade into "find some entry that matches".

2. Coded BYOG tasks depended on a tenant fixture that is gone

byog_middleware and byog_decorator ask the agent to pin byog-smoke-agent-pin and relied on it existing on the shared smoke tenant. Neither has a pre_run creating it.

2026-08-24  tenant held byog-smoke-pii, byog-harmful-content, cli-harmful-content-1
            — none named byog-smoke-agent-pin
2026-09-10  uip agent guardrails list --byo        -> Data: []
            uip guardrails byo-configurations list -> Data: []

The agent correctly refused to fabricate a validator name and changed nothing → 0.00. byog_decorator carries the identical defect, hidden because it was skipped that run and carried forward as a green 1.00.

Seeding per-run is not available: byo-configurations create probes the connection server-side with no skip flag, and the 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, from one shared shim under _fixtures/ByogMockCli.

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 still returns the built-in pii_detection entry so the agent must 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 with codex + gpt-5.6-terra (the harness that failed), --driver tempdir: byog_middleware SUCCESS 1.000, byog_decorator SUCCESS 1.000.

3. byog-delete judge rubric asked for more than the prompt

The 1.0 condition was a conjunction: convey that referencing guardrails 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."

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.

Dropped the remediation conjunct so the criterion grades what the user asked (naming a fix stays a bonus, never a deduction), and stated 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.

---

Two things worth raising separately

• Carry-forward can hide environment breakage. byog_decorator reported a green 1.00 while sitting on a fixture that no longer existed. Consider excluding tenant-dependent tasks from carry-forward, or expiring it after N runs.
• The runbook's Codex workaround clobbers task-owned mocks. Run Skill Smoke Tests prescribes -D 'sandbox.mock_path_dirs=["."]', and that list replaces rather than merges — so it silently disables the shim on any task that sets its own mock_path_dirs (byog_pinning, the four platform guardrails tasks, and now these two). Local Codex runs need ["mocks","."].

The unrelated escalation-smoke failure from the same nightlies is fixed harness-side in UiPath/coder_eval#162.

🤖 Generated with Claude Code

[github-actions]

Claude finished @apetraru-uipath's task in 1m 12s —— View job

---

PR Review: fix(uipath-review): read the newest review-history entry, not the oldest

• Read review criteria and project conventions
• Read full diff and changed files
• Analyze correctness of the fix
• Check test coverage
• Post review findings

---

Summary

Fixes check_review_history.py to read history[0] (newest) instead of history[-1] (oldest), matching the CLI's documented "most recent first" ordering. Updates the docstring, error messages, and test suite to match. The bug was invisible with single-entry histories and only surfaced when a reviewer recorded twice (documented with a real failure in run 2026-09-10_04-18-49).

Change-by-Change Review

1. tests/tasks/uipath-review/_shared/check_review_history.py

Severity: OK

The fix is correct and well-scoped:

• Line 114-115: history[-1] → history[0] — the core fix. Matches the CLI's documented "most recent first" ordering.
• Lines 9-17: Docstring updated to explain the ordering contract and why history[0] is correct. Good — the old docstring said "appends", which was wrong per the CLI's help.
• Lines 117, 121, 131: Error messages updated from "last" to "newest" — consistent with the semantic change.

No logic changes beyond the index and messaging. The rest of the checker (grade comparison, int field validation, CLI probe fallback) is untouched and still correct.

2. tests/tasks/uipath-review/_shared/test_check_review_history.py

Severity: OK

Strong test updates:

• Lines 77-87 (test_newest_entry_wins_when_history_has_multiple_entries): Renamed from test_last_entry_wins_.... Corrects the fixture ordering — [ENTRY, superseded] puts the matching entry at index 0 (newest-first), which is what the CLI actually produces. The old test had [older, ENTRY] which was append-order and coincidentally passed the pre-fix code. Good docstring explaining the real-world failure.
• Lines 90-98 (test_fails_when_the_newest_entry_is_the_one_that_disagrees): New companion test. Ensures reading history[0] doesn't degrade into "find any matching entry" — if the newest entry has the wrong grade, the checker must fail even if a later entry matches. This is the important guard against a different class of false-pass.

Both tests together form a proper bidirectional check: correct newest entry → pass, incorrect newest entry with matching old entry → fail.

What's Missing

Nothing identified.

Area Ratings

Area	Status	Notes
Frontmatter	N/A	No skill frontmatter changed
E2E Tests	OK	Checker unit tests updated with correct ordering model + new reverse-ordering guard
Skill Body	N/A	No SKILL.md changed
References & Assets	N/A	No references changed
Repo Hygiene	OK	Changes scoped to the two affected files; no secrets, no cross-skill deps

Issues for Manual Review

None found.

Conclusion

Clean, well-motivated fix. The bug analysis is thorough (real failure, replay validation, tamper check). The code change is minimal — one index flip and consistent messaging. The test corrections are sound: the renamed test now matches the CLI's actual ordering, and the new companion test prevents a different failure mode. LGTM.

---

[rockymadden]

What

Three unrelated false negatives from the 2026-09-10 guardrail nightlies, each landing in a different layer.

1. 🟢 check_review_history.py read the oldest entry. history[-1] → history[0], plus docstring and message wording. I confirmed the contract two ways: uip agent review-history add --help says "Keeps the newest 25 entries, most recent first", and recording C then D against a scratch project writes [D, C]. The fix is right, and the old test encoded the wrong model ([older, ENTRY]), which is exactly why it shipped green.
2. 🟢 Coded BYOG tasks depended on a tenant fixture. A shared uip shim now serves both discovery verbs. The premise holds: byo-configurations create probes the connection server-side with no skip flag (skills/uipath-platform/references/guardrails/byo-configurations.md:71), so per-run seeding is out. Serving the second verb is not gold-plating: coded Rule 18 (guardrails.md:490) tells the agent to cross-check Enabled/ValidConnection there before wiring. The mock's payload matches the documented response shape field for field, and the grading claim checks out, both checkers are pure AST over graph.py.
3. 🟢 byog-delete rubric asked for more than the prompt. The prompt says "tell me what happens to any agent still pointing at it", and the 1.0 condition demanded remediation on top. Dropping the conjunct is correct.

The PR description is accurate everywhere I could check it. No AI slop.

---

Overall findings

🔴 major, tests/tasks/uipath-review/ has no CI job, so the new guard never runs

test-helpers.yml runs pytest for uipath-maestro-bpmn, uipath-maestro-flow, uipath-maestro-case, uipath-agents, uipath-planner and uipath-admin/_shared. Not uipath-review. Its three suites (test_check_review_history.py, test_check_review_cli_provenance.py, test_check_guardrail_catalog_evidence.py) run only when someone runs them by hand.

That is the whole failure mode of this bug: #3123 shipped a checker whose test encoded the wrong ordering model, and nothing caught it. test_fails_when_the_newest_entry_is_the_one_that_disagrees is the guard against the next inversion, and today it is a file nobody executes. The "42 passed" in the description is a local result that will not re-run.

Pre-existing gap, not caused by this PR, but this PR is the one that now depends on it. Fix, mirroring the sibling jobs:

  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

Add the matching row to docs/REQUIRED-CHECKS.md:158, or required-check contract guard fails.

🟡 minor, the rubric defect is a class, and two siblings still carry it

The reasoning in commit 3 ("a binary rubric paired with a 1.0 threshold is only sound if the judge cannot return an intermediate value") condemns two more criteria in the same directory:

• tests/tasks/uipath-platform/guardrails/create_configuration_smoke.yaml:84
• tests/tasks/uipath-platform/guardrails/probe_abort_smoke.yaml:83

Both are prose "Score 1.0 if / Score 0.0 if" with pass_threshold: 1.0 and no scale statement, so both can return the same 0.9 that failed byog-delete. Either add the scale line to all three now or file the follow-up, but do not leave the next nightly to find them one at a time.

🟡 minor, validated on a driver the nightlies do not use

Commit 2 was validated with --driver tempdir (tests/experiments/default.yaml:13). The nightlies that failed run driver: docker on skills-image (tests/experiments/nightly.yaml:13). The PATH-prepend mechanism is byte-identical to byog_pinning's, which already runs under docker, so I expect it carries. I could not verify it, and neither could the author.

What I confirmed vs reasoned

• Confirmed against the installed CLI: review-history ordering, byo-configurations list field names.
• Confirmed by reading: both BYOG checkers touch no tenant, byog_pinning's shim serves only one verb, CI job list, the three judge rubrics.
• Reasoned, not confirmed: shim behavior under the docker driver, the "42 passed" count (tests read, not run), and every claim sourced from run artifacts 2026-09-10_04-17-44 / 04-18-49, which I cannot reach.

---

Recommendation

✅ Approve. All three fixes are correct, each traced to evidence I could reproduce or read, and scoped tightly. Every finding is additive: one CI job the repo has been missing since before this PR, two sibling rubrics with the same latent defect, and two shim cleanups. None of them argue for holding the merge, and holding it keeps three known-broken tasks in the nightlies.

---

tl;dr: Three real fixes, all correct, evidence checks out. The catch: tests/tasks/uipath-review/ runs in no CI job, so the regression test this PR just wrote will never execute. Add the pytest job. Two sibling judge rubrics have the same 0.9 defect and are still live. Approve and follow up.

[rockymadden]

🟡 minor — the second sentence hands the judge a reason to round up.

Return exactly 1.0 or 0.0. This rubric has no partial credit. does the job. Appending "the criterion's pass_threshold is 1.0, an intermediate score fails the task" tells the judge the consequence of its score, and a judge that is genuinely 50/50 now has one option that "fails the task" and one that does not. This PR exists to kill a false negative; that clause trades it for a false positive.

The repo already has the better wording, used in three uipath-admin audit rubrics (audit_login_history_e2e.yaml:82, audit_who_did_x_e2e.yaml:73, audit_scope_ambiguity_smoke.yaml:64). It states the scale and gives a tiebreak procedure instead of a consequence:

      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.

Suggest swapping to that, both for the neutrality and because a fourth spelling of the same instruction is one more thing to keep in sync.

[rockymadden]

🟡 minor — nothing grades the discovery this mock exists to serve.

The low-code sibling pairs its shim with a criterion (byog_pinning.yaml:44, weight 2.0):

  - type: command_executed
    description: "Agent discovered the BYO entry via uip agent guardrails list (--byo or full list)"
    tool_name: "Bash"
    command_pattern: 'uip\s+agent\s+guardrails\s+list'
    min_count: 1

Here there is no such criterion, and the prompt already names byog-smoke-agent-pin, so check_byog_middleware.py passes whether the agent discovered the configuration or copied the string out of the prompt. Two consequences:

• The mock is ungraded infrastructure. If it stops being installed, and the description itself documents one way that happens (the runbook's -D 'sandbox.mock_path_dirs=["."]' clobber), nothing reports it. The task just drifts back to depending on the agent's mood about verifying. That is the same shape as the byog_decorator carry-forward this PR is fixing: an environment fault wearing a skill result.
• Coded Rule 18 goes untested. guardrails.md:490 says to read ByoValidatorName from discovery and never from memory, and step 2 says cross-check Enabled/ValidConnection. The shim now serves both verbs specifically so the agent can do that. Nothing checks that it did.

Suggest adding the byog_pinning criterion to both this task and byog_decorator.yaml, matching on either verb.

[rockymadden]

🟡 minor — second copy of this payload, and the twin is now the weaker one.

tests/tasks/uipath-agents/lowcode/guardrails/byog_pinning/mock_template/mocks/uip carries the same BYO_ENTRY and BUILTIN_ENTRY, the same GUIDs, the same validator name. This file adds two things it does not have:

• guardrails byo-configurations list, so a low-code agent that follows the ByoConfigurationId cross-reference at lowcode/.../guardrails.md:642 still falls through to the real CLI and gets Data: [], the exact fixture-outage failure this PR is fixing on the coded half.
• the --help passthrough, so byog_pinning still answers a flag question with a canned payload.

Two copies that already disagree will keep disagreeing, and a CLI field rename now needs both edited. Suggest pointing byog_pinning.yaml at this fixture and deleting its local copy (it already reaches across with ../_fixtures/..., though I have not verified the harness accepts a path that climbs out of lowcode/), or hoisting the shim to a shared tests/tasks/uipath-agents/_fixtures/.

Not blocking, and either way this file is the better of the two.

[apetraru-uipath]

Thanks — all findings addressed, and rebased onto main (37 commits, clean).

🔴 tests/tasks/uipath-review/ had no CI job. You were right that this is the finding that makes the rest durable — the ordering guard this PR wrote was a file nobody executed. Added pytest-review-check mirroring the sibling jobs, plus the docs/REQUIRED-CHECKS.md row. The contract guard passes, and the job's exact command runs 42 tests.

🟡 The scale sentence handed the judge a reason to round up. This was the sharpest catch in the review. Naming the consequence gives a borderline judge one option that "fails the task" and one that does not — trading this PR's false negative for a false positive. Swapped to the wording the three uipath-admin audit rubrics already use, which gives a tiebreak procedure instead. A fourth spelling would also have been one more thing to keep in sync.

🟡 Two siblings carried the same defect. create_configuration_smoke and probe_abort_smoke both now carry the scale line. Done here rather than filed, since leaving them live means the next nightly finds them one at a time.

🟡 Nothing graded the discovery the mock exists to serve. Correct, and the Rule 18 point is the part that convinced me — the shim serves both verbs specifically so the agent can cross-check Enabled/ValidConnection, and nothing checked that it did. Added the byog_pinning-style command_executed criterion to both coded tasks, matching either verb.

🟡 Second copy of the payload, and the twin was weaker. Hoisted to tests/tasks/uipath-agents/_fixtures/ByogMockCli and pointed all three tasks at it, deleting byog_pinning's local copy. Went with the shared-fixture option rather than the reach-across, since all three then sit at the same depth. byog_pinning re-run on the shared fixture: SUCCESS 1.000, all four criteria including its own discovery criterion — so it now gets the byo-configurations list verb and the --help passthrough it was missing.

🟡 Validated on a driver the nightlies do not use. Partially closed. I verified the shim inside skills-image:latest under a PATH prepend — resolves to the shim, both verbs answer, passthrough to the real uip (1.197.1) works:

resolves to : /mocks/uip
agent guardrails list --byo : ['byog-smoke-agent-pin']
byo-configurations list     : [('byog-smoke-agent-pin', True, True)]

A full docker-driver task run is still unverified: the local skills-image bakes coder_eval 0.8.8 and rejects a 0.12.0 task config (extra_forbidden on record_cli, stop_early). So this is closed at the mechanism, not end to end — same honest boundary you drew.

One consequence worth flagging: because byog_pinning now uses the shared shim, it exercises this file under docker in the nightlies. That is more coverage than before, but it also means a fault in this shim would now affect three tasks rather than two.