fix(async-evaluator): drop redundant asyncio.Lock from evaluate (+ thread-safe RW lock prerequisite)

[Ken Tannenbaum (finnoybu)]

Summary

AsyncPolicyEvaluator.evaluate (agent_os/policies/async_evaluator.py:172-175) wrapped its run_in_executor call in async with self._async_lock, fully serializing every async evaluation across the event loop. The lock served no correctness purpose — the underlying _ReadWriteLock and ConcurrencyStats updates are already thread-safe — but it forced concurrent await evaluate(...) calls to run one at a time, defeating the whole reason for using an executor.

While building the regression test for that fix, the executor's actual concurrent execution exposed a latent thread-affinity bug in _ReadWriteLock: it acquired the inner threading.RLock writer lock on the first reader thread that took the count from 0 to 1, and released it on whichever reader thread later took the count back to 0. With true parallel execution those are different threads, and RLock.release raises RuntimeError from any thread other than the one that acquired it. The asyncio.Lock had been masking this by serializing everything to the same executor thread at any given moment.

This PR is two commits: the prerequisite RW-lock fix, then the named HIGH item.

Commit 1 — fix(async-evaluator): make _ReadWriteLock writer release thread-safe

Replace the two nested RLocks with a single threading.Condition that counts active readers and writers. Acquire/release are no longer thread-affine; readers and writers signal each other via notify_all when their counts drop to zero. This is the standard readers-writer-lock pattern and unblocks landing the asyncio.Lock removal without intermittent crashes.

Commit 2 — fix(async-evaluator): drop redundant asyncio.Lock from evaluate (HIGH #7)

• Remove async with self._async_lock: from evaluate. The executor call now stands alone; concurrency is bounded by the executor pool size and the inner RW lock.
• The asyncio.Lock is retained for reload_policies, where coroutine-level serialization across reload requests is still useful.
• Class- and module-level docstrings updated to reflect that the asyncio.Lock now applies to reloads only.

Tests

Adds test_async_evaluations_run_concurrently — 8 concurrent await evaluate(...) calls against an evaluator artificially slowed by a 50ms sleep. Asserts concurrent_peak > 1. Before this PR (with the asyncio.Lock holding the executor in serial), concurrent_peak topped out at 1; after, it comfortably exceeds 1 in the default thread pool.

tests\test_async_evaluator.py ................................           [100%]
======================== 32 passed, 1 warning in 1.55s ========================

(31 existing + 1 new.)

Test plan

• All 32 test_async_evaluator.py tests pass on Python 3.14 (Windows).
• Regression test demonstrates real parallel async evaluation.
• RW-lock thread-safety fix verified independently — commit 1 alone keeps existing 31 tests green.
• No change to evaluate_sync or reload_policies semantics.

Reported via the AEGIS Initiative review of microsoft/agent-governance-toolkit.

Signed-off-by: Kenneth Tannenbaum ktannenbaum@aegis-initiative.com

🤖 Generated with Claude Code

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

API Compatibility

Severity	Change	Impact
High	Removed asyncio.Lock from AsyncPolicyEvaluator.evaluate method.	Concurrent calls to evaluate are no longer serialized at the coroutine level, which may lead to unexpected behavior if users relied on this implicit serialization.
Medium	Modified _ReadWriteLock implementation to use threading.Condition instead of nested threading.RLocks.	Potential impact on users directly relying on _ReadWriteLock behavior, especially if they extended or subclassed it.

[github-actions]

🤖 AI Agent: security-scanner — View details

No security issues found.

[github-actions]

🤖 AI Agent: code-reviewer — Action Items:

TL;DR: 0 blockers, 1 warning. Fixes critical concurrency and thread-safety issues effectively.

#	Sev	Issue	Where
1	Warn	Missing test coverage for edge cases in _ReadWriteLock implementation	agent_os/policies/async_evaluator.py

Action Items:

• None. No blockers identified.

Warnings:

#	Issue	Fine as follow-up PRs
1	Add tests for edge cases in _ReadWriteLock (e.g., multiple writers, edge timing scenarios).	Yes

[github-actions]

🤖 AI Agent: test-generator — `agent_os/policies/async_evaluator.py`

agent_os/policies/async_evaluator.py

• test_evaluate_with_invalid_context -- Add a test to validate behavior when evaluate is called with an invalid or malformed context.
• test_evaluate_with_empty_context -- Test evaluate with an empty context to ensure proper handling of edge cases.
• test_evaluate_with_large_context -- Validate evaluate with a very large context to test performance and memory handling.
• test_reload_policies_concurrent_access -- Test reload_policies with concurrent access to ensure the asyncio.Lock correctly serializes reloads.
• test_rw_lock_writer_priority -- Add a test to ensure writers in _ReadWriteLock are not indefinitely starved by readers.

agent_os/policies/_ReadWriteLock

• test_rw_lock_multiple_readers -- Validate that multiple readers can acquire the lock concurrently.
• test_rw_lock_reader_writer_conflict -- Test behavior when a writer attempts to acquire the lock while readers are active.
• test_rw_lock_writer_release_thread_safety -- Ensure that writers can release the lock safely even when executed across threads.
• test_rw_lock_reader_writer_interleaving -- Test interleaving of readers and writers to ensure proper synchronization.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

Docs Sync

• Documentation is in sync.

[github-actions]

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	NONE
Overall	MEDIUM

Automated check by AGT Contributor Check.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Warning	See details
🛡️ Security Scan	✅ Passed	No issues found
🔄 Breaking Changes	✅ Completed	Analysis complete
📝 Docs Sync	✅ Passed	No issues found
🧪 Test Coverage	✅ Completed	Analysis complete

Verdict: ⚠️ Ready for human review

[Imran Siddique (imran-siddique)]

LGTM — Condition-based RW lock fixes cross-thread RLock release; async lock removal is correct.