Skip to content

feat: Add PolicyEngine.EvaluateAsync so external backends aren't forced into sync-over-async - #3778

Open
Garrett Beatty (GarrettBeatty) wants to merge 1 commit into
microsoft:mainfrom
GarrettBeatty:policyengine-evaluateasync
Open

feat: Add PolicyEngine.EvaluateAsync so external backends aren't forced into sync-over-async#3778
Garrett Beatty (GarrettBeatty) wants to merge 1 commit into
microsoft:mainfrom
GarrettBeatty:policyengine-evaluateasync

Conversation

@GarrettBeatty

Copy link
Copy Markdown

Draft implementation for #3775.

Problem

PolicyEngine only exposes a synchronous Evaluate that calls IExternalPolicyBackend.Evaluate. A backend whose decision requires a network call (an async-only SDK) has to block a thread with .GetAwaiter().GetResult(), which pressures the thread pool under load.

Change

  • Extract the shared prologue (Prepare: validate, snapshot, build context, run in-memory policies) and result combination (Combine: fail-closed resolution) out of Evaluate.
  • Add PolicyEngine.EvaluateAsync(agentDid, context, ct) that awaits each backend's EvaluateAsync. External backends run sequentially to preserve ordering and avoid assuming they're concurrency-safe.
  • Sync Evaluate keeps its exact previous behavior; internal policy evaluation and fail-closed semantics are unchanged.

Tests

Adds coverage that EvaluateAsync uses the backend async path, Evaluate uses the sync path, and EvaluateAsync still fails closed on a backend error. All existing policy tests pass (146 total).

Open questions for maintainers

  • Should the async path extend up through GovernanceKernel.EvaluateToolCall (and the MCP decorator, which is already async)? Kept this PR to the engine to stay focused; happy to follow up.
  • Sequential vs. concurrent backend evaluation — went sequential to match current semantics.

Draft: opening to align on shape before finalizing.

…c-over-async

PolicyEngine only exposed a synchronous Evaluate that calls IExternalPolicyBackend.Evaluate,
so backends whose work is async (a network call) had to block a thread with GetAwaiter().GetResult().

- Extract the shared prologue (Prepare) and result combination (Combine) from Evaluate.
- Add PolicyEngine.EvaluateAsync(agentDid, context, ct) that awaits backends' EvaluateAsync;
  external backends are evaluated sequentially to preserve ordering. Internal (in-memory)
  policy evaluation and the fail-closed resolution are unchanged.
- Sync Evaluate keeps its exact previous behavior.

Adds tests that EvaluateAsync uses the backend async path (and Evaluate the sync path),
plus async fail-closed on backend error.

Refs microsoft#3775
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions

Copy link
Copy Markdown

PR Review Summary

Check Status Details
🔍 Code Review ⚠️ Missing No current-run comment
🛡️ Security Scan ⚠️ Missing No current-run comment
🔄 Breaking Changes ⚠️ Missing No current-run comment
📝 Docs Sync ⚠️ Missing No current-run comment
🧪 Test Coverage ⚠️ Missing No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

@github-actions github-actions Bot added tests size/M Medium PR (< 200 lines) labels Aug 18, 2026
@GarrettBeatty

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Amazon"

@GarrettBeatty Garrett Beatty (GarrettBeatty) changed the title Add PolicyEngine.EvaluateAsync so external backends aren't forced into sync-over-async feat: Add PolicyEngine.EvaluateAsync so external backends aren't forced into sync-over-async Aug 18, 2026
@GarrettBeatty
Garrett Beatty (GarrettBeatty) marked this pull request as ready for review August 18, 2026 23:32
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR (< 200 lines) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant