feat(governance): guardrail-fallback compensation#123
Open
aditik0303 wants to merge 14 commits into
Open
Conversation
42e1f29 to
7111d27
Compare
8812695 to
2da1f67
Compare
7111d27 to
2db5f2d
Compare
2da1f67 to
5b119ac
Compare
2db5f2d to
bd19932
Compare
5b119ac to
2154aba
Compare
bd19932 to
add4e90
Compare
2154aba to
1f7bdad
Compare
Merged
6 tasks
d500f99 to
7cdc791
Compare
fb9756e to
0e9ad5b
Compare
7cdc791 to
4da27ce
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…not uipath-platform - guardrail_compensation.py: _resolve_trace_id reads the UIPATH_TRACE_ID env var via the runtime-local ENV_TRACE_ID constant instead of UiPathConfig; log messages no longer reference uipath-platform. - test_guardrail_compensation: import reset helper from tests._helpers; the trace-id fallback test pins UIPATH_TRACE_ID via monkeypatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ve_trace_id Restores the conversational trace-id binding fix. Native governance audit spans are exported under UIPATH_TRACE_ID (the platform rebinds spans to the agent's run trace), so the /govern compensation records must bind to that same id — not the live OTel span's id, which diverges in autonomous runs and is absent on the conversational hook thread. Resolve UIPATH_TRACE_ID first, then the live span, then the caller fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The compensation path used to hand-roll the HTTP POST — URL composition, auth, headers, JSON, env-backed job-context resolution. uipath-core now exposes a GovernanceCompensationProvider protocol and uipath-platform ships UiPathPlatformGovernanceProvider as the concrete implementation, so the runtime no longer needs any of that wire-level code. - submit_compensation gains a provider: GovernanceCompensationProvider first argument; the worker thread calls provider.compensate(request) with a GovernRequest built from the fired-rule metadata - delete request_governance (urllib/JSON/headers/auth all gone — that's the platform service's job; folder_key/job_key/process_key/reference_id/ agent_version are auto-filled by the provider from UiPathConfig) - disabled_guardrails returns list[FiredRule] (uipath-core pydantic wire model) instead of a list of TypedDicts - inline ENV_TRACE_ID + COMPENSATION_MAX_WORKERS — backend_client no longer exists on this branch and these were its only remaining users Tests: drop the 14 HTTP/auth/URL/header/payload tests (now provider concerns covered in uipath-platform); add provider-invocation tests (GovernRequest assembly, validator dedup, error swallowing); guard the evaluator-integration tests with importorskip so the file collects on this branch — they need rewriting when the evaluator lands to match the new provider-first signature. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…from wiring Addresses radu's recurring PR #121 patterns applied to the guardrail compensation slice. Resolves the post-PR-#121 ImportError in the test file (it referenced the deleted ``uipath.runtime.governance.config`` / ``tests._helpers.reset_enforcement_mode``). Architectural — match the AuditManager / PolicyLoader shape - New GuardrailCompensator class. Each GovernanceRuntime instance gets one — owns its own ThreadPoolExecutor, BoundedSemaphore, and provider. uipath eval parallel runtimes no longer share workers, queue slots, or saturation state. - Module globals _pool / _inflight / _INFLIGHT_CAP / @atexit.register decorator removed. Process cleanup via a weakref.WeakSet of live compensators + one process-level atexit hook (same pattern PR #122 introduced for AuditManager): N runtimes → 1 atexit slot, no strong ref pinning disposed compensators. - close() is an instance method, idempotent, logs at debug on failure. - The free submit_compensation function is gone — callers use compensator.submit(...). Boundary — env reads move to the wiring layer - _resolve_trace_id signature changed to (supplied, fallback). It no longer reads UIPATH_TRACE_ID. The runtime layer is now env-free for this code path. - GovernanceRuntime accepts a trace_id: str | None constructor arg and exposes it via the .trace_id property. The wiring layer (uipath CLI) reads UIPATH_TRACE_ID and passes the value in; the evaluator slice forwards it into GuardrailCompensator(provider, trace_id=...). - GuardrailCompensator accepts trace_id at construction; it becomes the authoritative source. Per-submit trace_id is a per-call fallback. Polish - Replaced bare except Exception: pass in _resolve_trace_id with a logger.debug (bandit B110 cleared on this file). - Removed ENV_TRACE_ID constant + the os import that backed it. Tests - Full rewrite of test_guardrail_compensation to drop deleted imports (config, reset_enforcement_mode), use GuardrailCompensator(provider), and mirror AuditManager's lifecycle test set (one atexit registration, weakref GC, idempotent close, cross-instance isolation, semaphore release on provider error). - New test_resolve_trace_id_does_not_read_env pins the boundary rule: even with UIPATH_TRACE_ID set, the runtime layer ignores it. - New test_compensator_trace_id_overrides_caller_supplied_value pins the construction-supplied value winning over per-submit. - New test_governance_runtime_stashes_trace_id + test_governance_runtime_default_trace_id_is_none cover the new GovernanceRuntime kwarg + property. 238 passed, ruff/mypy clean; bandit clean on the touched files (one pre-existing B101 in _yaml_to_index.py is unchanged and out of scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0e9ad5b to
470533e
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… in rule + cross-rule aggregation; align vader threshold default to -0.3 (matches docstring/comment/else + YAML default); importorskip wrapper in text-extraction test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… import - evaluator.py: inline `# type: ignore[import-untyped]` on the vaderSentiment import (replaces the removed [[tool.mypy.overrides]] entry; vaderSentiment ships no stubs). - test_evaluator / test_evaluator_operators: import reset helper from tests._helpers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…orts Closes radu's recurring boundary objection for the evaluator slice and makes the post-rebase stack actually import. The evaluator was the last place where everything PR #121-#123 instance-scoped collapsed back to process globals. Architectural - GovernanceEvaluator gains constructor injection: GovernanceEvaluator(policy_index, *, enforcement_mode=AUDIT, audit_manager=None, compensator=None) - Drop get_audit_manager() / get_enforcement_mode() / submit_compensation free-function lookups. The evaluator now consults zero process-globals on the hot path. - mode property is read-only (drop the setter); no two-writer race between the loader and evaluator. - audit_manager=None and compensator=None short-circuit cleanly so tests + minimal wirings work without injecting every dep. - Drop unused is_enforce_mode() public method (dead code; no caller in src/ or tests/). Post-rebase plumbing - _dispatch_compensation uses self._compensator.submit(...) instead of the deleted free function; reads r.validator (Pydantic attribute) instead of the old r["validator"] TypedDict access. - _emit_audit passes policy_id (PR #122 trace-contract field, was rule_id) and enforcement_mode=mode enum (PR #122 required arg). - Import EnforcementMode from uipath.core.governance (governance.config deleted in PR #121); import AuditManager from _audit.base (audit/ is _audit/ post-PR-#122). native/__init__.py - Drop the four module-level loader-function re-exports (get_policy_index / load_policy_index / prefetch_policy_index / reset_policy_index) — all deleted in PR #121's PolicyLoader refactor. - Export PolicyLoader instead. Tests - test_evaluator: full rewrite. Drop deleted-import paths (tests._helpers.reset_enforcement_mode, governance.config). Replace the global-manager fixture with a per-test AuditManager that uses register_default_sinks=False + a capturing sink. Every GovernanceEvaluator() call routes through a _build_evaluator helper with explicit mode + manager. New test_no_audit_manager_short_circuits replaces the previous test that mocked the global to raise. - test_evaluator_operators: drop the autouse mode-isolating fixture (no globals to isolate); DISABLED-mode test passes enforcement_mode=EnforcementMode.DISABLED via constructor. - test_guardrail_compensation: rebase-conflict resolution dropped the stale incoming-side imports (Action/LifecycleHook, backend_client, unguarded GovernanceEvaluator) since none of them are referenced in the rest of the file. 357 passed, 1 skipped (pre-existing wrapper skip). Ruff clean. Mypy clean (11 source files). Bandit shows only the pre-existing B101 in _yaml_to_index.py (out of scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
viswa-uipath
added a commit
that referenced
this pull request
Jun 25, 2026
…Runtime Closes architecture-review §2.1 + §2.2 — the UiPathWrappedRuntimeFactory bolted governance onto the generic runtime-factory registry (apply_wrappers=True turned every registered factory into a different type, breaking isinstance checks), and the second GovernanceRuntime in governance/wrapper.py reached into delegate._agent_definition / framework-specific private attrs through a 10-level walk to install framework-blind callbacks. Both patterns the doc unambiguously says to delete. Composition belongs in the host's decorator chain, FF-gated, where UiPathResumableRuntime already wraps the framework runtime; this PR's wrapper machinery was an end-run around that. Deletions - src/uipath/runtime/governance/wrapper.py (1002 LOC) — the second GovernanceRuntime with _AGENT_ATTRS / _replace_agent_in_delegate / model-context-var introspection. - src/uipath/runtime/wrapper.py (55 LOC) — the lazy-import dispatch shim that called the deleted governance_wrapper. - tests/test_dispose_isolation.py, tests/test_wrapper.py, tests/test_wrapper_internals.py (~650 LOC combined) — entire test suites for the deleted modules. Updates - src/uipath/runtime/registry.py — UiPathWrappedRuntimeFactory class and the apply_wrappers kwarg removed from get(). The registry returns the registered factory unchanged; cross-cutting concerns (governance, audit, …) are composed by the host into the decorator chain, not auto-applied here. - src/uipath/runtime/__init__.py — drop GOVERNANCE_FEATURE_FLAG / apply_governance_wrapper exports. - tests/test_registry.py — strip every apply_wrappers=False kwarg (the kwarg is gone) and drop the wrapping-behaviour section + its fixtures. Conflict resolution The rebase onto #125's tip replayed the upstream e186f5f commit (a cosmetic helper-import touch) into three test files that my PR #122/#123/#124 refactors had already rewritten end-to-end. HEAD-side resolution kept the refactored form in test_evaluator.py, test_evaluator_operators.py, test_guardrail_compensation.py — the incoming side referenced symbols (governance.audit, governance.config, tests._helpers.reset_enforcement_mode) that the post-rebase stack no longer ships. Verification - Monorepo grep for UiPathWrappedRuntimeFactory, apply_wrappers, apply_governance_wrapper, governance_wrapper, and the deleted module import paths: zero hits. - ruff clean, mypy clean (45 source files), 357 passed + 1 skipped. Net diff on top of #125's tip: −2005 / +38 LOC = −1967 net. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GovernanceRuntime now takes a resolved PolicyIndex + EnforcementMode at construction. The host (uipath CLI) does the async fetch via the GovernancePolicyProvider, compiles the YAML through build_policy_index_from_yaml, and hands the snapshot in. The runtime becomes a passive consumer; the host owns lifecycle. - Delete PolicyLoader (343 LOC) and its hand-rolled future (threading.Thread + Event). Async I/O belongs to the async host. - Delete StubPolicyProvider test helper + enforcement-mode-default tests (the mode is now a constructor arg, no default needed). - GovernanceRuntime ctor: (delegate, policy_index, enforcement_mode, *, trace_id=None). No more policy_provider / is_conversational parameters. Agent-type selection lives in the host's PolicyContext construction. - Expose build_policy_index_from_yaml from native/__init__.py for the host's compile step. Net: -890 LOC. Addresses architecture-review item Sec 2.4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- runtime.py: drop §2.4 PR ref and historical "staging caveat" language from module/class docstrings; drop downstream LangChain class name from the generic runtime layer; replace defensive getattr(result, "output", None) with result.output (the outer fail-open try/except already covers a malformed delegate). - evaluator.py: fix stale "loader" reference in docstring → GovernanceRuntime. - _audit/traces.py: rewrite three comments referencing the deleted PolicyLoader to describe the per-runtime model. - _audit/base.py: rewrite two docstrings referencing the deleted PolicyLoader. - native/_yaml_to_index.py: fix broken :mod: link to the deleted native.loader module; describe the platform-host compile flow. No behavior change. ruff/mypy clean, 326 passed + 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cristipufu
reviewed
Jun 26, 2026
feat(governance): in-runtime policy evaluator + native exports
refactor(governance): hoist policy fetch to host; drop PolicyLoader
…contextvars Addresses cristipufu's PR #133 review (rename + drop properties + drop local serializer) and the wider point that ``trace_id`` shouldn't live on the generic runtime layer at all. The platform side (uipath-platform / PR #1761) now self-resolves ``GovernRequest.trace_id`` when the runtime sends an empty value, and the compensator preserves live OTel context across its background-pool hop via ``contextvars.copy_context()`` — so the platform-side resolver still sees the agent's live span when the worker calls ``provider.compensate(...)``. Runtime wrapper (``runtime.py``) - Renamed ``GovernanceRuntime`` → ``UiPathGovernedRuntime`` to match the repo's other runtime names (UiPathResumableRuntime, UiPathDebugRuntime, etc.). - Dropped ``trace_id`` ctor arg. - Dropped the ``policy_index`` / ``enforcement_mode`` / ``trace_id`` read-only properties — they were dead surface area; consumers receive the values from the host at construction time and don't need to read them back through the wrapper. - Replaced the bespoke ``_serialize_payload`` (4 branches + nested try/except) with a 9-line version that delegates the complex case to ``uipath.core.serialization.serialize_object``. ``None → ""`` and ``str → passthrough`` stay as governance-scan special cases (the evaluator's regex / contains / sentiment checks would mismatch against ``"null"`` or ``'"hello"'``). Compensator (``guardrail_compensation.py``) - Dropped ``trace_id`` ctor arg. - Dropped the per-call ``trace_id`` arg from ``submit()``. - Deleted the ``_resolve_trace_id(supplied, fallback)`` helper. - Added ``import contextvars``; ``submit()`` snapshots the caller's context (``ctx = contextvars.copy_context()``) and the pool runs the worker as ``pool.submit(ctx.run, _run)``. The worker therefore sees the agent's live OTel span; the platform's ``resolve_trace_id`` resolves correctly on the worker thread. - ``GovernRequest.trace_id="" `` on the wire — platform fills. Evaluator (``native/evaluator.py``) - All six ``evaluate_*`` per-call methods now default ``trace_id: str = ""`` (was required). Callers that already supply a value (e.g. legacy callers passing through resolved ids) continue to work unchanged. - ``_dispatch_compensation`` no longer passes ``trace_id`` to ``compensator.submit(...)``. Tests - ``test_governance_runtime.py``: rewritten for the renamed class + dropped properties + dropped ctor arg. Asserts internal ``_policy_index`` / ``_enforcement_mode`` instead of properties. - ``test_guardrail_compensation.py``: dropped the four ``_resolve_trace_id`` tests + the constructor-trace-id test. Replaced ``test_submit_captures_live_trace_before_thread_hop`` with ``test_submit_propagates_otel_context_to_worker_thread``: now asserts that ``trace.get_current_span()`` *inside the worker callable* returns the agent's live span (proves the contextvars snapshot propagation works end-to-end). 319 passed, 1 skipped. - ``conftest.py`` / ``test_traces_severity.py``: docstring renames only. ruff + mypy clean (10 source files). Test count: 319 passed, 1 skipped (was 357 — drop is the deleted ``_resolve_trace_id`` tests + the ctor-trace-id test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked PR 4/7 — part of splitting
feat/governance-coreinto reviewable slices. Base:feat/governance-audit. One logical slice (branch is cumulative so CI is green). Merge in order #1 → #7 and delete each branch on merge so the next PR auto-retargets ontofeat/agentic-governance.feat/governance-corekept untouched as backup.