From 8541de9c2021b29b57532777611b2d360b056848 Mon Sep 17 00:00:00 2001 From: Amitfre15 Date: Mon, 7 Sep 2026 11:32:58 +0000 Subject: [PATCH 1/6] feat: Add end-to-end correctness suite scaffold (#2090) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires up the eval_correctness_e2e suite: reuses correctness_scorer.py and the primary 8-scenario corpus, but sources granted/denied pairs from the real Keycloak+PCE+OPA pipeline's rendered Rego data maps (subject_role_allow/deny_scopes, agent_role_scopes) instead of the PRB's raw output. Registers the marker in pyproject.toml and wires it into eval/conftest.py's report. WIP: the 4 pure-logic unit tests in the new file (_pairs_from_map, _accumulate_agent_gates) are currently wrongly excluded from the default fast pass because pytestmark is applied at module level, covering them too. Needs a decision (own unmarked file vs. per-test marker) before this is complete — see the handoff doc for the two options. A full live run against the sandbox's Keycloak also hit an environment-side 500 on realm creation, reproduced identically against the unmodified legacy suite — confirmed not caused by this change, but not yet verified against a healthy Keycloak either. Assisted-By: Claude (Anthropic AI) Signed-off-by: Amitfre15 --- aiac/eval/conftest.py | 33 ++- .../test_policy_pipeline_correctness_e2e.py | 246 ++++++++++++++++++ aiac/pyproject.toml | 3 +- 3 files changed, 270 insertions(+), 12 deletions(-) create mode 100644 aiac/eval/test_policy_pipeline_correctness_e2e.py diff --git a/aiac/eval/conftest.py b/aiac/eval/conftest.py index 7948cce7b..f9e99b2b8 100644 --- a/aiac/eval/conftest.py +++ b/aiac/eval/conftest.py @@ -1,19 +1,21 @@ """Per-run pass/fail/skip report for the policy-eval-scenarios, policy-eval-robustness, -policy-eval-consistency, and policy-eval-correctness-prb suites (spec: ``docs/specs/eval/ -policy-eval-scenarios.md``, ``docs/specs/eval/policy-eval-robustness-consistency.md``, and -``docs/specs/eval/policy-eval-correctness-prb.md``). +policy-eval-consistency, policy-eval-correctness-prb, and policy-eval-correctness-e2e suites +(spec: ``docs/specs/eval/policy-eval-scenarios.md``, ``docs/specs/eval/ +policy-eval-robustness-consistency.md``, ``docs/specs/eval/policy-eval-correctness-prb.md``, and +``docs/specs/eval/policy-eval-correctness-e2e.md``). Every run of ``test_policy_pipeline_eval.py`` (``@pytest.mark.eval_extended``), ``test_policy_pipeline_consistency.py`` (``@pytest.mark.eval_consistency``), -``test_policy_pipeline_robustness.py`` (``@pytest.mark.eval_robustness``), or -``test_policy_pipeline_correctness_prb.py`` (``@pytest.mark.eval_correctness_prb``) writes a +``test_policy_pipeline_robustness.py`` (``@pytest.mark.eval_robustness``), +``test_policy_pipeline_correctness_prb.py`` (``@pytest.mark.eval_correctness_prb``), or +``test_policy_pipeline_correctness_e2e.py`` (``@pytest.mark.eval_correctness_e2e``) writes a Markdown report to ``reports/`` listing every collected test's outcome — passed, failed, skipped, xfailed, xpassed, or a setup/collection error. All six sections are always present (even empty) so a reader can see at a glance that nothing was silently omitted. Failed/error entries carry the assertion's crash message (pytest's own computed diff, e.g. "assert True == False" or a custom mismatch message with expected/actual sets); skipped/xfailed entries carry the skip reason; every entry carries the test function's docstring so a reader doesn't have to open the source file to -know what was actually being checked. The report is scoped to these four markers (not just "any +know what was actually being checked. The report is scoped to these five markers (not just "any test collected while this conftest happens to be loaded"), so running the whole repo's test suite from a parent directory does not pull unrelated tests into this suite's report. @@ -28,10 +30,13 @@ rendered as "What it tests" / "Expected output" / "Output" instead of the generic docstring + crash-message fallback used by every other test in this suite (see ``_render_entry``). -``test_prb_correctness`` (the correctness-prb suite) similarly ``record_property``s -precision/recall/denial-precision plus the over-grants/under-grants/incorrectly-denied pair -breakdown per scenario -- rendered as its own metrics + detail block, always (pass or fail), since -the tracked-but-non-gating under-grant/denial detail is otherwise invisible on a passing run. +``test_prb_correctness`` (the correctness-prb suite) and ``test_e2e_correctness`` (the +correctness-e2e suite) similarly ``record_property``s precision/recall/denial-precision plus the +over-grants/under-grants/incorrectly-denied pair breakdown per scenario -- rendered as its own +metrics + detail block, always (pass or fail), since the tracked-but-non-gating under-grant/denial +detail is otherwise invisible on a passing run. The render branch dispatches generically on the +presence of ``precision``/``recall`` properties, so it covers both suites with no per-suite +special-casing. """ from __future__ import annotations @@ -47,7 +52,13 @@ HERE = Path(__file__).resolve().parent REPORTS_DIR = HERE / "reports" REPORT_TZ = ZoneInfo(os.environ.get("EVAL_REPORT_TZ", "UTC")) -MARKERS = {"eval_extended", "eval_consistency", "eval_robustness", "eval_correctness_prb"} +MARKERS = { + "eval_extended", + "eval_consistency", + "eval_robustness", + "eval_correctness_prb", + "eval_correctness_e2e", +} # Auto-load eval/.env so LLM_BASE_URL/KEYCLOAK_URL/etc. are set without having to # `set -a; . eval/.env; set +a` before invoking pytest. Existing environment diff --git a/aiac/eval/test_policy_pipeline_correctness_e2e.py b/aiac/eval/test_policy_pipeline_correctness_e2e.py new file mode 100644 index 000000000..fc4d27225 --- /dev/null +++ b/aiac/eval/test_policy_pipeline_correctness_e2e.py @@ -0,0 +1,246 @@ +"""End-to-end correctness suite (spec: ``docs/specs/eval/policy-eval-correctness-e2e.md``). + +Runs the same primary 8-scenario correctness corpus as +``test_policy_pipeline_correctness_prb.py`` (#2089), but through the **full** pipeline this +suite's sibling ``test_policy_pipeline_eval.py`` already drives — real Keycloak provisioning, +real Policy Rules Builder, real Policy Computation Engine, real ``opa eval`` against the rendered +Rego — instead of calling the PRB directly. Scores the result against each scenario's +hand-authored truth table using the same reusable, effect-aware ``eval.correctness_scorer`` the +PRB-level suite uses, unmodified. + +Distinguished from ``test_policy_pipeline_correctness_prb.py`` (PRB-direct, no Keycloak/OPA/PCE in +the loop) and from ``test_policy_pipeline_eval.py``'s own ``test_grant_set_matches_truth_table`` +(plain grant-set equality, no precision/recall, no awareness of ``PolicyRule.effect``). This is the +only level that can catch integration bugs between layers (PCE merge logic, Rego rendering, OPA +semantics) that a PRB-only check can't see. + +Gate: zero-tolerance on over-grants (any over-granted pair in any gate fails the test). +Under-grants and incorrectly-denied pairs are reported via ``record_property`` and a printed +summary line, never gating — same philosophy as the PRB-level suite. + +Known gap: the ``outbound_target`` gate's denial side (``AgentPolicyModel.outbound_target_deny_rules``) +is computed by the PCE but never rendered into the outbound Rego by +``aiac.pdp.service.policy.opa.rego.generate_outbound_rego`` — only the ALLOW side +(``agent_role_scopes``) is emitted. So for that one gate ``denied`` is always empty and its +``denial_precision`` reads vacuously ``1.0``; ``over_grants``/``under_grants`` for that gate are +unaffected (they depend only on ``granted``/``expected``). See +``docs/specs/eval/policy-eval-correctness-e2e.md`` for the full writeup — this is a pre-existing +production Rego-generator gap, not something this suite introduces or is scoped to fix. + +Run (needs KEYCLOAK_URL + admin creds + LLM_* exported, ``opa`` on PATH; unlike the PRB-level +suite, this one is NOT `-n`-parallelizable — all 8 scenarios share one session-scoped ``pipeline`` +fixture that provisions Keycloak sequentially in-process): + .venv/bin/pytest eval/test_policy_pipeline_correctness_e2e.py \ + -m eval_correctness_e2e -v -s +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from types import ModuleType + +import pytest + +pytestmark = pytest.mark.eval_correctness_e2e + +HERE = Path(__file__).resolve().parent # aiac/eval/ +REPO_ROOT = HERE.parent # -> aiac/ +SRC = REPO_ROOT / "src" +sys.path.insert(0, str(REPO_ROOT)) # so ``import test.integration.*``/``eval.*`` resolves +sys.path.insert(0, str(SRC)) # so ``import aiac.*`` resolves + +from eval.correctness_scorer import score_scenario # noqa: E402 +from eval.test_policy_pipeline_eval import ( # noqa: E402 + SCENARIOS, + _rego_path, + _require_scenario, + opa_bin, + opa_eval, + truth, +) +from eval.test_policy_pipeline_eval import pipeline as pipeline # noqa: E402,F401 - re-exported as a fixture +from test.integration.launcher import require_env # noqa: E402 + +Pair = tuple[str, str] + + +def _pairs_from_map(role_to_scopes: dict[str, list[str]]) -> set[Pair]: + """Flatten a rendered Rego ``{role_name: [scope_name, ...]}`` map (e.g. + ``subject_role_allow_scopes``) into a set of ``(role, scope)`` pairs.""" + return {(role, scope) for role, scopes in role_to_scopes.items() for scope in scopes} + + +def test_pairs_from_map_flattens_role_scope_map() -> None: + got = _pairs_from_map({"role-a": ["scope-x", "scope-y"], "role-b": ["scope-x"]}) + assert got == {("role-a", "scope-x"), ("role-a", "scope-y"), ("role-b", "scope-x")} + + +def test_pairs_from_map_empty_map_yields_no_pairs() -> None: + assert _pairs_from_map({}) == set() + + +def _accumulate_agent_gates( + granted: dict[str, set[Pair]], + denied: dict[str, set[Pair]], + *, + inbound_allow: dict[str, list[str]], + inbound_deny: dict[str, list[str]], + outbound_subject_allow: dict[str, list[str]], + outbound_subject_deny: dict[str, list[str]], + outbound_target_allow: dict[str, list[str]], +) -> None: + """Union one agent's rendered Rego maps into the three top-level gate buckets + (``inbound``/``outbound_subject``/``outbound_target``), in place. No ``outbound_target_deny`` + parameter — the outbound Rego generator never renders that map (see module docstring).""" + granted.setdefault("inbound", set()).update(_pairs_from_map(inbound_allow)) + denied.setdefault("inbound", set()).update(_pairs_from_map(inbound_deny)) + granted.setdefault("outbound_subject", set()).update(_pairs_from_map(outbound_subject_allow)) + denied.setdefault("outbound_subject", set()).update(_pairs_from_map(outbound_subject_deny)) + granted.setdefault("outbound_target", set()).update(_pairs_from_map(outbound_target_allow)) + + +def test_accumulate_agent_gates_classifies_into_three_buckets() -> None: + granted: dict[str, set[Pair]] = {} + denied: dict[str, set[Pair]] = {} + + _accumulate_agent_gates( + granted, + denied, + inbound_allow={"user-role-developer": ["agent-scope-repo-access"]}, + inbound_deny={"user-role-devops": ["agent-scope-repo-access"]}, + outbound_subject_allow={"user-role-developer": ["tool-scope-repo-read"]}, + outbound_subject_deny={}, + outbound_target_allow={"agent-role-repo-operations": ["tool-scope-repo-read"]}, + ) + + assert granted == { + "inbound": {("user-role-developer", "agent-scope-repo-access")}, + "outbound_subject": {("user-role-developer", "tool-scope-repo-read")}, + "outbound_target": {("agent-role-repo-operations", "tool-scope-repo-read")}, + } + assert denied == { + "inbound": {("user-role-devops", "agent-scope-repo-access")}, + "outbound_subject": set(), + } + assert "outbound_target" not in denied # never rendered — see module docstring + + +def test_accumulate_agent_gates_unions_across_multiple_agents() -> None: + granted: dict[str, set[Pair]] = {} + denied: dict[str, set[Pair]] = {} + + _accumulate_agent_gates( + granted, + denied, + inbound_allow={"user-role-developer": ["agent-scope-repo-access"]}, + inbound_deny={}, + outbound_subject_allow={}, + outbound_subject_deny={}, + outbound_target_allow={}, + ) + _accumulate_agent_gates( + granted, + denied, + inbound_allow={"user-role-tester": ["agent-scope-tracker-access"]}, + inbound_deny={}, + outbound_subject_allow={}, + outbound_subject_deny={}, + outbound_target_allow={}, + ) + + assert granted["inbound"] == { + ("user-role-developer", "agent-scope-repo-access"), + ("user-role-tester", "agent-scope-tracker-access"), + } + + +# ====================================================================================== +# opa eval wiring — thin, untested-in-isolation (same status as opa_eval itself, reused +# unmodified from eval.test_policy_pipeline_eval) +# ====================================================================================== + + +def _rego_map(rego: Path, doc: str) -> dict[str, list[str]]: + """Query one rendered Rego data document under ``data.authbridge.client.`` (e.g. + ``inbound.request.subject_role_allow_scopes``) with no input — these are plain declarations, + not decision rules, so they need none. ``{}`` when ``rego`` has no file on disk (an agent with + no rendered rego for this direction — see ``_e2e_grant_sets``).""" + if not rego.is_file(): + return {} + return opa_eval([rego], f"data.authbridge.client.{doc}", {}) + + +def _e2e_grant_sets(pipeline_result: dict, scenario: ModuleType) -> tuple[dict[str, set[Pair]], dict[str, set[Pair]]]: + """Build the ``(granted, denied)`` gate dicts ``score_scenario`` expects, sourced from the + rendered Rego of every agent in ``scenario`` (real Keycloak+PCE+OPA output, not the PRB's raw + rules) — see the module docstring for the per-gate map table and the ``outbound_target`` + denial gap. Agents with no rego on disk (declared/emergent ``EXPECT_NO_REGO``) contribute + nothing, same as ``test_inbound``/``test_outbound`` already handle it.""" + rego_dir = pipeline_result["rego_dir"] + granted: dict[str, set[Pair]] = {} + denied: dict[str, set[Pair]] = {} + + for agent_id in scenario.AGENTS: + inbound_rego = _rego_path(rego_dir, agent_id, "inbound") + outbound_rego = _rego_path(rego_dir, agent_id, "outbound") + _accumulate_agent_gates( + granted, + denied, + inbound_allow=_rego_map(inbound_rego, "inbound.request.subject_role_allow_scopes"), + inbound_deny=_rego_map(inbound_rego, "inbound.request.subject_role_deny_scopes"), + outbound_subject_allow=_rego_map(outbound_rego, "outbound.request.subject_role_allow_scopes"), + outbound_subject_deny=_rego_map(outbound_rego, "outbound.request.subject_role_deny_scopes"), + outbound_target_allow=_rego_map(outbound_rego, "outbound.request.agent_role_scopes"), + ) + return granted, denied + + +# ====================================================================================== +# The test +# ====================================================================================== + + +@pytest.mark.parametrize("scenario_name", sorted(SCENARIOS)) +def test_e2e_correctness(pipeline: dict[str, dict], scenario_name: str, record_property) -> None: + """The full Keycloak+PCE+OPA pipeline's rendered grant/deny output, scored against the + scenario's truth table, has zero over-grants (security-critical, gates this test) — + under-grants and incorrect denials are tracked/reported only (spec: threshold TBD, + deferred), same philosophy as the PRB-level suite.""" + require_env( + "KEYCLOAK_URL", + "KEYCLOAK_ADMIN_USERNAME", + "KEYCLOAK_ADMIN_PASSWORD", + "LLM_BASE_URL", + "LLM_MODEL", + "LLM_API_KEY", + ) + opa_bin() # skips cleanly if opa is not on PATH / OPA_BIN unset + scenario = SCENARIOS[scenario_name] + scenario_result = _require_scenario(pipeline, scenario_name) + + granted, denied = _e2e_grant_sets(scenario_result, scenario) + expected = truth(scenario) + score = score_scenario(scenario_name, granted, denied, expected) + + over_grants = {g: sorted(p) for g, p in score.over_grants.items()} + under_grants = {g: sorted(p) for g, p in score.under_grants.items()} + incorrectly_denied = {g: sorted(p) for g, p in score.incorrectly_denied.items()} + + record_property("precision", score.precision) + record_property("recall", score.recall) + record_property("denial_precision", score.denial_precision) + record_property("over_grants", over_grants) + record_property("under_grants", under_grants) + record_property("incorrectly_denied", incorrectly_denied) + print( + f"[correctness-e2e] {scenario_name}: precision={score.precision:.3f} " + f"recall={score.recall:.3f} denial_precision={score.denial_precision:.3f}\n" + f" over_grants={over_grants or '{}'}\n" + f" under_grants={under_grants or '{}'}\n" + f" incorrectly_denied={incorrectly_denied or '{}'}" + ) + + assert score.passed, ( + f"E2E pipeline over-granted for scenario '{scenario_name}' — zero-tolerance gate: {over_grants}" + ) diff --git a/aiac/pyproject.toml b/aiac/pyproject.toml index 2cf975f02..434529589 100644 --- a/aiac/pyproject.toml +++ b/aiac/pyproject.toml @@ -47,7 +47,7 @@ pythonpath = ["src"] # Bare `pytest` runs unit tests only — every live-infra marker is excluded by # default. `-m` on the command line overrides this (last `-m` wins), so # `pytest eval/ -m eval_extended` etc. still works to opt back in. -addopts = ["-m", "not integration and not eval_extended and not eval_consistency and not eval_robustness and not eval_correctness_prb"] +addopts = ["-m", "not integration and not eval_extended and not eval_consistency and not eval_robustness and not eval_correctness_prb and not eval_correctness_e2e"] markers = [ "integration: tests that call real LLM endpoints", "llm: tests that call the real LLM but mock descriptions/policy (no cluster)", @@ -55,4 +55,5 @@ markers = [ "eval_consistency: PRB run-to-run consistency (same scenario, N repeats, exact grant-set equality) — needs LLM_BASE_URL/LLM_MODEL/LLM_API_KEY only, no Keycloak/opa (see eval/test_policy_pipeline_consistency.py)", "eval_robustness: PRB robustness to mechanical + semantic input perturbation against the truth-table oracle — needs LLM_BASE_URL/LLM_MODEL/LLM_API_KEY only, no Keycloak/opa (see eval/test_policy_pipeline_robustness.py)", "eval_correctness_prb: PRB-level correctness — precision/recall + a denial-precision figure against the primary corpus's hand-authored truth tables, zero-tolerance over-grant gate — needs LLM_BASE_URL/LLM_MODEL/LLM_API_KEY only, no Keycloak/opa (see eval/test_policy_pipeline_correctness_prb.py)", + "eval_correctness_e2e: end-to-end correctness — same precision/recall + denial-precision scorer as eval_correctness_prb, but through the full Keycloak+PCE+OPA pipeline via opa eval — needs KEYCLOAK_URL + admin creds + LLM_BASE_URL/LLM_MODEL/LLM_API_KEY, plus opa on PATH (see eval/test_policy_pipeline_correctness_e2e.py)", ] From a7a152686eb7f7a21b95d37136a0e3c4a9fa8d37 Mon Sep 17 00:00:00 2001 From: Amitfre15 Date: Tue, 8 Sep 2026 10:23:55 +0300 Subject: [PATCH 2/6] fix: Resolve #2090 unit-test scoping and parallelize scenario provisioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splits the e2e correctness suite's pure-logic helpers into their own unmarked module (mirroring correctness_scorer.py's test-module split) so they run in the default fast pass instead of being wrongly swept into @pytest.mark.eval_correctness_e2e. Fixes a real classification bug found via a live run against the kind cluster: the outbound Rego's subject_role_allow_scopes document mixes user-role and agent-role grants, so outbound_subject scoring must filter to user roles only or it double-counts outbound_target's own true positives as over-grants. Also parallelizes the shared `pipeline` fixture's 8-scenario provisioning with ProcessPoolExecutor (not threads — os.environ/KeycloakAdmin state is shared and would race; not pytest-xdist -n, which would just duplicate provisioning 8x). A multiprocessing.Lock via the pool's initializer serializes realm creation only, since concurrent admin.create_realm calls 409 on this Keycloak instance; everything else per scenario, including the LLM-heavy PRB calls, runs fully concurrently. Verified against the real kind-cluster Keycloak + LLM endpoint, with a parallelism=1 isolation run confirming no regressions from the concurrency change itself (its failures are a strict subset of the serial run's, all attributable to the pre-existing, already-deferred PRB audit/retry non-determinism). Assisted-By: Claude (Anthropic AI) Signed-off-by: Amitfre15 --- aiac/docs/specs/PRD.md | 1 + .../specs/eval/policy-eval-correctness-e2e.md | 211 ++++++++++++++ aiac/eval/correctness_e2e_helpers.py | 51 ++++ aiac/eval/test_correctness_e2e_helpers.py | 90 ++++++ .../test_policy_pipeline_correctness_e2e.py | 117 ++------ aiac/eval/test_policy_pipeline_eval.py | 262 ++++++++++++------ 6 files changed, 554 insertions(+), 178 deletions(-) create mode 100644 aiac/docs/specs/eval/policy-eval-correctness-e2e.md create mode 100644 aiac/eval/correctness_e2e_helpers.py create mode 100644 aiac/eval/test_correctness_e2e_helpers.py diff --git a/aiac/docs/specs/PRD.md b/aiac/docs/specs/PRD.md index 2cd4a2137..6c82041f5 100644 --- a/aiac/docs/specs/PRD.md +++ b/aiac/docs/specs/PRD.md @@ -600,6 +600,7 @@ Beyond the marker-gated pytest tests above, individual integration tests are spe | `policy-eval-scenarios` — `test_policy_pipeline_eval.py` + guardrail tests | Generalized evaluation suite extending `policy-pipeline`'s single-agent/single-tool proof to ten scenarios: baseline-scale (many entities, names decoupled from roles, one agent→agent delegation grant), missing-details (emergent unreachability/zero-access under deny-by-default, a broad-sounding clause narrowed by an explicit qualifier, wildcard-grant expansion), adversarial-authoring (misleading names/descriptions, an identity/boundary-confusion probe, empty descriptions), and ambiguous-and-contradictory / adversarial-injection-and-edge-cases (whole-document `xfail` checks against the PRB directly, no Keycloak or `opa`). The eight heavy scenarios (`@pytest.mark.eval_extended`, scenario modules under `eval/scenarios/` except `agent_delegation`) assert full per-cell `opa eval` truth tables; the two light scenarios (`@pytest.mark.integration`) assert PRB-level rejection. | [eval/policy-eval-scenarios.md](eval/policy-eval-scenarios.md) | | `policy-eval-robustness-consistency` — `test_policy_pipeline_consistency.py` + `test_policy_pipeline_robustness.py` | Companion to `policy-eval-scenarios`, reusing its 8-scenario corpus to check the PRB's raw grant decisions (no OPA/PCE/k8s) for **consistency** (`@pytest.mark.eval_consistency`: N repeated runs on the same input, exact grant-set equality) and **robustness** (`@pytest.mark.eval_robustness`: mechanical text/order perturbation + a hand-reworded semantic-sibling corpus under `eval/scenarios_perturbed/`, both checked against the truth-table oracle). No Keycloak/`opa` needed — only `LLM_BASE_URL`/`LLM_MODEL`/`LLM_API_KEY`. | [eval/policy-eval-robustness-consistency.md](eval/policy-eval-robustness-consistency.md) | | `policy-eval-correctness-prb` — `test_policy_pipeline_correctness_prb.py` | Companion to `policy-eval-scenarios`/`policy-eval-robustness-consistency`, reusing the same 8-scenario corpus to score the PRB's raw grant/deny output (no OPA/PCE/k8s) against each scenario's truth table via a reusable, effect-aware scorer (`eval/correctness_scorer.py`): precision and recall tracked separately per gate and aggregated, plus a non-gating denial-precision figure for explicit `Deny` rules. `@pytest.mark.eval_correctness_prb`, zero-tolerance over-grant gate; under-grants/incorrect denials reported only. No Keycloak/`opa` needed — only `LLM_BASE_URL`/`LLM_MODEL`/`LLM_API_KEY`. | [eval/policy-eval-correctness-prb.md](eval/policy-eval-correctness-prb.md) | +| `policy-eval-correctness-e2e` — `test_policy_pipeline_correctness_e2e.py` | Companion to `policy-eval-correctness-prb`, scoring the same 8-scenario corpus and the same reusable scorer one layer further downstream: real Keycloak provisioning → real Policy Rules Builder → real Policy Computation Engine → real `opa eval` against the rendered Rego, sourced from the rendered data maps (`subject_role_allow/deny_scopes`, `agent_role_scopes`) rather than per-pair decision probing. `@pytest.mark.eval_correctness_e2e`, same zero-tolerance over-grant gate. The shared `pipeline` fixture (`eval/test_policy_pipeline_eval.py`, also used by `eval_extended`) now provisions all 8 scenarios concurrently via `ProcessPoolExecutor`. Needs `KEYCLOAK_URL` + admin creds + `LLM_BASE_URL`/`LLM_MODEL`/`LLM_API_KEY`, plus `opa` on `PATH`. | [eval/policy-eval-correctness-e2e.md](eval/policy-eval-correctness-e2e.md) | Tracking issues: the live-Keycloak pytest integration tests in `testing/5.1-integration-tests.md`; the PDP Policy Writer integration test in `testing/5.2-pdp-writer-integration-test.md`; the policy-pipeline integration test in `testing/5.3-policy-pipeline-integration-test.md`; the UC-1 onboarding pipeline integration-test ladder in `testing/5.4-uc1-onboarding-integration-test.md` (epic) with rungs `testing/5.4.1`/`5.4.2`/`5.4.3` and the deferred two-policy `testing/5.4.4`. diff --git a/aiac/docs/specs/eval/policy-eval-correctness-e2e.md b/aiac/docs/specs/eval/policy-eval-correctness-e2e.md new file mode 100644 index 000000000..b9d773135 --- /dev/null +++ b/aiac/docs/specs/eval/policy-eval-correctness-e2e.md @@ -0,0 +1,211 @@ +# Eval Spec: policy-eval-correctness-e2e — `test_policy_pipeline_correctness_e2e.py` + +> **One spec among several.** This document specifies **one** integration test. +> Eval specs live **one spec per test** under `docs/specs/eval/` +> (a sibling of `components/`), and the master PRD's *Integration test specifications* section +> ([../PRD.md](../PRD.md)) is the index of them. This is a **companion to**, not a replacement +> for, [policy-eval-scenarios.md](policy-eval-scenarios.md) and +> [policy-eval-correctness-prb.md](policy-eval-correctness-prb.md): all three families reuse the +> same eight-scenario corpus (`SCENARIOS`, `truth`, from `eval/test_policy_pipeline_eval.py`), +> unmodified, but each isolates a different property or layer of the pipeline. + +## Location + +- `aiac/eval/correctness_scorer.py` — the reusable scorer (`score_gate`/`score_scenario`), + reused **unmodified** from #2089 — see + [policy-eval-correctness-prb.md § Scorer design](policy-eval-correctness-prb.md#scorer-design) + for the scorer itself; this spec does not repeat it. +- `aiac/eval/correctness_e2e_helpers.py` — this suite's own pure-logic helpers (`_pairs_from_map`, + `_user_role_rows`, `_accumulate_agent_gates`); `aiac/eval/test_correctness_e2e_helpers.py` — + their unmarked unit tests, runs in the default fast pass. Mirrors + `eval/correctness_scorer.py`/`eval/test_correctness_scorer.py`'s logic-module/test-module split. +- `aiac/eval/test_policy_pipeline_correctness_e2e.py` — the suite itself, + `@pytest.mark.eval_correctness_e2e`. +- Reuses, unmodified, from `eval.test_policy_pipeline_eval`: `SCENARIOS`, the `pipeline` fixture + (re-exported by import — see [Runbook](#runbook)), `_require_scenario`, `_rego_path`, `opa_bin`, + `opa_eval`, `truth`. + +## Description + +`policy-eval-correctness-prb.md`'s suite scores the PRB's raw grant/deny output directly — no +Keycloak, no PCE, no OPA. This suite scores the **same** primary corpus's grant/deny output one +layer further downstream: real Keycloak provisioning → real Policy Rules Builder → real Policy +Computation Engine → real `opa eval` against the rendered Rego. It is the only correctness check +that can catch an integration bug between those layers (PCE merge logic, Rego rendering, OPA +semantics) that a PRB-only check structurally cannot see — the PRB could return exactly the right +rules and a bug in the PCE's merge or in `rego.py`'s rendering could still produce the wrong +Rego. + +Same scoring philosophy as the PRB-level suite: precision and recall tracked separately per gate +and aggregated (never blended), a non-gating denial-precision figure, zero-tolerance on +over-grants, under-grants/incorrect denials reported only. + +### Scoring source: the rendered Rego data maps, not per-pair decision probing + +`opa eval` is queried directly against the plain data documents the Rego generator already +declares — `subject_role_allow_scopes`/`subject_role_deny_scopes` (inbound and outbound-subject) +and `agent_role_scopes` (outbound-target, ALLOW only) — rather than exhaustively probing every +`(role, scope)` pair's `allow`/`deny_ok` decision. Two `opa eval` calls per agent per direction +return the entire role→scopes table in one shot; `_pairs_from_map` flattens each into `(role, +scope)` pairs, and `_accumulate_agent_gates` unions every agent's pairs into the three top-level +gate buckets `score_scenario` expects. Both maps are already keyed by bare (deprefixed) role/scope +names — the same names the truth tables use — so no re-prefixing logic is needed. + +| Gate | Direction | ALLOW map | DENY map | +|---|---|---|---| +| `inbound` | inbound rego | `subject_role_allow_scopes` | `subject_role_deny_scopes` | +| `outbound_subject` | outbound rego | `subject_role_allow_scopes` | `subject_role_deny_scopes` | +| `outbound_target` | outbound rego | `agent_role_scopes` (deprefixed `outbound_target_allow_rules`) | **none** — see below | + +### Known gap: `outbound_target` denial is unrenderable, not just untested + +`AgentPolicyModel.outbound_target_deny_rules` is computed by the PCE, but +`aiac.pdp.service.policy.opa.rego.generate_outbound_rego` never renders it into any queryable Rego +document — only the ALLOW side (`agent_role_scopes`) is emitted. So for the `outbound_target` gate, +`denied` is always `set()`, and that gate's `denial_precision` reads vacuously `1.0`. +`over_grants`/`under_grants` for `outbound_target` are unaffected (they depend only on +`granted`/`expected`, not `denied`). **This is a pre-existing production Rego-generator gap, not +introduced by this suite and not fixed by it** — see [Out of Scope](#out-of-scope). + +## Configuration (env) + +| Variable | Purpose | +|---|---| +| `KEYCLOAK_URL` / `KEYCLOAK_ADMIN_USERNAME` / `KEYCLOAK_ADMIN_PASSWORD` | Real Keycloak admin API — the `pipeline` fixture provisions one realm per scenario. | +| `LLM_BASE_URL` / `LLM_MODEL` / `LLM_API_KEY` | The PRB's real LLM calls (`orchestrate_prb`, same as every other suite reusing this fixture). | +| `OPA_BIN` (optional) | Path to the `opa` binary; falls back to `opa` on `PATH`. The suite skips cleanly (not a failure) if neither resolves. | +| `EVAL_PIPELINE_PARALLELISM` (optional, default = scenario count, currently 8) | Max concurrent `ProcessPoolExecutor` workers in the shared `pipeline` fixture — see [Parallelization](#parallelization). An escape hatch, not a tuning knob with a "correct" lower value; lower it only if the LLM endpoint rate-limits under full concurrency. | + +`eval/conftest.py` auto-loads `eval/.env` (gitignored, override=False) if present, so a local +`eval/.env` with the above removes the need to `export`/source anything before invoking `pytest` +directly. + +## Parallelization + +The shared `pipeline` fixture (`eval/test_policy_pipeline_eval.py`, also used by the `eval_extended` +suite) provisions all 8 scenarios **concurrently**, one `ProcessPoolExecutor` worker per scenario — +not `pytest-xdist`'s `-n` (still unsupported/unneeded here: the parallelism lives inside the +fixture, across scenarios, not across pytest's own test-collection workers). + +Separate **processes**, not threads, because the fixture's per-scenario setup mutates +process-global state while it runs — `os.environ["KEYCLOAK_REALM"]`/`["AIAC_POLICY_FILE"]` +(read at call time by `compute_and_apply`/`FilePolicySource.fetch()`) and a `KeycloakAdmin` +connection's `change_current_realm` — which two *threads* sharing one process would race on, but +separate OS processes don't (each gets its own `os.environ` and its own `KeycloakAdmin`). Each +worker also gets its own idp/store/opa port triple (offset from the module defaults by scenario +index), since fixed ports collide regardless of thread vs. process. + +One further constraint, discovered empirically against a real Keycloak instance: concurrent +`admin.create_realm(...)` calls 409 with `"Duplicate resource error"` even across *distinct* realm +names — an internal Keycloak race on concurrent realm creation, not a naming collision on this +fixture's side. A `multiprocessing.Lock()`, passed to every worker via the pool's +`initializer`/`initargs` (a plain `multiprocessing.Lock()` cannot be passed as a regular per-task +argument under the `forkserver`/`spawn` start methods — only through that inheritance path), +serializes just the realm-provisioning step (`provision_keycloak_admin`). Everything after that per +scenario — the LLM-heavy `orchestrate_prb` calls and the idp/store/opa subprocess trio — still runs +fully concurrently; realm provisioning is a small fraction of one scenario's wall-clock next to the +PRB's several sequential LLM calls. + +This benefits every suite that shares the `pipeline` fixture (`eval_extended` primarily, plus this +suite), not just `eval_correctness_e2e` — see [Relationship to other integration +tests](#relationship-to-other-integration-tests). + +## Runbook + +```bash +# Unit-test the pure-logic helpers first (no live infra, runs in the default fast pass): +.venv/bin/pytest eval/test_correctness_e2e_helpers.py -v + +# The suite itself — needs KEYCLOAK_URL + admin creds + LLM_* + opa on PATH: +.venv/bin/pytest eval/test_policy_pipeline_correctness_e2e.py -m eval_correctness_e2e -v -s + +# Sanity-check the fixture's primary consumer still passes against the now-parallelized fixture: +.venv/bin/pytest eval/test_policy_pipeline_eval.py -m eval_extended -v +``` + +`require_env(...)` is the first line of the parametrized test function; `opa_bin()` skips the test +cleanly (not a failure) if `opa` isn't resolvable. Same skip-clean philosophy as every sibling +suite — this suite never false-passes when its infra isn't available. + +## Expected output + +Parametrized over all 8 scenario names (`sorted(SCENARIOS)`); expects all 8 to pass (zero +over-grants) given a healthy Keycloak instance and a well-behaved LLM endpoint. In practice, a real +run against the rossoctl kind cluster currently shows 6/8 passing and 2/8 failing at the *setup* +stage (`PolicyContradictionError`/`PolicyRulesBuilderError` from the PRB's own audit/retry loop, +`aiac.agent.policy_rules_builder.graph._audit`) — this is the **pre-existing, already-deferred** +audit/retry-convergence bug (the auditor rejects the generator's proposal identically on all 3 +retries for `agent_delegation` and, in this run, `unreachable_resources`), confirmed to reproduce +identically in the PRB-direct `test_policy_pipeline_correctness_prb.py` suite (no Keycloak/PCE/OPA +involved at all), so it is unrelated to this suite, to the pipeline fixture's parallelization, or +to anything else changed here. Not fixed as part of this ticket — see [Out of +Scope](#out-of-scope). Each test case +`record_property`s `precision`, `recall`, `denial_precision`, `over_grants`, `under_grants`, and +`incorrectly_denied` (the latter three as `{gate: sorted(pairs)}`), and prints: + +```text +[correctness-e2e] wildcard_grant: precision=1.000 recall=1.000 denial_precision=1.000 + over_grants={} + under_grants={} + incorrectly_denied={} +``` + +A failing case's assertion message names the scenario and the exact over-granted `(role, scope)` +pairs per gate. + +## Test report + +Covered by `eval/conftest.py`'s existing Markdown report and its generic +`"precision" in props and "recall" in props` render-branch dispatch (already used by +`test_prb_correctness`) — no per-suite special-casing was needed; `eval_correctness_e2e` was simply +added to the `MARKERS` set. See +[policy-eval-correctness-prb.md § Test report](policy-eval-correctness-prb.md#test-report) for the +render behavior itself. + +## Relationship to other integration tests + +This is **one** integration-test spec among several indexed by the master PRD +([../PRD.md](../PRD.md), § *Integration test specifications*). + +- **Companion to, not a replacement for, + [policy-eval-correctness-prb.md](policy-eval-correctness-prb.md).** Same corpus, same scorer, + same philosophy — the PRB-level suite isolates the PRB's own grant decisions with no + Keycloak/PCE/OPA in the loop; this suite scores the same corpus one layer further downstream, + through the full pipeline, and is the only one of the two that can catch a PCE-merge or + Rego-rendering bug. +- **Shares the `pipeline` fixture with `eval_extended`** (`test_policy_pipeline_eval.py`) — the + parallelization work described above (see [Parallelization](#parallelization)) speeds up both + suites, since it lives in the shared fixture, not in this suite's own file. +- **New marker, registered in `pyproject.toml`** (`eval_correctness_e2e`), distinct from + `eval_extended`/`eval_consistency`/`eval_robustness`/`eval_correctness_prb`. + +## Out of Scope + +- **Fixing the PRB audit/retry-convergence bug** (`aiac.agent.policy_rules_builder.graph._audit`, + around line 200-220) that causes `agent_delegation`/`unreachable_resources` (or, on other runs, + `confusable_agents` — which scenario(s) hit it varies with LLM sampling, but at least one of + `agent_delegation`/`confusable_agents` reproduces consistently) to fail at setup with + `PolicyContradictionError`/`PolicyRulesBuilderError`. Confirmed pre-existing and unrelated to + this suite (reproduces identically in the PRB-direct `eval_correctness_prb` suite). The user has + already deferred fixing `graph.py` itself as a separate follow-up — not this ticket's job. +- **Fixing the `outbound_target` denial-rendering gap** in + `aiac.pdp.service.policy.opa.rego.generate_outbound_rego` — see + [Known gap](#known-gap-outbound_target-denial-is-unrenderable-not-just-untested). This is + production code, unrelated to this eval suite's own scope; the gap is documented and reported + (vacuous `denial_precision=1.0` for that one gate), not silently hidden. +- **A committed trend log** — same deferral as + [policy-eval-correctness-prb.md § Out of Scope](policy-eval-correctness-prb.md#out-of-scope), + #2091. +- **An under-grant tolerance threshold.** Still TBD per the originating spec; under-grants are + tracked/reported only, never gated. +- **New scenarios.** Reuses the existing 8-scenario corpus unmodified — see + [policy-eval-correctness-prb.md § Taxonomy cross-check](policy-eval-correctness-prb.md#taxonomy-cross-check). +- **Parallelizing across pytest itself (`-n`/xdist).** The parallelism lives inside the shared + `pipeline` fixture (see [Parallelization](#parallelization)), which already gets the wall-clock + benefit without the 8x duplicated-provisioning problem `-n` would cause for a suite built on one + session-scoped fixture. + +## Blocked-by + +None — #2089 (the PRB-level suite and `correctness_scorer.py`) is done, and this suite is the last +consumer that `correctness_scorer.py` was explicitly designed to support. diff --git a/aiac/eval/correctness_e2e_helpers.py b/aiac/eval/correctness_e2e_helpers.py new file mode 100644 index 000000000..b809fc17b --- /dev/null +++ b/aiac/eval/correctness_e2e_helpers.py @@ -0,0 +1,51 @@ +"""Pure-logic helpers for ``test_policy_pipeline_correctness_e2e.py`` — no LLM, no Keycloak, no +``opa``, no I/O. Mirrors ``correctness_scorer.py``'s split from its own test module +(``test_correctness_scorer.py``): logic lives here (no ``test_`` prefix, not collected by +pytest), unit tests live in ``test_correctness_e2e_helpers.py`` (unmarked, runs in the default +fast pass). +""" + +from __future__ import annotations + +Pair = tuple[str, str] + + +def _pairs_from_map(role_to_scopes: dict[str, list[str]]) -> set[Pair]: + """Flatten a rendered Rego ``{role_name: [scope_name, ...]}`` map (e.g. + ``subject_role_allow_scopes``) into a set of ``(role, scope)`` pairs.""" + return {(role, scope) for role, scopes in role_to_scopes.items() for scope in scopes} + + +def _user_role_rows(role_to_scopes: dict[str, list[str]], user_roles: set[str]) -> dict[str, list[str]]: + """Keep only the rows of a rendered ``subject_role_*_scopes`` map whose role is a genuine user + role of the scenario. That map's "subject" isn't exclusively human — an agent calling another + agent (outbound-target, ``agent_role_scopes``) is rendered into the *same* + ``subject_role_allow_scopes`` document as real user-role grants, since the outbound Rego's + ``subject_role`` doesn't distinguish the caller's role by human-vs-agent, only by role name. + Without this filter, every agent-role row double-counts as an ``outbound_subject`` over-grant + even though it's already correctly counted under ``outbound_target`` — confirmed empirically: + a real run's every over-grant was exactly its scenario's ``outbound_target`` true positives, + reappearing here. Mirrors the ``role.name in user_role_names`` discrimination + ``eval.test_policy_pipeline_eval.grant_sets`` already applies to the PRB's raw rules.""" + return {role: scopes for role, scopes in role_to_scopes.items() if role in user_roles} + + +def _accumulate_agent_gates( + granted: dict[str, set[Pair]], + denied: dict[str, set[Pair]], + *, + inbound_allow: dict[str, list[str]], + inbound_deny: dict[str, list[str]], + outbound_subject_allow: dict[str, list[str]], + outbound_subject_deny: dict[str, list[str]], + outbound_target_allow: dict[str, list[str]], +) -> None: + """Union one agent's rendered Rego maps into the three top-level gate buckets + (``inbound``/``outbound_subject``/``outbound_target``), in place. No ``outbound_target_deny`` + parameter — the outbound Rego generator never renders that map (see + ``test_policy_pipeline_correctness_e2e.py``'s module docstring).""" + granted.setdefault("inbound", set()).update(_pairs_from_map(inbound_allow)) + denied.setdefault("inbound", set()).update(_pairs_from_map(inbound_deny)) + granted.setdefault("outbound_subject", set()).update(_pairs_from_map(outbound_subject_allow)) + denied.setdefault("outbound_subject", set()).update(_pairs_from_map(outbound_subject_deny)) + granted.setdefault("outbound_target", set()).update(_pairs_from_map(outbound_target_allow)) diff --git a/aiac/eval/test_correctness_e2e_helpers.py b/aiac/eval/test_correctness_e2e_helpers.py new file mode 100644 index 000000000..2655858e3 --- /dev/null +++ b/aiac/eval/test_correctness_e2e_helpers.py @@ -0,0 +1,90 @@ +"""Unit tests for ``correctness_e2e_helpers.py`` (spec: ``docs/specs/eval/ +policy-eval-correctness-e2e.md``). + +Pure-logic, unmarked — runs in the default fast pass (``testpaths`` already includes ``eval/``), +mirroring ``test_correctness_scorer.py``'s status as a separate, unmarked file. No LLM, no +Keycloak, no ``opa``, no fixtures beyond plain dicts/sets — the module-level +``pytestmark = pytest.mark.eval_correctness_e2e`` in ``test_policy_pipeline_correctness_e2e.py`` +would otherwise wrongly sweep these up and exclude them from the default pass. +""" + +from __future__ import annotations + +from eval.correctness_e2e_helpers import _accumulate_agent_gates, _pairs_from_map, _user_role_rows + + +def test_pairs_from_map_flattens_role_scope_map() -> None: + got = _pairs_from_map({"role-a": ["scope-x", "scope-y"], "role-b": ["scope-x"]}) + assert got == {("role-a", "scope-x"), ("role-a", "scope-y"), ("role-b", "scope-x")} + + +def test_pairs_from_map_empty_map_yields_no_pairs() -> None: + assert _pairs_from_map({}) == set() + + +def test_user_role_rows_drops_agent_role_rows() -> None: + rows = { + "user-role-inventory-manager": ["tool-scope-inventory-check"], + "agent-role-inventory-operations": ["tool-scope-inventory-check"], + } + got = _user_role_rows(rows, user_roles={"user-role-inventory-manager"}) + assert got == {"user-role-inventory-manager": ["tool-scope-inventory-check"]} + + +def test_user_role_rows_empty_map_yields_empty_map() -> None: + assert _user_role_rows({}, user_roles={"user-role-x"}) == {} + + +def test_accumulate_agent_gates_classifies_into_three_buckets() -> None: + granted: dict[str, set[tuple[str, str]]] = {} + denied: dict[str, set[tuple[str, str]]] = {} + + _accumulate_agent_gates( + granted, + denied, + inbound_allow={"user-role-developer": ["agent-scope-repo-access"]}, + inbound_deny={"user-role-devops": ["agent-scope-repo-access"]}, + outbound_subject_allow={"user-role-developer": ["tool-scope-repo-read"]}, + outbound_subject_deny={}, + outbound_target_allow={"agent-role-repo-operations": ["tool-scope-repo-read"]}, + ) + + assert granted == { + "inbound": {("user-role-developer", "agent-scope-repo-access")}, + "outbound_subject": {("user-role-developer", "tool-scope-repo-read")}, + "outbound_target": {("agent-role-repo-operations", "tool-scope-repo-read")}, + } + assert denied == { + "inbound": {("user-role-devops", "agent-scope-repo-access")}, + "outbound_subject": set(), + } + assert "outbound_target" not in denied # never rendered — see module docstring + + +def test_accumulate_agent_gates_unions_across_multiple_agents() -> None: + granted: dict[str, set[tuple[str, str]]] = {} + denied: dict[str, set[tuple[str, str]]] = {} + + _accumulate_agent_gates( + granted, + denied, + inbound_allow={"user-role-developer": ["agent-scope-repo-access"]}, + inbound_deny={}, + outbound_subject_allow={}, + outbound_subject_deny={}, + outbound_target_allow={}, + ) + _accumulate_agent_gates( + granted, + denied, + inbound_allow={"user-role-tester": ["agent-scope-tracker-access"]}, + inbound_deny={}, + outbound_subject_allow={}, + outbound_subject_deny={}, + outbound_target_allow={}, + ) + + assert granted["inbound"] == { + ("user-role-developer", "agent-scope-repo-access"), + ("user-role-tester", "agent-scope-tracker-access"), + } diff --git a/aiac/eval/test_policy_pipeline_correctness_e2e.py b/aiac/eval/test_policy_pipeline_correctness_e2e.py index fc4d27225..5fe809676 100644 --- a/aiac/eval/test_policy_pipeline_correctness_e2e.py +++ b/aiac/eval/test_policy_pipeline_correctness_e2e.py @@ -27,9 +27,11 @@ ``docs/specs/eval/policy-eval-correctness-e2e.md`` for the full writeup — this is a pre-existing production Rego-generator gap, not something this suite introduces or is scoped to fix. -Run (needs KEYCLOAK_URL + admin creds + LLM_* exported, ``opa`` on PATH; unlike the PRB-level -suite, this one is NOT `-n`-parallelizable — all 8 scenarios share one session-scoped ``pipeline`` -fixture that provisions Keycloak sequentially in-process): +Run (needs KEYCLOAK_URL + admin creds + LLM_* exported, ``opa`` on PATH). Unlike the PRB-level +suite, this one is NOT `-n`-parallelizable (all 8 scenarios share one session-scoped ``pipeline`` +fixture) — but that fixture now parallelizes its own scenario provisioning internally via +``ProcessPoolExecutor`` (see ``eval/test_policy_pipeline_eval.py``'s module docstring), so `-n` +was never needed for wall-clock gains here: .venv/bin/pytest eval/test_policy_pipeline_correctness_e2e.py \ -m eval_correctness_e2e -v -s """ @@ -50,11 +52,13 @@ sys.path.insert(0, str(REPO_ROOT)) # so ``import test.integration.*``/``eval.*`` resolves sys.path.insert(0, str(SRC)) # so ``import aiac.*`` resolves +from eval.correctness_e2e_helpers import _accumulate_agent_gates, _user_role_rows # noqa: E402 from eval.correctness_scorer import score_scenario # noqa: E402 from eval.test_policy_pipeline_eval import ( # noqa: E402 SCENARIOS, _rego_path, _require_scenario, + _user_role_names, opa_bin, opa_eval, truth, @@ -65,96 +69,6 @@ Pair = tuple[str, str] -def _pairs_from_map(role_to_scopes: dict[str, list[str]]) -> set[Pair]: - """Flatten a rendered Rego ``{role_name: [scope_name, ...]}`` map (e.g. - ``subject_role_allow_scopes``) into a set of ``(role, scope)`` pairs.""" - return {(role, scope) for role, scopes in role_to_scopes.items() for scope in scopes} - - -def test_pairs_from_map_flattens_role_scope_map() -> None: - got = _pairs_from_map({"role-a": ["scope-x", "scope-y"], "role-b": ["scope-x"]}) - assert got == {("role-a", "scope-x"), ("role-a", "scope-y"), ("role-b", "scope-x")} - - -def test_pairs_from_map_empty_map_yields_no_pairs() -> None: - assert _pairs_from_map({}) == set() - - -def _accumulate_agent_gates( - granted: dict[str, set[Pair]], - denied: dict[str, set[Pair]], - *, - inbound_allow: dict[str, list[str]], - inbound_deny: dict[str, list[str]], - outbound_subject_allow: dict[str, list[str]], - outbound_subject_deny: dict[str, list[str]], - outbound_target_allow: dict[str, list[str]], -) -> None: - """Union one agent's rendered Rego maps into the three top-level gate buckets - (``inbound``/``outbound_subject``/``outbound_target``), in place. No ``outbound_target_deny`` - parameter — the outbound Rego generator never renders that map (see module docstring).""" - granted.setdefault("inbound", set()).update(_pairs_from_map(inbound_allow)) - denied.setdefault("inbound", set()).update(_pairs_from_map(inbound_deny)) - granted.setdefault("outbound_subject", set()).update(_pairs_from_map(outbound_subject_allow)) - denied.setdefault("outbound_subject", set()).update(_pairs_from_map(outbound_subject_deny)) - granted.setdefault("outbound_target", set()).update(_pairs_from_map(outbound_target_allow)) - - -def test_accumulate_agent_gates_classifies_into_three_buckets() -> None: - granted: dict[str, set[Pair]] = {} - denied: dict[str, set[Pair]] = {} - - _accumulate_agent_gates( - granted, - denied, - inbound_allow={"user-role-developer": ["agent-scope-repo-access"]}, - inbound_deny={"user-role-devops": ["agent-scope-repo-access"]}, - outbound_subject_allow={"user-role-developer": ["tool-scope-repo-read"]}, - outbound_subject_deny={}, - outbound_target_allow={"agent-role-repo-operations": ["tool-scope-repo-read"]}, - ) - - assert granted == { - "inbound": {("user-role-developer", "agent-scope-repo-access")}, - "outbound_subject": {("user-role-developer", "tool-scope-repo-read")}, - "outbound_target": {("agent-role-repo-operations", "tool-scope-repo-read")}, - } - assert denied == { - "inbound": {("user-role-devops", "agent-scope-repo-access")}, - "outbound_subject": set(), - } - assert "outbound_target" not in denied # never rendered — see module docstring - - -def test_accumulate_agent_gates_unions_across_multiple_agents() -> None: - granted: dict[str, set[Pair]] = {} - denied: dict[str, set[Pair]] = {} - - _accumulate_agent_gates( - granted, - denied, - inbound_allow={"user-role-developer": ["agent-scope-repo-access"]}, - inbound_deny={}, - outbound_subject_allow={}, - outbound_subject_deny={}, - outbound_target_allow={}, - ) - _accumulate_agent_gates( - granted, - denied, - inbound_allow={"user-role-tester": ["agent-scope-tracker-access"]}, - inbound_deny={}, - outbound_subject_allow={}, - outbound_subject_deny={}, - outbound_target_allow={}, - ) - - assert granted["inbound"] == { - ("user-role-developer", "agent-scope-repo-access"), - ("user-role-tester", "agent-scope-tracker-access"), - } - - # ====================================================================================== # opa eval wiring — thin, untested-in-isolation (same status as opa_eval itself, reused # unmodified from eval.test_policy_pipeline_eval) @@ -178,6 +92,7 @@ def _e2e_grant_sets(pipeline_result: dict, scenario: ModuleType) -> tuple[dict[s denial gap. Agents with no rego on disk (declared/emergent ``EXPECT_NO_REGO``) contribute nothing, same as ``test_inbound``/``test_outbound`` already handle it.""" rego_dir = pipeline_result["rego_dir"] + user_roles = _user_role_names(scenario) granted: dict[str, set[Pair]] = {} denied: dict[str, set[Pair]] = {} @@ -187,10 +102,18 @@ def _e2e_grant_sets(pipeline_result: dict, scenario: ModuleType) -> tuple[dict[s _accumulate_agent_gates( granted, denied, - inbound_allow=_rego_map(inbound_rego, "inbound.request.subject_role_allow_scopes"), - inbound_deny=_rego_map(inbound_rego, "inbound.request.subject_role_deny_scopes"), - outbound_subject_allow=_rego_map(outbound_rego, "outbound.request.subject_role_allow_scopes"), - outbound_subject_deny=_rego_map(outbound_rego, "outbound.request.subject_role_deny_scopes"), + inbound_allow=_user_role_rows( + _rego_map(inbound_rego, "inbound.request.subject_role_allow_scopes"), user_roles + ), + inbound_deny=_user_role_rows( + _rego_map(inbound_rego, "inbound.request.subject_role_deny_scopes"), user_roles + ), + outbound_subject_allow=_user_role_rows( + _rego_map(outbound_rego, "outbound.request.subject_role_allow_scopes"), user_roles + ), + outbound_subject_deny=_user_role_rows( + _rego_map(outbound_rego, "outbound.request.subject_role_deny_scopes"), user_roles + ), outbound_target_allow=_rego_map(outbound_rego, "outbound.request.agent_role_scopes"), ) return granted, denied diff --git a/aiac/eval/test_policy_pipeline_eval.py b/aiac/eval/test_policy_pipeline_eval.py index 80f7cbd67..ddbeddb45 100644 --- a/aiac/eval/test_policy_pipeline_eval.py +++ b/aiac/eval/test_policy_pipeline_eval.py @@ -45,17 +45,39 @@ Without ``-m eval_extended`` the suite is skipped; without ``opa`` each node skips at runtime. This suite is heavier than ``test_policy_pipeline.py`` (eight full pipeline runs, more PRB/LLM calls) hence the separate marker. + +The ``pipeline`` fixture provisions all 8 scenarios in parallel via ``ProcessPoolExecutor`` +(``EVAL_PIPELINE_PARALLELISM``, default = scenario count) — separate OS processes, not threads: +each scenario mutates process-global state while it runs (``os.environ["KEYCLOAK_REALM"]``/ +``["AIAC_POLICY_FILE"]``, consumed at call time by ``compute_and_apply``/ +``FilePolicySource.fetch()``; a ``KeycloakAdmin`` connection's ``change_current_realm``), which +two threads sharing one process would race on but separate processes don't. Each worker also gets +its own idp/store/opa port triple (offset from the module defaults by scenario index) since fixed +ports collide regardless of thread vs. process. This is *not* the same thing as pytest-xdist's +``-n`` (still not supported/needed here — the parallelism is inside the fixture, not across +pytest workers). + +A shared ``multiprocessing.Lock()`` serializes just the ``provision_keycloak_admin`` step (realm + +role/user/client creation) across workers — concurrent ``admin.create_realm(...)`` calls against a +real Keycloak instance were observed to 409 with ``"Duplicate resource error"`` even across +*distinct* realm names (an internal Keycloak race on concurrent realm creation, not a naming +collision on this fixture's side). Everything after that per scenario — the LLM-heavy +``orchestrate_prb`` calls and the idp/store/opa subprocesses — still runs fully concurrently, since +realm provisioning is a small fraction of one scenario's wall-clock next to the PRB's several +sequential LLM calls. """ from __future__ import annotations import json import logging +import multiprocessing import os import shutil import subprocess import sys import tempfile +from concurrent.futures import ProcessPoolExecutor, as_completed from pathlib import Path from types import ModuleType from typing import Any @@ -330,9 +352,11 @@ def opa_bin() -> str: return found -def opa_eval(rego_paths: list[Path], query: str, input_doc: dict) -> bool: +def opa_eval(rego_paths: list[Path], query: str, input_doc: dict) -> Any: """Evaluate ``query`` against the given Rego file(s) with ``input_doc`` on stdin; return the - boolean result. Raises (via ``check=True``) if OPA rejects the Rego or the query errors.""" + decoded JSON result — a ``bool`` for a decision query (e.g. ``...request.allow``), or a data + document (e.g. a ``{role: [scope, ...]}`` map) for a plain declaration. Raises (via + ``check=True``) if OPA rejects the Rego or the query errors.""" cmd = [ opa_bin(), "eval", @@ -504,23 +528,148 @@ def truth(scenario: ModuleType) -> dict[str, set[tuple[str, str]]]: } +# ====================================================================================== +# ProcessPoolExecutor worker init — a multiprocessing.Lock() can only reach a worker via the +# pool's initializer/initargs ("inheritance" at process-start time), not as a regular per-task +# submit() argument — passing one through the ongoing call queue instead raises "Lock objects +# should only be shared between processes through inheritance" under the forkserver/spawn start +# methods (this Python's default is forkserver; plain fork wouldn't need this, but forkserver +# does). See ``_provision_scenario`` for why the lock exists at all. +# ====================================================================================== + +_REALM_LOCK: Any = None + + +def _init_worker(realm_lock: Any) -> None: + global _REALM_LOCK + _REALM_LOCK = realm_lock + + # ====================================================================================== # Session fixture — one pipeline run per scenario # ====================================================================================== +def _provision_scenario(name: str, idp_port: int, store_port: int, opa_port: int) -> dict: + """Provision one scenario's realm and run the real PRB+PCE pipeline, leaving ``.rego`` on disk + under ``rego_out/policy_pipeline_eval//``. Returns ``{"rego_dir": Path, "rules": + list[PolicyRule], "reasoning_by_scope": dict[str, str], "reasoning_by_agent_role": dict[str, + str]}`` (the two reasoning dicts feed the eval report's per-cell "Output" field, see + ``conftest.py``), or ``{"error": }`` on failure. + + Fully self-contained — own ``KeycloakAdmin`` connection, own env-var writes, own + idp/store/opa ports — so it can run as an independent ``ProcessPoolExecutor`` worker (see the + module docstring for why a *shared* admin object / shared ``os.environ`` would race under + threads). Takes ``name`` and looks ``SCENARIOS[name]`` up itself rather than receiving the + scenario ``ModuleType`` as a parameter — module objects aren't picklable, and every argument + here has to survive being pickled across the process boundary to the worker. The error return + is stringified for the same reason: not every exception type is guaranteed picklable back. + + ``_REALM_LOCK`` (a ``multiprocessing.Lock()`` set once per worker by ``_init_worker``, see + above) serializes just ``provision_keycloak_admin`` — concurrent ``admin.create_realm(...)`` + calls against this Keycloak instance were observed to 409 with ``"Duplicate resource error"`` + even across *distinct* realm names, i.e. an internal Keycloak race on concurrent realm + creation, not a bug in this fixture's realm naming. Everything after that (the LLM-heavy + ``orchestrate_prb`` calls, the idp/store/opa subprocesses) stays fully concurrent — realm + provisioning is a small fraction of one scenario's wall-clock next to the PRB's several + sequential LLM calls. + """ + try: + scenario = SCENARIOS[name] + idp_host, _ = _host_port(os.environ["AIAC_PDP_CONFIG_URL"], DEFAULT_IDP_PORT) + store_host, _ = _host_port(os.environ["AIAC_POLICY_STORE_URL"], DEFAULT_STORE_PORT) + opa_host, _ = _host_port(os.environ["AIAC_PDP_POLICY_URL"], DEFAULT_OPA_PORT) + + admin = _connect_admin() + os.environ["KEYCLOAK_REALM"] = scenario.REALM_DEFAULT # PCE reads this back + with _REALM_LOCK: + provision_keycloak_admin(admin, scenario.REALM_DEFAULT, scenario) + + rego_dir = HERE / "rego_out" / "policy_pipeline_eval" / name + if rego_dir.exists(): + shutil.rmtree(rego_dir) + rego_dir.mkdir(parents=True) + db_path = Path(tempfile.mkdtemp(prefix=f"aiac-store-eval-{name}-")) / "policy_model.db" + scenario_dir = Path(scenario.__file__).resolve().parent + os.environ["AIAC_POLICY_FILE"] = str(scenario_dir / scenario.POLICY_FILE) + os.environ["AIAC_PDP_CONFIG_URL"] = f"http://{idp_host}:{idp_port}" + os.environ["AIAC_POLICY_STORE_URL"] = f"http://{store_host}:{store_port}" + # The model-store client actually reads AIAC_POLICY_MODEL_STORE_URL, not + # AIAC_POLICY_STORE_URL (which nothing consumes) — set both so this worker's PCE calls + # land on its own store subprocess instead of every worker colliding on the hardcoded + # 127.0.0.1:7074 default once ports diverge per worker. + os.environ["AIAC_POLICY_MODEL_STORE_URL"] = f"http://{store_host}:{store_port}" + os.environ["AIAC_PDP_POLICY_URL"] = f"http://{opa_host}:{opa_port}" + log.info( + "scenario %s: realm=%s policy=%s rego_dir=%s ports=(idp=%d store=%d opa=%d)", + name, + scenario.REALM_DEFAULT, + os.environ["AIAC_POLICY_FILE"], + rego_dir, + idp_port, + store_port, + opa_port, + ) + + idp = Service("aiac.idp.service.configuration.keycloak.main:app", port=idp_port, host=idp_host) + store = Service( + "aiac.policy.model_store.service.main:app", + port=store_port, + host=store_host, + env={"SERVICEPOLICY_DB_PATH": str(db_path)}, + ) + opa = Service( + "aiac.pdp.service.policy.opa.main:app", + port=opa_port, + host=opa_host, + env={"REGO_OUTPUT_DIR": str(rego_dir), "POLICY_WRITER_DUMP_REGO": "true"}, + ) + with running_services([idp, store, opa], src=SRC): + config = Configuration.for_realm(scenario.REALM_DEFAULT) + provision_via_config(config, scenario) # exactly once — not idempotent + roles, scopes = _read_back(config) + rules, reasoning_by_scope, reasoning_by_agent_role = orchestrate_prb(roles, scopes, scenario) + compute_and_apply(rules, override=False) + + # Assert every agent's rego actually landed here at setup — EXCEPT agents the scenario + # itself declares as deliberately/emergently unreachable (Scenario 4), so a real + # pipeline failure still surfaces as one clear error instead of cryptic per-test skips. + allow_missing = set(getattr(scenario, "EXPECT_NO_REGO", frozenset())) + expected = [ + _rego_path(rego_dir, agent_id, direction) + for agent_id in scenario.AGENTS + for direction in ("inbound", "outbound") + if agent_id not in allow_missing + ] + missing = [str(p.relative_to(rego_dir)) for p in expected if not p.is_file()] + if missing: + raise RuntimeError( + f"scenario {name!r}: compute_and_apply produced no {missing} in {rego_dir} " + f"(PRB returned {len(rules)} rule(s)); the pipeline failed silently — " + f"check the compute_and_apply logs above for a swallowed exception." + ) + return { + "rego_dir": rego_dir, + "rules": rules, + "reasoning_by_scope": reasoning_by_scope, + "reasoning_by_agent_role": reasoning_by_agent_role, + } + except Exception as exc: # noqa: BLE001 - isolate one scenario's setup failure from the rest + log.exception("scenario %s: setup failed, isolating from the rest of the session", name) + return {"error": f"{exc!r}"} + + @pytest.fixture(scope="session") def pipeline() -> dict[str, dict]: - """Provision Keycloak and run the real PRB+PCE pipeline once per scenario, leaving ``.rego`` on - disk under ``rego_out/policy_pipeline_eval//``. Returns ``{scenario_name: {"rego_dir": - Path, "rules": list[PolicyRule], "reasoning_by_scope": dict[str, str], - "reasoning_by_agent_role": dict[str, str]}}`` — the two reasoning dicts feed the eval report's - per-cell "Output" field (see ``conftest.py``). - - Each scenario gets its own realm (``scenario.REALM_DEFAULT``) and its own fresh IdP/Store/OPA - subprocess trio — unlike ``test_policy_pipeline.py``'s two variants (which share one realm and - reuse a single IdP process), these scenarios' realms differ, so nothing can safely be kept warm - across them. + """Provision Keycloak and run the real PRB+PCE pipeline once per scenario — in parallel, one + ``ProcessPoolExecutor`` worker per scenario (see the module docstring) — leaving ``.rego`` on + disk under ``rego_out/policy_pipeline_eval//``. Returns ``{scenario_name: + _provision_scenario(...)}`` for every scenario. + + Each scenario gets its own realm (``scenario.REALM_DEFAULT``), its own fresh IdP/Store/OPA + subprocess trio, and its own port triple (offset from the module defaults by scenario index) — + unlike ``test_policy_pipeline.py``'s two variants (which share one realm and reuse a single IdP + process), these scenarios' realms differ, so nothing can safely be kept warm across them. """ require_env( "KEYCLOAK_URL", @@ -531,76 +680,27 @@ def pipeline() -> dict[str, dict]: "LLM_API_KEY", ) - admin = _connect_admin() - - idp_host, idp_port = _host_port(os.environ["AIAC_PDP_CONFIG_URL"], DEFAULT_IDP_PORT) - store_host, store_port = _host_port(os.environ["AIAC_POLICY_STORE_URL"], DEFAULT_STORE_PORT) - opa_host, opa_port = _host_port(os.environ["AIAC_PDP_POLICY_URL"], DEFAULT_OPA_PORT) - + max_workers = int(os.environ.get("EVAL_PIPELINE_PARALLELISM", str(len(SCENARIOS)))) + realm_lock = multiprocessing.Lock() # serializes admin.create_realm — see _provision_scenario results: dict[str, dict] = {} - for name, scenario in SCENARIOS.items(): - try: - os.environ["KEYCLOAK_REALM"] = scenario.REALM_DEFAULT # PCE reads this back - provision_keycloak_admin(admin, scenario.REALM_DEFAULT, scenario) - - rego_dir = HERE / "rego_out" / "policy_pipeline_eval" / name - if rego_dir.exists(): - shutil.rmtree(rego_dir) - rego_dir.mkdir(parents=True) - db_path = Path(tempfile.mkdtemp(prefix=f"aiac-store-eval-{name}-")) / "policy_model.db" - scenario_dir = Path(scenario.__file__).resolve().parent - os.environ["AIAC_POLICY_FILE"] = str(scenario_dir / scenario.POLICY_FILE) - log.info( - "scenario %s: realm=%s policy=%s rego_dir=%s", - name, scenario.REALM_DEFAULT, os.environ["AIAC_POLICY_FILE"], rego_dir, - ) - - idp = Service("aiac.idp.service.configuration.keycloak.main:app", port=idp_port, host=idp_host) - store = Service( - "aiac.policy.model_store.service.main:app", - port=store_port, - host=store_host, - env={"SERVICEPOLICY_DB_PATH": str(db_path)}, - ) - opa = Service( - "aiac.pdp.service.policy.opa.main:app", - port=opa_port, - host=opa_host, - env={"REGO_OUTPUT_DIR": str(rego_dir), "POLICY_WRITER_DUMP_REGO": "true"}, - ) - with running_services([idp, store, opa], src=SRC): - config = Configuration.for_realm(scenario.REALM_DEFAULT) - provision_via_config(config, scenario) # exactly once — not idempotent - roles, scopes = _read_back(config) - rules, reasoning_by_scope, reasoning_by_agent_role = orchestrate_prb(roles, scopes, scenario) - compute_and_apply(rules, override=False) - - # Assert every agent's rego actually landed here at setup — EXCEPT agents the scenario - # itself declares as deliberately/emergently unreachable (Scenario 4), so a real - # pipeline failure still surfaces as one clear error instead of cryptic per-test skips. - allow_missing = set(getattr(scenario, "EXPECT_NO_REGO", frozenset())) - expected = [ - _rego_path(rego_dir, agent_id, direction) - for agent_id in scenario.AGENTS - for direction in ("inbound", "outbound") - if agent_id not in allow_missing - ] - missing = [str(p.relative_to(rego_dir)) for p in expected if not p.is_file()] - if missing: - raise RuntimeError( - f"scenario {name!r}: compute_and_apply produced no {missing} in {rego_dir} " - f"(PRB returned {len(rules)} rule(s)); the pipeline failed silently — " - f"check the compute_and_apply logs above for a swallowed exception." - ) - results[name] = { - "rego_dir": rego_dir, - "rules": rules, - "reasoning_by_scope": reasoning_by_scope, - "reasoning_by_agent_role": reasoning_by_agent_role, - } - except Exception as exc: # noqa: BLE001 - isolate one scenario's setup failure from the rest - log.exception("scenario %s: setup failed, isolating from the rest of the session", name) - results[name] = {"error": exc} + with ProcessPoolExecutor(max_workers=max_workers, initializer=_init_worker, initargs=(realm_lock,)) as executor: + futures = { + executor.submit( + _provision_scenario, + name, + DEFAULT_IDP_PORT + i * 10, + DEFAULT_STORE_PORT + i * 10, + DEFAULT_OPA_PORT + i * 10, + ): name + for i, name in enumerate(SCENARIOS) + } + for future in as_completed(futures): + name = futures[future] + try: + results[name] = future.result() + except Exception as exc: # noqa: BLE001 - isolate one scenario's worker crash from the rest + log.exception("scenario %s: worker crashed, isolating from the rest of the session", name) + results[name] = {"error": f"{exc!r}"} yield results From 649e3510ae470b70fd0287b4953f6f7f91959147 Mon Sep 17 00:00:00 2001 From: Amitfre15 Date: Tue, 8 Sep 2026 11:56:29 +0300 Subject: [PATCH 3/6] fix: Show correctness-suite metrics for setup-failed scenarios in the report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scenario whose own setup fails (a PRB/Keycloak/PCE error) never reaches score_scenario, so test_prb_correctness/test_e2e_correctness never record_property the precision/recall/denial-precision/over-under-grant fields. The report's render branch dispatched on those properties' presence, so a setup-failed entry silently fell back to the generic docstring + crash-message rendering, with no metrics fields at all. Give it the same six-field shape every other entry gets, values marked "unavailable — scenario setup failed before scoring could run", identified by nodeid (::test_prb_correctness[/::test_e2e_correctness[) since there are no properties to dispatch on in this case. Assisted-By: Claude (Anthropic AI) Signed-off-by: Amitfre15 --- aiac/eval/conftest.py | 52 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/aiac/eval/conftest.py b/aiac/eval/conftest.py index f9e99b2b8..ead7d4ba7 100644 --- a/aiac/eval/conftest.py +++ b/aiac/eval/conftest.py @@ -37,6 +37,14 @@ detail is otherwise invisible on a passing run. The render branch dispatches generically on the presence of ``precision``/``recall`` properties, so it covers both suites with no per-suite special-casing. + +A scenario whose own *setup* fails (a Keycloak/PRB/PCE error before ``score_scenario`` ever runs -- +these two suites isolate a failing scenario's setup per-scenario, so this is common, not +exceptional) never gets those properties recorded at all. Such an entry still gets the crash detail +*and* the same six-field metrics block, values marked ``unavailable`` with why -- identified by +nodeid (``::test_prb_correctness[``/``::test_e2e_correctness[``, see ``_CORRECTNESS_TEST_MARKERS``) +since there are no properties to dispatch on -- rather than silently falling back to the generic +docstring + crash-message rendering every other test in this suite gets. """ from __future__ import annotations @@ -146,12 +154,39 @@ def _format_pairs_dict(pairs_by_gate: dict) -> str: ) +# Nodeid substrings identifying the two correctness suites' single test function each (parametrized +# by scenario name) — used to give a scenario whose *setup* failed (before score_scenario ever ran, +# so none of precision/recall/etc got record_property'd) the same six-field metrics shape every +# other entry gets, instead of silently omitting it. See `_render_entry`'s middle branch. +_CORRECTNESS_TEST_MARKERS = ("::test_prb_correctness[", "::test_e2e_correctness[") + + +def _render_metrics_block(lines: list[str], props: dict, *, unavailable_reason: str | None = None) -> None: + """Render the precision/recall/denial-precision + over-/under-grant/incorrect-denial breakdown + ``test_prb_correctness``/``test_e2e_correctness`` record. When ``unavailable_reason`` is given + (the scenario's own setup failed before scoring could run, so ``props`` has none of this), + render the same six fields with a uniform placeholder instead — so a reader always sees the + same shape, pass or fail, setup-failed or scored.""" + if unavailable_reason is not None: + for label in ("Precision", "Recall", "Denial precision", "Over-grants", "Under-grants", "Incorrectly denied"): + lines.append(f"- **{label}:** unavailable — {unavailable_reason}") + return + lines.append(f"- **Precision:** {props['precision']:.3f}") + lines.append(f"- **Recall:** {props['recall']:.3f}") + lines.append(f"- **Denial precision:** {props['denial_precision']:.3f}") + _render_field(lines, "Over-grants", _format_pairs_dict(props.get("over_grants", {}))) + _render_field(lines, "Under-grants", _format_pairs_dict(props.get("under_grants", {}))) + _render_field(lines, "Incorrectly denied", _format_pairs_dict(props.get("incorrectly_denied", {}))) + + def _render_entry(lines: list[str], nodeid: str, report: pytest.TestReport, category: str) -> None: """Per-cell tests (``test_inbound``/``test_outbound``) ``record_property`` a concrete description + expected/actual boolean + explanation; ``test_prb_correctness`` (correctness-prb) ``record_property``s precision/recall/denial-precision + the over-/under-grant/incorrect-denial pair breakdown; render each instead of the generic docstring + crash/skip-reason fallback every - other test in this suite gets.""" + other test in this suite gets. A correctness-suite scenario whose *setup* failed (a pipeline + error before ``score_scenario`` ever ran) gets the crash detail *and* the same six-field metrics + block, marked unavailable with why — not silently dropped to the generic fallback.""" lines.append(f"### `{nodeid}`") props = dict(report.user_properties) if "expected" in props and "output" in props: @@ -166,12 +201,15 @@ def _render_entry(lines: list[str], nodeid: str, report: pytest.TestReport, cate description = _docstrings.get(nodeid) if description: lines.append(f"- **What it tests:** {description}") - lines.append(f"- **Precision:** {props['precision']:.3f}") - lines.append(f"- **Recall:** {props['recall']:.3f}") - lines.append(f"- **Denial precision:** {props['denial_precision']:.3f}") - _render_field(lines, "Over-grants", _format_pairs_dict(props.get("over_grants", {}))) - _render_field(lines, "Under-grants", _format_pairs_dict(props.get("under_grants", {}))) - _render_field(lines, "Incorrectly denied", _format_pairs_dict(props.get("incorrectly_denied", {}))) + _render_metrics_block(lines, props) + elif any(marker in nodeid for marker in _CORRECTNESS_TEST_MARKERS): + doc = _docstrings.get(nodeid) + if doc: + lines.append(f"- **What it tests:** {doc}") + detail = _detail(report, category) + if detail: + _render_field(lines, "Failure", detail) + _render_metrics_block(lines, props, unavailable_reason="scenario setup failed before scoring could run") else: doc = _docstrings.get(nodeid) if doc: From c912884c0c3959cdb6f629c7139fa0b91363cb77 Mon Sep 17 00:00:00 2001 From: Amitfre15 Date: Tue, 8 Sep 2026 16:33:33 +0300 Subject: [PATCH 4/6] feat: Score correctness suites against best-effort PRB proposals When the PRB's generate/audit loop rejects a scope/role decision (a genuine contradiction, or an exhausted retry budget), the exception used to propagate all the way up and abort the whole scenario -- every other decision's already-approved rules discarded too, and the correctness suites' report showed "setup failed" with no metrics at all for that scenario. orchestrate_prb/_invoke_graph gain an opt-in best_effort parameter: when set, a rejected decision falls back to whatever was last proposed (captured via graph.stream(..., stream_mode="values") since the compiled graphs attach no checkpointer) instead of aborting, built into a real PolicyRule set the same way the PRB's own build node would (eval/best_effort_rules.py, reusing graph.py's _denied_names). Every other decision in the scenario proceeds normally. By explicit user request: a best-effort pair may not represent real production behavior (the auditor rejected it for a reason), so this is scored as an accepted tradeoff, not silently -- best_effort_notes names exactly which decisions this applies to, and eval/conftest.py's report renders it as an explicit caveat whenever non-empty. Both correctness suites opt in; eval_consistency/eval_robustness keep the default (unaffected). The shared pipeline fixture (used by both eval_extended and the e2e suite) hardcodes best_effort=True -- the two suites can't cleanly have different behavior there since they share one Keycloak+PRB provisioning pass per scenario, and the user confirmed accepting that eval_extended's own per-cell tests are affected too rather than duplicate that pass. Also declares pytest-xdist as a real test-extra dependency (was an ad-hoc local install) so `-n 8` works for the fully-independent PRB-direct suites without a separate install step. Assisted-By: Claude (Anthropic AI) Signed-off-by: Amitfre15 --- .../specs/eval/policy-eval-correctness-e2e.md | 75 +++++++++--- .../specs/eval/policy-eval-correctness-prb.md | 47 +++++++- aiac/eval/best_effort_rules.py | 46 +++++++ aiac/eval/conftest.py | 22 ++++ aiac/eval/test_best_effort_rules.py | 80 +++++++++++++ aiac/eval/test_policy_pipeline_consistency.py | 2 +- .../test_policy_pipeline_correctness_e2e.py | 22 +++- .../test_policy_pipeline_correctness_prb.py | 38 ++++-- aiac/eval/test_policy_pipeline_eval.py | 112 ++++++++++++++++-- aiac/eval/test_policy_pipeline_robustness.py | 4 +- aiac/pyproject.toml | 1 + aiac/uv.lock | 24 ++++ 12 files changed, 425 insertions(+), 48 deletions(-) create mode 100644 aiac/eval/best_effort_rules.py create mode 100644 aiac/eval/test_best_effort_rules.py diff --git a/aiac/docs/specs/eval/policy-eval-correctness-e2e.md b/aiac/docs/specs/eval/policy-eval-correctness-e2e.md index b9d773135..6fb47408b 100644 --- a/aiac/docs/specs/eval/policy-eval-correctness-e2e.md +++ b/aiac/docs/specs/eval/policy-eval-correctness-e2e.md @@ -21,6 +21,9 @@ `eval/correctness_scorer.py`/`eval/test_correctness_scorer.py`'s logic-module/test-module split. - `aiac/eval/test_policy_pipeline_correctness_e2e.py` — the suite itself, `@pytest.mark.eval_correctness_e2e`. +- `aiac/eval/best_effort_rules.py` — the best-effort fallback's own pure-logic helper + (`_best_effort_rules`); `aiac/eval/test_best_effort_rules.py` — its unmarked unit tests. See + [Best-effort proposals](#best-effort-proposals). - Reuses, unmodified, from `eval.test_policy_pipeline_eval`: `SCENARIOS`, the `pipeline` fixture (re-exported by import — see [Runbook](#runbook)), `_require_scenario`, `_rego_path`, `opa_bin`, `opa_eval`, `truth`. @@ -110,6 +113,31 @@ This benefits every suite that shares the `pipeline` fixture (`eval_extended` pr suite), not just `eval_correctness_e2e` — see [Relationship to other integration tests](#relationship-to-other-integration-tests). +## Best-effort proposals + +The PRB's generate→audit loop (`aiac.agent.policy_rules_builder.graph`'s `_audit`) can reject a +scope/role decision outright — a genuine contradiction (`PolicyContradictionError`) or an +exhausted retry budget (`PolicyRulesBuilderError`, after `MAX_AUDIT_RETRIES=3`). By default this +aborts `orchestrate_prb()` entirely for that scenario — `compute_and_apply` never runs, so there's +no Rego to score at all, and the scenario's report entry shows "setup failed" with nothing else. + +The shared `pipeline` fixture instead calls `orchestrate_prb(..., best_effort=True)` (see +[Parallelization](#parallelization) — same fixture, applies to both its consumers, confirmed with +the user): a rejected decision falls back to whatever was last proposed before the auditor +rejected it, built the same way the PRB's own `build` node would have +(`eval.best_effort_rules._best_effort_rules`, unit-tested unmarked). That best-effort rule flows +into `compute_and_apply` and gets rendered into real Rego like any other rule — every other +decision in the scenario proceeds normally, and the scenario as a whole now completes and scores. + +**This is an explicit, user-requested tradeoff**: the rendered Rego for a scenario with a +best-effort decision can contain a rule a real deployment never would (the auditor rejected it — +a real pipeline run aborts instead). `record_property("best_effort_notes", ...)` — a +`{scope_or_role_name: reason}` dict — and the printed summary line name exactly which decisions +this applies to; the report renders it as an extra field with an explicit caveat whenever +non-empty. See [`policy-eval-correctness-prb.md` §Best-effort +proposals](policy-eval-correctness-prb.md#best-effort-proposals) for the sibling suite's identical +mechanism (same underlying `orchestrate_prb` parameter). + ## Runbook ```bash @@ -130,24 +158,27 @@ suite — this suite never false-passes when its infra isn't available. ## Expected output Parametrized over all 8 scenario names (`sorted(SCENARIOS)`); expects all 8 to pass (zero -over-grants) given a healthy Keycloak instance and a well-behaved LLM endpoint. In practice, a real -run against the rossoctl kind cluster currently shows 6/8 passing and 2/8 failing at the *setup* -stage (`PolicyContradictionError`/`PolicyRulesBuilderError` from the PRB's own audit/retry loop, -`aiac.agent.policy_rules_builder.graph._audit`) — this is the **pre-existing, already-deferred** -audit/retry-convergence bug (the auditor rejects the generator's proposal identically on all 3 -retries for `agent_delegation` and, in this run, `unreachable_resources`), confirmed to reproduce -identically in the PRB-direct `test_policy_pipeline_correctness_prb.py` suite (no Keycloak/PCE/OPA -involved at all), so it is unrelated to this suite, to the pipeline fixture's parallelization, or -to anything else changed here. Not fixed as part of this ticket — see [Out of -Scope](#out-of-scope). Each test case -`record_property`s `precision`, `recall`, `denial_precision`, `over_grants`, `under_grants`, and -`incorrectly_denied` (the latter three as `{gate: sorted(pairs)}`), and prints: +over-grants) given a healthy Keycloak instance and a well-behaved LLM endpoint. Before +`best_effort=True` was wired in (see [Best-effort proposals](#best-effort-proposals)), a real run +against the rossoctl kind cluster showed 6/8 passing and 2/8 failing at the *setup* stage +(`PolicyContradictionError`/`PolicyRulesBuilderError` from the PRB's own audit/retry loop, +`aiac.agent.policy_rules_builder.graph._audit`) — the **pre-existing, already-deferred** +audit/retry-convergence bug (confirmed to reproduce identically in the PRB-direct +`test_policy_pipeline_correctness_prb.py` suite, no Keycloak/PCE/OPA involved at all — unrelated +to this suite or its parallelization). `best_effort=True` doesn't fix that bug (still not this +ticket's job — see [Out of Scope](#out-of-scope)), it changes what a rejected scenario reports: +instead of "setup failed" with nothing else, it now scores (real, possibly imperfect +precision/recall) with `best_effort_notes` naming exactly which decisions weren't auditor-approved. +Each test case `record_property`s `precision`, `recall`, `denial_precision`, `over_grants`, +`under_grants`, `incorrectly_denied` (the latter three as `{gate: sorted(pairs)}`), and +`best_effort_notes`, and prints: ```text [correctness-e2e] wildcard_grant: precision=1.000 recall=1.000 denial_precision=1.000 over_grants={} under_grants={} incorrectly_denied={} + best_effort_notes={} ``` A failing case's assertion message names the scenario and the exact over-granted `(role, scope)` @@ -175,7 +206,14 @@ This is **one** integration-test spec among several indexed by the master PRD Rego-rendering bug. - **Shares the `pipeline` fixture with `eval_extended`** (`test_policy_pipeline_eval.py`) — the parallelization work described above (see [Parallelization](#parallelization)) speeds up both - suites, since it lives in the shared fixture, not in this suite's own file. + suites, since it lives in the shared fixture, not in this suite's own file. The + [best-effort fallback](#best-effort-proposals) is the same story: it's a property of the shared + fixture, so `eval_extended`'s own per-cell tests (`test_inbound`/`test_outbound`/ + `test_grant_set_matches_truth_table`) are affected too, not just `test_e2e_correctness` — a + scenario whose PRB rejects a decision no longer gets a clean scenario-level skip; it runs + through with a best-effort fallback for *that* decision only, so those specific per-cell + assertions can show a real (and possibly wrong) result instead of a skip. Confirmed as the + accepted tradeoff with the user, not treated as a regression to fix. - **New marker, registered in `pyproject.toml`** (`eval_correctness_e2e`), distinct from `eval_extended`/`eval_consistency`/`eval_robustness`/`eval_correctness_prb`. @@ -184,10 +222,13 @@ This is **one** integration-test spec among several indexed by the master PRD - **Fixing the PRB audit/retry-convergence bug** (`aiac.agent.policy_rules_builder.graph._audit`, around line 200-220) that causes `agent_delegation`/`unreachable_resources` (or, on other runs, `confusable_agents` — which scenario(s) hit it varies with LLM sampling, but at least one of - `agent_delegation`/`confusable_agents` reproduces consistently) to fail at setup with - `PolicyContradictionError`/`PolicyRulesBuilderError`. Confirmed pre-existing and unrelated to - this suite (reproduces identically in the PRB-direct `eval_correctness_prb` suite). The user has - already deferred fixing `graph.py` itself as a separate follow-up — not this ticket's job. + `agent_delegation`/`confusable_agents` reproduces consistently) to reject a scope/role decision + with `PolicyContradictionError`/`PolicyRulesBuilderError`. Confirmed pre-existing and unrelated + to this suite (reproduces identically in the PRB-direct `eval_correctness_prb` suite). + `best_effort=True` (see [Best-effort proposals](#best-effort-proposals)) changes how a rejection + is *reported* (scored with a caveat instead of "setup failed") — it does not fix the underlying + bug. The user has already deferred fixing `graph.py` itself as a separate follow-up — not this + ticket's job. - **Fixing the `outbound_target` denial-rendering gap** in `aiac.pdp.service.policy.opa.rego.generate_outbound_rego` — see [Known gap](#known-gap-outbound_target-denial-is-unrenderable-not-just-untested). This is diff --git a/aiac/docs/specs/eval/policy-eval-correctness-prb.md b/aiac/docs/specs/eval/policy-eval-correctness-prb.md index f1a1dcd70..73b31e42f 100644 --- a/aiac/docs/specs/eval/policy-eval-correctness-prb.md +++ b/aiac/docs/specs/eval/policy-eval-correctness-prb.md @@ -24,6 +24,10 @@ synthetic-`Role`/`Scope` builder `policy-eval-robustness-consistency.md`'s two suites use) and imports `SCENARIOS`, `orchestrate_prb`, `grant_sets`, `truth` from `eval.test_policy_pipeline_eval` unmodified. +- `aiac/eval/best_effort_rules.py` — the best-effort fallback's own pure-logic helper + (`_best_effort_rules`, called from `orchestrate_prb`/`_invoke_graph`); `aiac/eval/ + test_best_effort_rules.py` — its unmarked unit tests. See [Best-effort + proposals](#best-effort-proposals). ## Description @@ -103,12 +107,46 @@ more precisely diagnosed under-grant, not a new failure class). `denial_precisio scenario and per gate via `record_property`, never blended into grant precision/recall, and never gates the test. +## Best-effort proposals + +The PRB's generate→audit loop (`aiac.agent.policy_rules_builder.graph`'s `_audit`) can reject a +scope/role decision outright — a genuine contradiction (`PolicyContradictionError`, fails closed +immediately) or an exhausted retry budget (`PolicyRulesBuilderError`, after `MAX_AUDIT_RETRIES=3`). +By default this aborts `orchestrate_prb()` entirely, discarding every other decision's +already-approved rules along with it — one bad scope used to mean the whole scenario showed +"setup failed" with no precision/recall at all. + +This suite calls `orchestrate_prb(..., best_effort=True)`: a rejected decision instead falls back +to whatever was last proposed (before the auditor rejected it) — a real, if never-approved, guess +at the grant/deny set for that one scope/role, built the same way the PRB's own `build` node would +have (`aiac.agent.policy_rules_builder.graph._denied_names` reused; see +`eval.best_effort_rules._best_effort_rules`, unit-tested unmarked in +`eval/test_best_effort_rules.py`). Every other decision in the scenario proceeds normally. + +**This is an explicit, user-requested tradeoff, not free lunch**: a best-effort pair may not +represent what a real deployment would ever contain — the auditor rejected it for a reason, and a +real pipeline run would never emit it. `record_property("best_effort_notes", ...)` — a +`{scope_or_role_name: reason}` dict — and the printed summary line name exactly which decisions +this applies to, so a reader can tell which numbers are "real" and which are best-effort. The +report (`eval/conftest.py`) renders this as an extra field with an explicit caveat whenever +non-empty. `eval_consistency`/`eval_robustness` keep `best_effort=False` (the default, at their +own direct `orchestrate_prb` call sites) — a rejected decision still aborts their scenario/repeat +as before. `eval_extended` is the one exception: it shares the same session-scoped `pipeline` +fixture `test_e2e_correctness` uses, and that fixture's `orchestrate_prb` call hardcodes +`best_effort=True` unconditionally (confirmed with the user — see +[policy-eval-correctness-e2e.md § Best-effort +proposals](policy-eval-correctness-e2e.md#best-effort-proposals) and +`eval.test_policy_pipeline_eval`'s module docstring for why this couldn't cleanly be made +e2e-only), so `eval_extended`'s own per-cell tests are affected too, not just this suite or +`test_e2e_correctness`. + ## Expected output Parametrized over all 8 scenario names (`sorted(SCENARIOS)`); expects **all 8 to pass** (zero over-grants) given a well-behaved LLM endpoint. Each test case `record_property`s `precision`, -`recall`, `denial_precision`, `over_grants`, `under_grants`, and `incorrectly_denied` (each of the -latter three as `{gate: sorted(pairs)}`), and prints a one-line summary: +`recall`, `denial_precision`, `over_grants`, `under_grants`, `incorrectly_denied` (each of the +latter three as `{gate: sorted(pairs)}`), and `best_effort_notes` (see [Best-effort +proposals](#best-effort-proposals)), and prints a one-line summary: ```text [correctness] wildcard_grant: precision=1.000 recall=1.000 denial_precision=1.000 @@ -255,6 +293,11 @@ This is **one** integration-test spec among several indexed by the master PRD tracked/reported via `record_property` and the printed summary line, never gated. - **New scenarios.** The taxonomy cross-check above confirms the existing 8-scenario corpus already covers every taxonomy theme; none is needed. +- **Fixing the PRB audit/retry-convergence bug** that causes the auditor to reject a scope/role + decision (`PolicyContradictionError`/`PolicyRulesBuilderError`) for a variable subset of + scenarios depending on LLM sampling — see [Best-effort proposals](#best-effort-proposals), which + changes how a rejection is *reported*, not the underlying bug. Already deferred by the user as a + separate follow-up to `aiac.agent.policy_rules_builder.graph._audit` — not this ticket's job. ## Blocked-by diff --git a/aiac/eval/best_effort_rules.py b/aiac/eval/best_effort_rules.py new file mode 100644 index 000000000..c5e6b6ae1 --- /dev/null +++ b/aiac/eval/best_effort_rules.py @@ -0,0 +1,46 @@ +"""Pure-logic helper for ``test_policy_pipeline_eval.py``'s ``_invoke_graph`` — no LLM, no +Keycloak, no ``opa``, no I/O. Mirrors ``correctness_e2e_helpers.py``'s split from its own test +module: logic lives here (no ``test_`` prefix, not collected by pytest), unit tests live in +``test_best_effort_rules.py`` (unmarked, runs in the default fast pass). +""" + +from __future__ import annotations + +from typing import Any + +from aiac.agent.policy_rules_builder.graph import _denied_names +from aiac.policy.model.models import PolicyRule, RuleEffect + + +def _best_effort_rules(entity: dict[str, Any], state: dict[str, Any]) -> list[PolicyRule]: + """Replicate ``graph.py``'s ``build`` node logic (role-focal or scope-focal, whichever + ``entity``'s shape indicates) against a proposal the auditor never approved — same + ``_denied_names()``-driven exclusivity-complement + explicit-prohibition set, same + ALLOW-then-DENY ``PolicyRule`` shape (``graph.py``'s ``build_role_graph``/``build_scope_graph`` + closures, not importable — they're nested — hence replicated here rather than reused). + + Only ever called from ``eval.test_policy_pipeline_eval._invoke_graph`` after catching a + rejection; the caller is responsible for flagging the result as best-effort (not a real, + auditor-approved decision) — see ``orchestrate_prb``'s ``best_effort_notes``. + """ + selected = set(state.get("selected_names", [])) + denied_explicit = state.get("denied_names", []) + exclusive = bool(state.get("exclusive", False)) + if "role" in entity: # ROLE_GRAPH shape: role-focal + role = entity["role"] + candidate_scopes = entity["scopes"] + denied = _denied_names(denied_explicit, exclusive, [sc.name for sc in candidate_scopes], selected) + allows = [ + PolicyRule(role=role, scope=sc, effect=RuleEffect.ALLOW) for sc in candidate_scopes if sc.name in selected + ] + denies = [ + PolicyRule(role=role, scope=sc, effect=RuleEffect.DENY) for sc in candidate_scopes if sc.name in denied + ] + return allows + denies + # SCOPE_GRAPH shape: scope-focal + scope = entity["scope"] + candidate_roles = entity["roles"] + denied = _denied_names(denied_explicit, exclusive, [r.name for r in candidate_roles], selected) + allows = [PolicyRule(role=r, scope=scope, effect=RuleEffect.ALLOW) for r in candidate_roles if r.name in selected] + denies = [PolicyRule(role=r, scope=scope, effect=RuleEffect.DENY) for r in candidate_roles if r.name in denied] + return allows + denies diff --git a/aiac/eval/conftest.py b/aiac/eval/conftest.py index ead7d4ba7..cbc82690a 100644 --- a/aiac/eval/conftest.py +++ b/aiac/eval/conftest.py @@ -45,6 +45,13 @@ nodeid (``::test_prb_correctness[``/``::test_e2e_correctness[``, see ``_CORRECTNESS_TEST_MARKERS``) since there are no properties to dispatch on -- rather than silently falling back to the generic docstring + crash-message rendering every other test in this suite gets. + +Both suites also ``record_property("best_effort_notes", ...)`` -- a ``{scope_or_role_name: +reason}`` dict naming every decision that fell back to a never-approved PRB proposal instead of +aborting the scenario (``eval.test_policy_pipeline_eval.orchestrate_prb``'s ``best_effort=True``, +by explicit user request so a scenario the auditor partly rejects still scores). When non-empty, +``_render_metrics_block`` appends one more field listing them, with an explicit caveat that those +pairs don't represent real production behavior. """ from __future__ import annotations @@ -161,6 +168,14 @@ def _format_pairs_dict(pairs_by_gate: dict) -> str: _CORRECTNESS_TEST_MARKERS = ("::test_prb_correctness[", "::test_e2e_correctness[") +def _format_best_effort_notes(notes: dict[str, str]) -> str: + """Render a ``{scope_or_role_name: reason}`` dict (``orchestrate_prb``'s ``best_effort_notes``) + as one line per entry, or ``"none"``.""" + if not notes: + return "none" + return "\n".join(f"{name}: {reason}" for name, reason in sorted(notes.items())) + + def _render_metrics_block(lines: list[str], props: dict, *, unavailable_reason: str | None = None) -> None: """Render the precision/recall/denial-precision + over-/under-grant/incorrect-denial breakdown ``test_prb_correctness``/``test_e2e_correctness`` record. When ``unavailable_reason`` is given @@ -177,6 +192,13 @@ def _render_metrics_block(lines: list[str], props: dict, *, unavailable_reason: _render_field(lines, "Over-grants", _format_pairs_dict(props.get("over_grants", {}))) _render_field(lines, "Under-grants", _format_pairs_dict(props.get("under_grants", {}))) _render_field(lines, "Incorrectly denied", _format_pairs_dict(props.get("incorrectly_denied", {}))) + best_effort_notes = props.get("best_effort_notes", {}) + if best_effort_notes: + _render_field( + lines, + "Best-effort proposals used (not real production behavior — the auditor never approved these)", + _format_best_effort_notes(best_effort_notes), + ) def _render_entry(lines: list[str], nodeid: str, report: pytest.TestReport, category: str) -> None: diff --git a/aiac/eval/test_best_effort_rules.py b/aiac/eval/test_best_effort_rules.py new file mode 100644 index 000000000..38aacbf2f --- /dev/null +++ b/aiac/eval/test_best_effort_rules.py @@ -0,0 +1,80 @@ +"""Unit tests for ``best_effort_rules.py`` (spec: ``docs/specs/eval/policy-eval-correctness-e2e.md``). + +Pure-logic, unmarked — runs in the default fast pass (``testpaths`` already includes ``eval/``). +No LLM, no Keycloak, no ``opa`` — just real ``Role``/``Scope``/``PolicyRule`` model construction +(cheap, no I/O), mirroring ``eval.prb_direct``'s synthetic-object style. Needs ``src/`` on +``sys.path`` itself (unlike ``test_correctness_e2e_helpers.py``'s sibling, whose logic module has +no ``aiac.*`` imports at all) since ``best_effort_rules.py`` imports real production types. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +HERE = Path(__file__).resolve().parent # aiac/eval/ +SRC = HERE.parent / "src" +sys.path.insert(0, str(SRC)) # so ``import aiac.*`` resolves + +from aiac.idp.configuration.models import Role, Scope # noqa: E402 +from aiac.policy.model.models import RuleEffect # noqa: E402 +from eval.best_effort_rules import _best_effort_rules # noqa: E402 + + +def _role(name: str) -> Role: + return Role(id=f"role-{name}", name=name, composite=False) + + +def _scope(name: str) -> Scope: + return Scope(id=f"scope-{name}", name=name) + + +def test_role_focal_allows_selected_and_denies_explicit() -> None: + role = _role("agent-role-inventory-operations") + scopes = [_scope("tool-scope-inventory-read"), _scope("tool-scope-inventory-write")] + state = { + "selected_names": ["tool-scope-inventory-read"], + "denied_names": ["tool-scope-inventory-write"], + "exclusive": False, + } + rules = _best_effort_rules({"role": role, "scopes": scopes}, state) + + assert len(rules) == 2 + allow = next(r for r in rules if r.effect == RuleEffect.ALLOW) + deny = next(r for r in rules if r.effect == RuleEffect.DENY) + assert allow.role is role and allow.scope.name == "tool-scope-inventory-read" + assert deny.role is role and deny.scope.name == "tool-scope-inventory-write" + + +def test_role_focal_exclusive_denies_the_unselected_complement() -> None: + role = _role("agent-role-inventory-operations") + scopes = [_scope("tool-scope-a"), _scope("tool-scope-b"), _scope("tool-scope-c")] + state = {"selected_names": ["tool-scope-a"], "denied_names": [], "exclusive": True} + rules = _best_effort_rules({"role": role, "scopes": scopes}, state) + + denied_scope_names = {r.scope.name for r in rules if r.effect == RuleEffect.DENY} + assert denied_scope_names == {"tool-scope-b", "tool-scope-c"} + + +def test_scope_focal_allows_selected_and_denies_explicit() -> None: + scope = _scope("agent-scope-tracker-access") + roles = [_role("user-role-developer"), _role("user-role-tester")] + state = { + "selected_names": ["user-role-tester"], + "denied_names": ["user-role-developer"], + "exclusive": False, + } + rules = _best_effort_rules({"scope": scope, "roles": roles}, state) + + assert len(rules) == 2 + allow = next(r for r in rules if r.effect == RuleEffect.ALLOW) + deny = next(r for r in rules if r.effect == RuleEffect.DENY) + assert allow.scope is scope and allow.role.name == "user-role-tester" + assert deny.scope is scope and deny.role.name == "user-role-developer" + + +def test_empty_proposal_yields_no_rules() -> None: + role = _role("agent-role-x") + scopes = [_scope("tool-scope-a")] + state = {"selected_names": [], "denied_names": [], "exclusive": False} + assert _best_effort_rules({"role": role, "scopes": scopes}, state) == [] diff --git a/aiac/eval/test_policy_pipeline_consistency.py b/aiac/eval/test_policy_pipeline_consistency.py index 034a17cf9..0fd3efad0 100644 --- a/aiac/eval/test_policy_pipeline_consistency.py +++ b/aiac/eval/test_policy_pipeline_consistency.py @@ -62,7 +62,7 @@ def test_prb_consistent_across_repeats(scenario_name: str, monkeypatch: pytest.M runs = [] for _ in range(N): - rules, _, _ = orchestrate_prb(roles, scopes, scenario) + rules, _, _, _ = orchestrate_prb(roles, scopes, scenario) runs.append(grant_sets(scenario, rules)) baseline = runs[0] diff --git a/aiac/eval/test_policy_pipeline_correctness_e2e.py b/aiac/eval/test_policy_pipeline_correctness_e2e.py index 5fe809676..851f2ecaf 100644 --- a/aiac/eval/test_policy_pipeline_correctness_e2e.py +++ b/aiac/eval/test_policy_pipeline_correctness_e2e.py @@ -18,6 +18,17 @@ Under-grants and incorrectly-denied pairs are reported via ``record_property`` and a printed summary line, never gating — same philosophy as the PRB-level suite. +The shared ``pipeline`` fixture provisions each scenario with ``orchestrate_prb(..., +best_effort=True)`` — a scope/role decision the PRB's auditor rejects contributes a best-effort, +never-approved fallback rule instead of aborting the whole scenario, by explicit user request, so +every scenario's real Rego/OPA output completes and scores instead of the whole thing showing +"setup failed" with nothing to show. This means a scored pair may not represent what a real +deployment would ever contain; ``best_effort_notes`` (``record_property``'d, printed) names +exactly which scope/role decisions this applies to. See +``eval.test_policy_pipeline_eval.orchestrate_prb``/``_invoke_graph``'s docstrings for the +mechanism, and its module docstring for why this applies uniformly to the shared fixture's other +consumer (``eval_extended``) too. + Known gap: the ``outbound_target`` gate's denial side (``AgentPolicyModel.outbound_target_deny_rules``) is computed by the PCE but never rendered into the outbound Rego by ``aiac.pdp.service.policy.opa.rego.generate_outbound_rego`` — only the ALLOW side @@ -149,6 +160,13 @@ def test_e2e_correctness(pipeline: dict[str, dict], scenario_name: str, record_p over_grants = {g: sorted(p) for g, p in score.over_grants.items()} under_grants = {g: sorted(p) for g, p in score.under_grants.items()} incorrectly_denied = {g: sorted(p) for g, p in score.incorrectly_denied.items()} + # A scope/role decision the PRB's auditor rejected during this scenario's pipeline setup (see + # eval.test_policy_pipeline_eval.orchestrate_prb's best_effort=True) contributed a best-effort, + # never-approved fallback rule instead of aborting the whole scenario — flowed all the way + # through compute_and_apply/OPA like any other rule, by explicit user request, so this + # scenario scores instead of showing "setup failed" with nothing to show. Named here so a + # reader knows which of this scenario's numbers don't represent real production behavior. + best_effort_notes = scenario_result.get("best_effort_notes", {}) record_property("precision", score.precision) record_property("recall", score.recall) @@ -156,12 +174,14 @@ def test_e2e_correctness(pipeline: dict[str, dict], scenario_name: str, record_p record_property("over_grants", over_grants) record_property("under_grants", under_grants) record_property("incorrectly_denied", incorrectly_denied) + record_property("best_effort_notes", best_effort_notes) print( f"[correctness-e2e] {scenario_name}: precision={score.precision:.3f} " f"recall={score.recall:.3f} denial_precision={score.denial_precision:.3f}\n" f" over_grants={over_grants or '{}'}\n" f" under_grants={under_grants or '{}'}\n" - f" incorrectly_denied={incorrectly_denied or '{}'}" + f" incorrectly_denied={incorrectly_denied or '{}'}\n" + f" best_effort_notes={best_effort_notes or '{}'}" ) assert score.passed, ( diff --git a/aiac/eval/test_policy_pipeline_correctness_prb.py b/aiac/eval/test_policy_pipeline_correctness_prb.py index 2317a8690..ce19aecab 100644 --- a/aiac/eval/test_policy_pipeline_correctness_prb.py +++ b/aiac/eval/test_policy_pipeline_correctness_prb.py @@ -21,16 +21,22 @@ Under-grants and incorrectly-denied pairs are reported via ``record_property`` and a printed summary line, never gating (spec: under-grant threshold TBD, deferred). -Run (needs LLM_BASE_URL/LLM_MODEL/LLM_API_KEY exported; no Keycloak/opa needed): - .venv/bin/pytest eval/test_policy_pipeline_correctness_prb.py \ - -m eval_correctness_prb -v -s - -The 8 scenarios are fully independent (separate synthetic Role/Scope, separate -AIAC_POLICY_FILE), so they can run concurrently for a near-linear wall-clock speedup — -``orchestrate_prb()`` makes ~5-8 sequential LLM calls per scenario, so the suite is otherwise -dominated by LLM round-trip latency. Requires ``pip install pytest-xdist`` first (not a repo -dependency, opt-in for local speed): - .venv/bin/pip install pytest-xdist +Calls ``orchestrate_prb(..., best_effort=True)``: a scope/role decision the PRB's auditor rejects +(``PolicyContradictionError``/exhausted-retry ``PolicyRulesBuilderError``) contributes a +best-effort, never-approved fallback rule instead of aborting the whole scenario — by explicit +user request, so every scenario scores instead of one rejected decision hiding the rest. This +means a scored pair may not represent what a real deployment would ever contain; +``best_effort_notes`` (``record_property``'d, printed) names exactly which scope/role decisions +this applies to. See ``eval.test_policy_pipeline_eval.orchestrate_prb``'s docstring for the +mechanism. + +Run (needs LLM_BASE_URL/LLM_MODEL/LLM_API_KEY exported; no Keycloak/opa needed). The 8 scenarios +are fully independent (separate synthetic Role/Scope, separate AIAC_POLICY_FILE), so always run +with ``-n 8`` for a near-linear wall-clock speedup — ``orchestrate_prb()`` makes ~5-8 sequential +LLM calls per scenario, so a plain sequential run is dominated by LLM round-trip latency (a real +run took ~36 minutes; with ``-n 8`` it's close to 1/8th that). ``pytest-xdist`` is a declared +`test`-extra dependency (``aiac/pyproject.toml``'s ``[project.optional-dependencies].test``) +picked up by a normal ``uv sync``/``pip install -e ".[test]"`` — no separate install step needed: .venv/bin/pytest eval/test_policy_pipeline_correctness_prb.py \ -m eval_correctness_prb -n 8 -v -s """ @@ -73,7 +79,13 @@ def test_prb_correctness(scenario_name: str, monkeypatch: pytest.MonkeyPatch, re policy_path = Path(scenario.__file__).resolve().parent / scenario.POLICY_FILE monkeypatch.setenv("AIAC_POLICY_FILE", str(policy_path)) - rules, _, _ = orchestrate_prb(roles, scopes, scenario) + # best_effort=True: a scope/role decision the auditor rejects contributes a best-effort + # (never-approved) fallback rule instead of aborting the whole scenario — see + # orchestrate_prb's docstring. Deliberate, by explicit user request, so every scenario scores + # instead of the whole thing showing "setup failed"; best_effort_notes names exactly which + # scope/role decisions this applies to, reported below so a reader knows which of this + # scenario's numbers don't represent real production behavior. + rules, _, _, best_effort_notes = orchestrate_prb(roles, scopes, scenario, best_effort=True) granted = grant_sets(scenario, [r for r in rules if r.effect == RuleEffect.ALLOW]) denied = grant_sets(scenario, [r for r in rules if r.effect == RuleEffect.DENY]) expected = truth(scenario) @@ -89,12 +101,14 @@ def test_prb_correctness(scenario_name: str, monkeypatch: pytest.MonkeyPatch, re record_property("over_grants", over_grants) record_property("under_grants", under_grants) record_property("incorrectly_denied", incorrectly_denied) + record_property("best_effort_notes", best_effort_notes) print( f"[correctness] {scenario_name}: precision={score.precision:.3f} " f"recall={score.recall:.3f} denial_precision={score.denial_precision:.3f}\n" f" over_grants={over_grants or '{}'}\n" f" under_grants={under_grants or '{}'}\n" - f" incorrectly_denied={incorrectly_denied or '{}'}" + f" incorrectly_denied={incorrectly_denied or '{}'}\n" + f" best_effort_notes={best_effort_notes or '{}'}" ) assert score.passed, f"PRB over-granted for scenario '{scenario_name}' — zero-tolerance gate: {over_grants}" diff --git a/aiac/eval/test_policy_pipeline_eval.py b/aiac/eval/test_policy_pipeline_eval.py index ddbeddb45..e61353332 100644 --- a/aiac/eval/test_policy_pipeline_eval.py +++ b/aiac/eval/test_policy_pipeline_eval.py @@ -65,6 +65,24 @@ ``orchestrate_prb`` calls and the idp/store/opa subprocesses — still runs fully concurrently, since realm provisioning is a small fraction of one scenario's wall-clock next to the PRB's several sequential LLM calls. + +``_provision_scenario`` calls ``orchestrate_prb(..., best_effort=True)`` — a scope/role decision +the PRB's auditor rejects contributes a best-effort (never-approved) fallback rule instead of +aborting the whole scenario (see ``orchestrate_prb``/``_invoke_graph``'s docstrings), by explicit +user request so every scenario's real Rego/OPA output completes and scores instead of showing +"setup failed" with nothing to show. This is one shared, session-scoped fixture serving both this +file's own tests and ``test_e2e_correctness`` (``eval/test_policy_pipeline_correctness_e2e.py``), +so it applies uniformly to both — there is no way to make it e2e-only without either duplicating +the whole Keycloak+LLM provisioning pass (rejected: expensive, and the two suites would then score +against two *different* PRB runs of the same scenario) or parametrizing the fixture (rejected: +defeats the sharing this fixture exists for). Confirmed with the user (2026-09-08): this file's +own per-cell tests (``test_inbound``/``test_outbound``/``test_grant_set_matches_truth_table``) are +affected too, not just ``test_e2e_correctness`` — a scenario whose PRB hits a rejection no longer +gets a clean scenario-level skip via ``_require_scenario``'s ``pytest.fail``; it now runs through +with a best-effort fallback for the rejected decision only, so *those specific* per-cell +assertions can show a real (and possibly wrong) result instead. This is the accepted tradeoff, not +a bug — a real run after wiring this in showed 17 such failures, all traceable to the same +scope/role names ``test_e2e_correctness``'s ``best_effort_notes`` names for the same scenarios. """ from __future__ import annotations @@ -123,11 +141,17 @@ from keycloak import KeycloakAdmin # noqa: E402 from keycloak.exceptions import KeycloakError # noqa: E402 -from aiac.agent.policy_rules_builder.graph import ROLE_GRAPH, SCOPE_GRAPH # noqa: E402 +from aiac.agent.policy_rules_builder.graph import ( # noqa: E402 + ROLE_GRAPH, + SCOPE_GRAPH, + PolicyContradictionError, + PolicyRulesBuilderError, +) from aiac.idp.configuration.api import Configuration # noqa: E402 from aiac.idp.configuration.models import Role, Scope # noqa: E402 from aiac.policy.computation.engine import compute_and_apply # noqa: E402 from aiac.policy.model.models import PolicyRule # noqa: E402 +from eval.best_effort_rules import _best_effort_rules # noqa: E402 log = logging.getLogger(__name__) @@ -266,12 +290,35 @@ def _read_back(config: Configuration) -> tuple[dict[str, Role], dict[str, Scope] return roles, scopes -def _invoke_graph(graph: Any, **entity: object) -> tuple[list[PolicyRule], str]: +def _invoke_graph( + graph: Any, *, best_effort: bool = False, **entity: object +) -> tuple[list[PolicyRule], str, str | None]: """Same state shape ``build_scope_rules``/``build_role_rules`` build internally, invoked directly so the final state's ``reasoning`` string (discarded by the wrapper) comes back too. ``entity`` carries the one field that differs between the two graphs: ``roles``+``scope`` for ``SCOPE_GRAPH``, ``role``+``scopes`` for ``ROLE_GRAPH``. + + Returns ``(rules, reasoning, best_effort_note)`` — the third element is ``None`` for a normal, + auditor-approved decision. + + ``best_effort=False`` (default): unchanged from before this parameter existed — a plain + ``graph.invoke(state)``, still letting ``PolicyContradictionError``/``PolicyRulesBuilderError`` + propagate on a rejection. Every caller that doesn't opt in (``eval_extended``'s own tests via + the shared ``pipeline`` fixture, ``eval_consistency``, ``eval_robustness``) keeps today's exact + behavior — one rejected decision still aborts the whole scenario for them. + + ``best_effort=True`` (the two correctness suites only): drives the graph via + ``graph.stream(state, stream_mode="values")`` instead of ``.invoke()`` so that if ``audit`` + raises, the last state snapshot from immediately before the raise (i.e. right after + ``precheck`` — the node just before ``audit`` in ``fetch -> propose -> precheck -> audit -> + build``) is still available, even though ``ROLE_GRAPH``/``SCOPE_GRAPH`` attach no + checkpointer. On catching, falls back to ``_best_effort_rules`` built from that last-proposed + (never-approved) state, and returns a short string describing why — the correctness suites + record this per scope/role so their report can flag it: this fallback path scores something + that would never actually reach a real deployment (the auditor rejected it), by explicit user + request, to get full precision/recall coverage even for a scenario an ordinary run would + abort entirely. """ state = { **entity, @@ -286,13 +333,23 @@ def _invoke_graph(graph: Any, **entity: object) -> tuple[list[PolicyRule], str]: "retry_count": 0, "rules": [], } - out = graph.invoke(state) - return out["rules"], out["reasoning"] + if not best_effort: + out = graph.invoke(state) + return out["rules"], out["reasoning"], None + + last_state = state + try: + for chunk in graph.stream(state, stream_mode="values"): + last_state = chunk + except (PolicyContradictionError, PolicyRulesBuilderError) as exc: + rules = _best_effort_rules(entity, last_state) + return rules, last_state.get("reasoning", ""), f"{type(exc).__name__}: {exc}" + return last_state["rules"], last_state["reasoning"], None def orchestrate_prb( - roles: dict[str, Role], scopes: dict[str, Scope], scenario: ModuleType -) -> tuple[list[PolicyRule], dict[str, str], dict[str, str]]: + roles: dict[str, Role], scopes: dict[str, Scope], scenario: ModuleType, *, best_effort: bool = False +) -> tuple[list[PolicyRule], dict[str, str], dict[str, str], dict[str, str]]: """Run the three PRB mappings against the real LLM and concatenate the rules, generalized over every agent's inbound/target scopes and every tool's scopes. @@ -309,6 +366,11 @@ def orchestrate_prb( instead of ``build_role_rules``/``build_scope_rules`` purely to get that reasoning back — those wrapper functions discard it, and are shared production code used elsewhere, so they are not modified. + + ``best_effort`` (default ``False``) is threaded into every ``_invoke_graph`` call — see its + docstring. The 4th return value, ``best_effort_notes``, maps a scope/role name to a short + reason string for every decision that fell back to an unapproved proposal; empty when + ``best_effort=False`` (the default) or when every decision was cleanly approved. """ user_roles = [roles[name] for name in scenario.USER_ROLES] @@ -324,19 +386,32 @@ def orchestrate_prb( rules: list[PolicyRule] = [] reasoning_by_scope: dict[str, str] = {} reasoning_by_agent_role: dict[str, str] = {} + best_effort_notes: dict[str, str] = {} for agent_scope in inbound_scopes: # (a) user role -> agent inbound scope - scope_rules, reasoning = _invoke_graph(SCOPE_GRAPH, roles=user_roles, scope=agent_scope) + scope_rules, reasoning, note = _invoke_graph( + SCOPE_GRAPH, roles=user_roles, scope=agent_scope, best_effort=best_effort + ) rules += scope_rules reasoning_by_scope[agent_scope.name] = reasoning + if note is not None: + best_effort_notes[agent_scope.name] = note for target_scope in target_scopes: # (b) user role -> tool/agent-target scope - scope_rules, reasoning = _invoke_graph(SCOPE_GRAPH, roles=user_roles, scope=target_scope) + scope_rules, reasoning, note = _invoke_graph( + SCOPE_GRAPH, roles=user_roles, scope=target_scope, best_effort=best_effort + ) rules += scope_rules reasoning_by_scope[target_scope.name] = reasoning + if note is not None: + best_effort_notes[target_scope.name] = note for agent_role in agent_roles: # (c) agent role -> all tool/agent-target scopes - role_rules, reasoning = _invoke_graph(ROLE_GRAPH, role=agent_role, scopes=target_scopes) + role_rules, reasoning, note = _invoke_graph( + ROLE_GRAPH, role=agent_role, scopes=target_scopes, best_effort=best_effort + ) rules += role_rules reasoning_by_agent_role[agent_role.name] = reasoning - return rules, reasoning_by_scope, reasoning_by_agent_role + if note is not None: + best_effort_notes[agent_role.name] = note + return rules, reasoning_by_scope, reasoning_by_agent_role, best_effort_notes # ====================================================================================== @@ -554,8 +629,10 @@ def _provision_scenario(name: str, idp_port: int, store_port: int, opa_port: int """Provision one scenario's realm and run the real PRB+PCE pipeline, leaving ``.rego`` on disk under ``rego_out/policy_pipeline_eval//``. Returns ``{"rego_dir": Path, "rules": list[PolicyRule], "reasoning_by_scope": dict[str, str], "reasoning_by_agent_role": dict[str, - str]}`` (the two reasoning dicts feed the eval report's per-cell "Output" field, see - ``conftest.py``), or ``{"error": }`` on failure. + str], "best_effort_notes": dict[str, str]}`` (the two reasoning dicts feed the eval report's + per-cell "Output" field, see ``conftest.py``; ``best_effort_notes`` names every scope/role + decision — if any — that fell back to a never-approved proposal rather than aborting the + scenario, see ``orchestrate_prb``), or ``{"error": }`` on failure. Fully self-contained — own ``KeycloakAdmin`` connection, own env-var writes, own idp/store/opa ports — so it can run as an independent ``ProcessPoolExecutor`` worker (see the @@ -628,7 +705,15 @@ def _provision_scenario(name: str, idp_port: int, store_port: int, opa_port: int config = Configuration.for_realm(scenario.REALM_DEFAULT) provision_via_config(config, scenario) # exactly once — not idempotent roles, scopes = _read_back(config) - rules, reasoning_by_scope, reasoning_by_agent_role = orchestrate_prb(roles, scopes, scenario) + # best_effort=True: a scope/role decision the auditor rejects contributes a best-effort + # (never-approved) fallback rule instead of aborting the whole scenario — see + # orchestrate_prb's docstring. This means the Rego rendered below can include a rule a + # real deployment never would; deliberate, by explicit user request, so every scenario's + # pipeline completes and scores instead of showing "setup failed" with no metrics at + # all. `best_effort_notes` names exactly which scope/role decisions this applies to. + rules, reasoning_by_scope, reasoning_by_agent_role, best_effort_notes = orchestrate_prb( + roles, scopes, scenario, best_effort=True + ) compute_and_apply(rules, override=False) # Assert every agent's rego actually landed here at setup — EXCEPT agents the scenario @@ -653,6 +738,7 @@ def _provision_scenario(name: str, idp_port: int, store_port: int, opa_port: int "rules": rules, "reasoning_by_scope": reasoning_by_scope, "reasoning_by_agent_role": reasoning_by_agent_role, + "best_effort_notes": best_effort_notes, } except Exception as exc: # noqa: BLE001 - isolate one scenario's setup failure from the rest log.exception("scenario %s: setup failed, isolating from the rest of the session", name) diff --git a/aiac/eval/test_policy_pipeline_robustness.py b/aiac/eval/test_policy_pipeline_robustness.py index d07065970..b259f967f 100644 --- a/aiac/eval/test_policy_pipeline_robustness.py +++ b/aiac/eval/test_policy_pipeline_robustness.py @@ -152,7 +152,7 @@ def test_prb_robust_to_perturbation( mech_policy_path = tmp_path / f"{scenario_name}.mechanical.md" mech_policy_path.write_text(_mangle_text(policy_path.read_text(encoding="utf-8"))) monkeypatch.setenv("AIAC_POLICY_FILE", str(mech_policy_path)) - mech_rules, _, _ = orchestrate_prb(mech_roles, mech_scopes, _reordered(scenario)) + mech_rules, _, _, _ = orchestrate_prb(mech_roles, mech_scopes, _reordered(scenario)) mech_got = grant_sets(scenario, mech_rules) for gate in ("inbound", "outbound_subject", "outbound_target"): diff = want[gate] ^ mech_got[gate] @@ -164,7 +164,7 @@ def test_prb_robust_to_perturbation( p_roles, p_scopes = build_roles_and_scopes(perturbed) p_policy_path = Path(perturbed.__file__).resolve().parent / perturbed.POLICY_FILE monkeypatch.setenv("AIAC_POLICY_FILE", str(p_policy_path)) - sem_rules, _, _ = orchestrate_prb(p_roles, p_scopes, perturbed) + sem_rules, _, _, _ = orchestrate_prb(p_roles, p_scopes, perturbed) sem_got = grant_sets(scenario, sem_rules) for gate in ("inbound", "outbound_subject", "outbound_target"): diff = want[gate] ^ sem_got[gate] diff --git a/aiac/pyproject.toml b/aiac/pyproject.toml index 434529589..0a6e644e4 100644 --- a/aiac/pyproject.toml +++ b/aiac/pyproject.toml @@ -24,6 +24,7 @@ dependencies = [ # Test/dev tooling — install with: uv pip install -e ".[test]" test = [ "pytest>=9.1.1", + "pytest-xdist>=3.8.0", ] [tool.uv] diff --git a/aiac/uv.lock b/aiac/uv.lock index e3231cef7..130ffad50 100644 --- a/aiac/uv.lock +++ b/aiac/uv.lock @@ -33,6 +33,7 @@ dependencies = [ [package.optional-dependencies] test = [ { name = "pytest" }, + { name = "pytest-xdist" }, ] [package.metadata] @@ -46,6 +47,7 @@ requires-dist = [ { name = "nats-py" }, { name = "pydantic" }, { name = "pytest", marker = "extra == 'test'", specifier = ">=9.1.1" }, + { name = "pytest-xdist", marker = "extra == 'test'", specifier = ">=3.8.0" }, { name = "python-dotenv" }, { name = "python-keycloak" }, { name = "requests" }, @@ -603,6 +605,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/b0/0d/9feae160378a3553fa9a339b0e9c1a048e147a4127210e286ef18b730f03/durationpy-0.10-py3-none-any.whl", hash = "sha256:3b41e1b601234296b4fb368338fdcd3e13e0b4fb5b67345948f4f2bf9868b286", size = 3922, upload-time = "2025-05-17T13:52:36.463Z" }, ] +[[package]] +name = "execnet" +version = "2.1.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/bf/89/780e11f9588d9e7128a3f87788354c7946a9cbb1401ad38a48c4db9a4f07/execnet-2.1.2.tar.gz", hash = "sha256:63d83bfdd9a23e35b9c6a3261412324f964c2ec8dcd8d3c6916ee9373e0befcd", size = 166622, upload-time = "2025-11-12T09:56:37.75Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ab/84/02fc1827e8cdded4aa65baef11296a9bbe595c474f0d6d758af082d849fd/execnet-2.1.2-py3-none-any.whl", hash = "sha256:67fba928dd5a544b783f6056f449e5e3931a5c378b128bc18501f7ea79e296ec", size = 40708, upload-time = "2025-11-12T09:56:36.333Z" }, +] + [[package]] name = "fastapi" version = "0.141.1" @@ -2116,6 +2127,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/24/25/1de2678b631f5a49215c6c96fff41ba892b0a34df68d6d80292b1b48aa7f/pytest-9.1.1-py3-none-any.whl", hash = "sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c", size = 386536, upload-time = "2026-06-19T10:58:31.347Z" }, ] +[[package]] +name = "pytest-xdist" +version = "3.8.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "execnet" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/78/b4/439b179d1ff526791eb921115fca8e44e596a13efeda518b9d845a619450/pytest_xdist-3.8.0.tar.gz", hash = "sha256:7e578125ec9bc6050861aa93f2d59f1d8d085595d6551c2c90b6f4fad8d3a9f1", size = 88069, upload-time = "2025-07-01T13:30:59.346Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ca/31/d4e37e9e550c2b92a9cbc2e4d0b7420a27224968580b5a447f420847c975/pytest_xdist-3.8.0-py3-none-any.whl", hash = "sha256:202ca578cfeb7370784a8c33d6d05bc6e13b4f25b5053c30a152269fd10f0b88", size = 46396, upload-time = "2025-07-01T13:30:56.632Z" }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" From 2e5491849e5853113cd481493e76072b212b5a02 Mon Sep 17 00:00:00 2001 From: Amitfre15 Date: Tue, 8 Sep 2026 16:56:21 +0300 Subject: [PATCH 5/6] chore: Move passed section second in the eval report Was failed, error, xpassed, xfailed, skipped, passed. User wants the common case (passed) visible right after the section that needs attention (failed) instead of last. Assisted-By: Claude (Anthropic AI) Signed-off-by: Amitfre15 --- aiac/eval/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiac/eval/conftest.py b/aiac/eval/conftest.py index cbc82690a..9b2e03bc5 100644 --- a/aiac/eval/conftest.py +++ b/aiac/eval/conftest.py @@ -247,7 +247,7 @@ def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None: if not _reports: return # this session collected none of this suite's tests -- nothing to report - order = ["failed", "error", "xpassed", "xfailed", "skipped", "passed"] + order = ["failed", "passed", "error", "xpassed", "xfailed", "skipped"] buckets: dict[str, list[tuple[str, pytest.TestReport]]] = {cat: [] for cat in order} for nodeid, report in _reports.items(): buckets[_categorize(report)].append((nodeid, report)) From d960c04445124d481a69a3253edaaae7799a8f72 Mon Sep 17 00:00:00 2001 From: Amitfre15 Date: Wed, 9 Sep 2026 10:45:49 +0300 Subject: [PATCH 6/6] fix: Address CodeRabbit + review findings on the e2e correctness suite PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit conftest.py: the correctness-suite render branch dispatched on nodeid substring alone, so a skipped/xfailed entry (e.g. opa missing from PATH) was mislabeled "Failure" with "unavailable — scenario setup failed" instead of falling through to the generic branch and showing its real skip reason. Gated the branch on category in ("failed", "error") too. Verified against a scored pass/fail/skip/setup-failure matrix in an isolated repro harness: normally-scored entries already dispatched correctly on the presence of precision/recall properties (that part of the review finding didn't reproduce); only the skip/ xfail mislabeling was real. test_policy_pipeline_eval.py: documented two low-likelihood limits of the pipeline fixture's per-scenario isolation (an abnormal worker death raises BrokenProcessPool for every pending future in the pool, not just the crashed one; a worker killed externally leaves its idp/ store/opa subprocess trio orphaned, since running_services() only tears down on a normal Python-level exception) rather than engineering around them. Floored EVAL_PIPELINE_PARALLELISM at 1 so a 0/negative value gives a clear guard instead of an opaque ProcessPoolExecutor ValueError. best_effort_rules.py: added scope-focal exclusive=True coverage (previously only tested role-focal) and a test for exclusive=True combined with a non-empty explicit denied_names -- the shape a partially-approved multi-scope proposal can actually produce. correctness_e2e_helpers.py: documented that _user_role_rows' filter is a genuine no-op on the inbound maps (verified against the PCE's _derive in engine.py: RoleKind is an exhaustive USER/AGENT enum, and an AGENT-kind inbound edge is routed to the source bucket, never the subject bucket -- there's no path for an agent role to reach inbound_subject_allow/deny_rules), unlike the outbound side where it fixes a real double-count. Assisted-By: Claude (Anthropic AI) Signed-off-by: Amitfre15 --- aiac/eval/conftest.py | 8 +++++-- aiac/eval/correctness_e2e_helpers.py | 9 ++++++- aiac/eval/test_best_effort_rules.py | 33 ++++++++++++++++++++++++++ aiac/eval/test_policy_pipeline_eval.py | 15 +++++++++++- 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/aiac/eval/conftest.py b/aiac/eval/conftest.py index 9b2e03bc5..f925d7a85 100644 --- a/aiac/eval/conftest.py +++ b/aiac/eval/conftest.py @@ -164,7 +164,11 @@ def _format_pairs_dict(pairs_by_gate: dict) -> str: # Nodeid substrings identifying the two correctness suites' single test function each (parametrized # by scenario name) — used to give a scenario whose *setup* failed (before score_scenario ever ran, # so none of precision/recall/etc got record_property'd) the same six-field metrics shape every -# other entry gets, instead of silently omitting it. See `_render_entry`'s middle branch. +# other entry gets, instead of silently omitting it. See `_render_entry`'s middle branch. Gated on +# `category in ("failed", "error")` there too, not just this nodeid check — a *skipped*/xfailed +# correctness entry (e.g. `opa` missing from PATH) also matches this nodeid substring but never +# reached score_scenario for an unrelated, non-failure reason, so it must fall through to the +# generic branch and render its actual skip reason instead of a misleading "setup failed". _CORRECTNESS_TEST_MARKERS = ("::test_prb_correctness[", "::test_e2e_correctness[") @@ -224,7 +228,7 @@ def _render_entry(lines: list[str], nodeid: str, report: pytest.TestReport, cate if description: lines.append(f"- **What it tests:** {description}") _render_metrics_block(lines, props) - elif any(marker in nodeid for marker in _CORRECTNESS_TEST_MARKERS): + elif category in ("failed", "error") and any(marker in nodeid for marker in _CORRECTNESS_TEST_MARKERS): doc = _docstrings.get(nodeid) if doc: lines.append(f"- **What it tests:** {doc}") diff --git a/aiac/eval/correctness_e2e_helpers.py b/aiac/eval/correctness_e2e_helpers.py index b809fc17b..7fd7b02ce 100644 --- a/aiac/eval/correctness_e2e_helpers.py +++ b/aiac/eval/correctness_e2e_helpers.py @@ -26,7 +26,14 @@ def _user_role_rows(role_to_scopes: dict[str, list[str]], user_roles: set[str]) even though it's already correctly counted under ``outbound_target`` — confirmed empirically: a real run's every over-grant was exactly its scenario's ``outbound_target`` true positives, reappearing here. Mirrors the ``role.name in user_role_names`` discrimination - ``eval.test_policy_pipeline_eval.grant_sets`` already applies to the PRB's raw rules.""" + ``eval.test_policy_pipeline_eval.grant_sets`` already applies to the PRB's raw rules. + + Also applied to the inbound maps (``subject_role_allow/deny_scopes`` from the *inbound* Rego) + for the same reason, though it's a no-op there in practice: the inbound Rego has no + agent-calling-agent concept to begin with, so its ``subject_role`` rows are user roles only — + nothing gets filtered out. Applying the same call uniformly to both directions, rather than + conditionally skipping it for inbound, avoids two different call shapes for what is + conceptually the same "keep user rows only" step.""" return {role: scopes for role, scopes in role_to_scopes.items() if role in user_roles} diff --git a/aiac/eval/test_best_effort_rules.py b/aiac/eval/test_best_effort_rules.py index 38aacbf2f..2cbf4d7c4 100644 --- a/aiac/eval/test_best_effort_rules.py +++ b/aiac/eval/test_best_effort_rules.py @@ -78,3 +78,36 @@ def test_empty_proposal_yields_no_rules() -> None: scopes = [_scope("tool-scope-a")] state = {"selected_names": [], "denied_names": [], "exclusive": False} assert _best_effort_rules({"role": role, "scopes": scopes}, state) == [] + + +def test_scope_focal_exclusive_denies_the_unselected_complement() -> None: + scope = _scope("agent-scope-tracker-access") + roles = [_role("user-role-developer"), _role("user-role-tester"), _role("user-role-manager")] + state = {"selected_names": ["user-role-tester"], "denied_names": [], "exclusive": True} + rules = _best_effort_rules({"scope": scope, "roles": roles}, state) + + denied_role_names = {r.role.name for r in rules if r.effect == RuleEffect.DENY} + assert denied_role_names == {"user-role-developer", "user-role-manager"} + + +def test_scope_focal_exclusive_with_explicit_denied_names_is_the_same_complement() -> None: + """A partially-approved multi-scope proposal can leave `exclusive=True` set alongside an + explicit `denied_names` the auditor approved before rejecting the rest — exercises the + combination `_denied_names` is designed for (explicit denials unioned with the derived + complement), not just each independently (see `test_role_focal_exclusive_denies_the_unselected_ + complement`/`test_scope_focal_allows_selected_and_denies_explicit`, which each cover only one + side).""" + scope = _scope("agent-scope-tracker-access") + roles = [_role("user-role-developer"), _role("user-role-tester"), _role("user-role-manager")] + state = { + "selected_names": ["user-role-tester"], + "denied_names": ["user-role-developer"], # explicit — already part of the complement too + "exclusive": True, + } + rules = _best_effort_rules({"scope": scope, "roles": roles}, state) + + allow_role_names = {r.role.name for r in rules if r.effect == RuleEffect.ALLOW} + denied_role_names = {r.role.name for r in rules if r.effect == RuleEffect.DENY} + assert allow_role_names == {"user-role-tester"} + assert denied_role_names == {"user-role-developer", "user-role-manager"} + assert len(rules) == 3 # no duplicate DENY rule for the role in both the explicit set and the complement diff --git a/aiac/eval/test_policy_pipeline_eval.py b/aiac/eval/test_policy_pipeline_eval.py index e61353332..de44e781b 100644 --- a/aiac/eval/test_policy_pipeline_eval.py +++ b/aiac/eval/test_policy_pipeline_eval.py @@ -66,6 +66,16 @@ realm provisioning is a small fraction of one scenario's wall-clock next to the PRB's several sequential LLM calls. +Two known limits of this isolation, both low-likelihood for this workload and left as-is rather +than engineered around: an ordinary Python exception in one worker is caught and isolated to that +scenario alone (see the per-future ``except Exception`` below), but an *abnormal* worker death +(OOM-kill, segfault) raises ``concurrent.futures.process.BrokenProcessPool`` for every other +pending/in-flight future in the same pool too, not just the one that crashed — a wider blast radius +than "isolate one scenario's worker crash from the rest" implies. And each worker's own +``running_services(...)`` context manager tears down its idp/store/opa subprocess trio on a normal +Python-level exception, but there's no top-level safety net if the worker *process* itself is +killed externally — those subprocesses would be orphaned rather than cleaned up. + ``_provision_scenario`` calls ``orchestrate_prb(..., best_effort=True)`` — a scope/role decision the PRB's auditor rejects contributes a best-effort (never-approved) fallback rule instead of aborting the whole scenario (see ``orchestrate_prb``/``_invoke_graph``'s docstrings), by explicit @@ -766,7 +776,10 @@ def pipeline() -> dict[str, dict]: "LLM_API_KEY", ) - max_workers = int(os.environ.get("EVAL_PIPELINE_PARALLELISM", str(len(SCENARIOS)))) + # max(1, ...): ProcessPoolExecutor raises a bare ValueError("max_workers must be greater than + # 0") for 0 or negative — clearer to floor it here than to let a maintainer's typo in this + # escape-hatch env var surface as an opaque crash deep inside the executor. + max_workers = max(1, int(os.environ.get("EVAL_PIPELINE_PARALLELISM", str(len(SCENARIOS))))) realm_lock = multiprocessing.Lock() # serializes admin.create_realm — see _provision_scenario results: dict[str, dict] = {} with ProcessPoolExecutor(max_workers=max_workers, initializer=_init_worker, initargs=(realm_lock,)) as executor: