Python: Bound abandoned PolicyEnforcement pending approvals

[Shivani . (Shivani767)]

Motivation & Context

PolicyEnforcementFunctionMiddleware retains pending policy-approval entries until they are successfully consumed. When approval_on_violation=True and approvals are abandoned, rejected, or never returned, _pending_policy_approvals can grow without bound on a long-lived middleware instance.

Description & Review Guide

• What are the major changes?

  • Cap pending approvals with FIFO eviction (max_pending_policy_approvals, default 256).
  • Expire abandoned approvals after a TTL (pending_policy_approval_ttl, default 1 hour; None disables).
  • Clear pending state when an approval is explicitly rejected via the approval resolver (rejected decisions do not re-enter function middleware).
  • Add regression tests for max-size eviction, TTL discard, and resolver-path rejection cleanup.

• What is the impact of these changes?

  • Abandoned approvals can no longer accumulate unbounded memory on reused middleware instances.
  • Valid approve-and-replay flows and the call-binding isolation from Python: bind policy-enforcement approvals to a single tool invocation #6966 are unchanged.
  • Callers can tune the bounds via keyword-only constructor args.

• What do you want reviewers to focus on?

  • Whether the default max/TTL are appropriate for experimental FIDES policy approvals.
  • Whether notifying middleware from _resolve_approval_responses is the right seam for rejection cleanup.

Related Issue

Fixes #7890

Note: Patel Namraa (@Namraa310806) previously offered to work on this; there was no open PR when this was started. Happy to coordinate if preferred.

Contribution Checklist

• The code builds clean without any errors or warnings
• All unit tests pass, and I have added new tests where possible
• The PR follows the Contribution Guidelines
• This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
• This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

[Copilot]

Pull request overview

Bounds abandoned policy-approval state in long-lived Python middleware instances.

Changes:

• Adds FIFO size limits and TTL expiration.
• Adds rejection cleanup logic and regression tests.
• Rejection cleanup is not reached by the normal agent approval flow.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
python/packages/core/agent_framework/security.py	Implements bounded pending-approval lifecycle.
python/packages/core/tests/test_security.py	Tests eviction, expiration, and direct rejection handling.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Shivani . (Shivani767)]

Good catch — rejected approvals never re-enter process().

Updated to notify middleware from _resolve_approval_responses via FunctionMiddlewarePipeline.notify_rejected_approvals → PolicyEnforcementFunctionMiddleware.discard_rejected_policy_approvals, and replaced the direct-process test with a resolver resume regression that asserts pending state is cleared without executing the tool.

[Patel Namraa (Namraa310806)]

Shivani . (@Shivani767) Nice work on this! I was actually working on #7890 too and had a fix ready locally, but saw your draft first and compared against it instead of duplicating the effort. I verified locally against the original repro — 5,000 unapproved calls are bounded at 256 instead of growing unbounded — and the PolicyEnforcement tests pass.

A few optional thoughts from comparing the approaches, none blocking:

1. test_expired_policy_approvals_are_discarded uses a real asyncio.sleep(0.02) against a 1ms TTL — the margin is comfortable, but an injectable clock would make it fully deterministic under CI load.

2. FIFO eviction at the pending-approval limit is silent — a logger.debug there might help diagnose a "live" approval that stops matching after eviction.

3. Since _prune_pending_policy_approvals() already drops expired entries, is the extra _pending_approval_is_alive() check in _matches_pending_approval() intentional defense-in-depth? Might be worth a comment either way.

The resolver-side rejection cleanup is a nice catch — I hadn't considered that rejected approvals skip the normal middleware path.

To be clear, these are just suggestions, so take or leave them. I won't open a competing PR — I just want to see #7890 fixed correctly, and it doesn't matter whose PR it is. Nice work!

[Shivani . (Shivani767)]

Patel Namraa (@Namraa310806) Thanks a lot, Namraa! Really appreciate you taking the time to verify the fix against the original repro and for sharing these suggestions.

I’ll take a closer look at the deterministic clock approach, the optional debug logging for FIFO eviction, and the defense-in-depth check in _matches_pending_approval(). Your point about the resolver-side rejection path was especially helpful — it exposed an important gap in my initial test coverage.

Glad we can get #7890 fixed without duplicating the work. Thanks again for the review and validation!