feat(crewai): add native GovernanceHooks using CrewAI execution hooks#1588
Conversation
Replace fragile proxy-based monkey-patching with CrewAI's native execution hooks (@before_tool_call, @after_tool_call, @before_llm_call, @after_llm_call) introduced in CrewAI 0.80+. Changes: - Add GovernanceHooks class that registers four global governance hooks for tool allowlist, blocked-pattern scanning, Cedar/OPA pre_execute gates, and output content filtering - Add CrewAIKernel.as_hooks() factory method as the recommended integration path - Deprecate CrewAIKernel.wrap() with DeprecationWarning pointing to as_hooks() - Export CrewAIGovernanceHooks from integrations package - Add 43 new tests covering all four hook types, deprecation warnings, Cedar evaluator integration, and backward compatibility - All 12 existing CrewAI regression tests pass unchanged Resolves microsoft#1576
🤖 AI Agent: docs-sync-checker — Docs SyncDocs Sync
|
🤖 AI Agent: breaking-change-detector — API CompatibilityAPI Compatibility
|
🤖 AI Agent: security-scanner — View detailsNo security issues found. |
🤖 AI Agent: code-reviewer — Review SummaryReview SummaryThis pull request introduces a significant enhancement to the Below is a detailed review of the changes, focusing on the specified focus areas. CRITICAL
WARNING
SUGGESTIONS
ConclusionThis PR introduces a robust and modular approach to governance in the |
🤖 AI Agent: test-generator — `agent_os/integrations/crewai_adapter.py`Test Coverage Analysis
|
PR Review Summary
Verdict: ❌ Changes needed |
imran-siddique
left a comment
There was a problem hiding this comment.
Code Review: CrewAI GovernanceHooks
Clean API design, @miyannishar. The as_hooks() pattern is consistent with the other adapters. Two blocking issues:
Blocking
1. unregister() doesn't actually remove hooks from CrewAI
unregister() sets self._registered = False and clears self._hook_fns, but the closures registered with CrewAI still hold live references and continue enforcing governance. The user believes they cleaned up, but governance is silently still active.
Fix: add an _active guard inside each hook closure:
python def governance_before_tool(context): if not self._registered: return None # passthrough when unregistered # ... existing logic
2. Non-string tool/LLM results bypass blocked-pattern scanning
after_tool_call and after_llm_call only scan str results. Dict, list, or object results skip scanning entirely. A tool returning {"query": "DROP TABLE users"} passes undetected. The before_tool_call hook already handles this correctly by coercing with str(tool_input). Apply the same pattern to after hooks.
Warnings
3. call_count incremented before max-check
Blocked calls still inflate the counter. Check before incrementing:
python if kernel.policy.max_tool_calls and ctx.call_count + 1 > kernel.policy.max_tool_calls: return False ctx.call_count += 1
4. Double deprecation warning from module-level wrap()
Same pattern as other adapters. Suppress the inner warning.
5. before_llm_call uses inconsistent content extraction for Cedar/OPA
The per-message loop handles dict/str/object, but combined_input for Cedar/OPA only handles two types. Extract a helper _extract_content(msg) and use in both places.
imran-siddique
left a comment
There was a problem hiding this comment.
Updated review (condensed):
TL;DR: 2 blockers. Fix those and this ships.
| # | Sev | Issue | Where |
|---|---|---|---|
| 1 | Block | unregister() doesn't guard hook closures -- governance silently persists after cleanup |
unregister() |
| 2 | Block | Non-string tool/LLM results bypass blocked-pattern scanning | after_tool_call, after_llm_call |
| 3 | Warn | call_count incremented before max-check (blocked calls inflate counter) |
before_tool_call |
| 4 | Warn | Double deprecation warning from module-level wrap() |
wrap() |
| 5 | Warn | combined_input for Cedar/OPA uses different content extraction than pattern scanner |
before_llm_call |
#1: Add if not self._registered: return None guard inside each hook closure.
#2: Coerce with str(tool_result) before scanning, matching before_tool_call pattern.
#3-5 are fine as follow-ups.
imran-siddique
left a comment
There was a problem hiding this comment.
Approving native hooks migration.
…microsoft#1588) Replace fragile proxy-based monkey-patching with CrewAI's native execution hooks (@before_tool_call, @after_tool_call, @before_llm_call, @after_llm_call) introduced in CrewAI 0.80+. Changes: - Add GovernanceHooks class that registers four global governance hooks for tool allowlist, blocked-pattern scanning, Cedar/OPA pre_execute gates, and output content filtering - Add CrewAIKernel.as_hooks() factory method as the recommended integration path - Deprecate CrewAIKernel.wrap() with DeprecationWarning pointing to as_hooks() - Export CrewAIGovernanceHooks from integrations package - Add 43 new tests covering all four hook types, deprecation warnings, Cedar evaluator integration, and backward compatibility - All 12 existing CrewAI regression tests pass unchanged Resolves microsoft#1576 Co-authored-by: Nishar <you@example.com>
Summary
Replaces fragile proxy-based monkey-patching in the CrewAI adapter with CrewAI's native execution hooks (
@before_tool_call,@after_tool_call,@before_llm_call,@after_llm_call) introduced in CrewAI 0.80+.Resolves #1587
Changes
New:
GovernanceHooksclassRegisters four global governance hooks that intercept every tool and LLM call across all agents:
before_tool_callpre_executegate, max call countafter_tool_callpost_executedrift detectionbefore_llm_callpre_executegateafter_llm_callpost_executedrift detectionNew:
CrewAIKernel.as_hooks()factoryDeprecated:
wrap()and module-levelwrap()Both now emit
DeprecationWarningpointing toas_hooks(). Full backward compatibility maintained — all 12 existing regression tests pass unchanged.Export
CrewAIGovernanceHooksexported fromagent_os.integrations.Testing
crewai.hooksmodule since CrewAI is not installed in CIDesign Decisions
isinstance()checks and composabilityPolicyViolationErrortry/exceptimport ensures the adapter works even whencrewai.hooksis unavailable (falls back to legacywrap())as_hooks()mirrors ADK'sas_plugin()and OpenAI'sas_hooks()for consistent API surfaceRelated
BasePluginintegration:google_adk_adapter.pyRunHooksrefactor: OpenAI Agents SDK adapter: use nativeRunHooksinstead ofwrap()workaround #1576AgentMiddlewarerefactor: fix(ci): make publish workflow green by fixing ESRP stubs and pip hash syntax #1577