feat(hooks): add optional agent-hooks governance engine - #6710
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an optional agent-hooks governance engine with fail-closed interception across crewAI execution, tool calls, and model calls. It adds correlation metadata, transformed-input cache ordering, lifecycle controls, bounded audit records, documentation, and comprehensive tests. ChangesAgent Hooks Governance
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 438dc75b8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/utilities/agent_utils.py (1)
2045-2054: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestrict strict Pydantic reparsing to governed post-model hooks.
This block runs for every registered
after_llm_callhook chain, not only the optional agent-hooks governance adapter. A generic hook returning a modified string will now hard-fail on pydantic response_model output, changing the generalafter_llm_callhook contract. Scope this behavior to governance hooks or document/emit the breaking contract clearly for all hook consumers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/agent_utils.py` around lines 2045 - 2054, Restrict the strict Pydantic reparsing in the hook-modified response path to the governed post-model hooks handled by the agent-hooks governance adapter, rather than every after_llm_call hook chain. Preserve the existing generic hook contract by bypassing this model_validate_json validation for non-governance hooks, using the surrounding hook identification or governance adapter symbol to distinguish the paths.
🧹 Nitpick comments (1)
lib/crewai/tests/tools/test_tool_usage.py (1)
278-294: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider an autouse fixture for global hook cleanup.
Both tests hand-roll
clear_*+ try/finally, and each clears only the hook type it registers, so a hook leaked by another module (or by a failure betweenregister_*andtry) still bleeds in. A small autouse fixture clearing both registries before and after each test would make this suite order-independent.Also applies to: 326-340
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/tools/test_tool_usage.py` around lines 278 - 294, Replace the duplicated manual cleanup around the synchronous and asynchronous tool execution tests with an autouse fixture that clears both before-tool-call and after-tool-call hook registries before and after every test. Remove the local clear/try/finally blocks while preserving the existing test execution paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/edge/en/learn/agent-hooks.mdx`:
- Line 84: Update docs/edge/en/learn/agent-hooks.mdx lines 84-84 to call
AgentHooksEngine.records as engine.records() when printing the auditable
records; update lines 173-175 to document engine.records() as the non-draining
accessor.
In `@lib/crewai/src/crewai/agents/crew_agent_executor.py`:
- Around line 1023-1035: Update the after-hook governance context in
_execute_single_native_tool_call and execute_single_native_tool_call so is_error
also becomes true for unresolved, non-cached tool calls where output_tool is
None. Apply this in both lib/crewai/src/crewai/agents/crew_agent_executor.py
lines 1023-1035 and lib/crewai/src/crewai/utilities/agent_utils.py lines
1730-1742, preserving each method’s existing error flags.
In `@lib/crewai/src/crewai/experimental/agent_executor.py`:
- Around line 2056-2058: Update the post-tool governance record’s is_error
calculation near call_id and was_blocked to also treat the output_tool is
None/tool-not-found path as an error. Preserve the existing hook_blocked,
max_usage_reached, and error_event_emitted conditions while ensuring a missing
tool cannot be recorded as successful.
In `@lib/crewai/src/crewai/hooks/agent_hooks_engine.py`:
- Around line 610-641: Update _active_sessions bookkeeping in
_current_session_id, _begin_session, and _finish_session to retain and validate
the owner object rather than relying solely on id(owner). Use weak references or
an equivalent identity check so garbage-collected owners cannot cause recycled
addresses to inherit stale sessions or builders, while preserving current
session stack behavior.
In `@lib/crewai/src/crewai/llm.py`:
- Around line 1768-1771: Ensure argument parsing and dictionary validation in
the tool-call flow completes before the region guarded by the before/after hook
lifecycle, or track whether `run_before_tool_call_hooks` actually ran and only
invoke the corresponding after-hooks when it did. Update both affected paths
around `tool_call.function.arguments` so parse failures cannot emit an orphan
`post_tool_call` for the call_id.
In `@lib/crewai/src/crewai/utilities/agent_utils.py`:
- Around line 1877-1896: Replace the string-type check on aborted.source in the
HookAborted handler with a dedicated sentinel marker owned by AgentHooksEngine,
and recognize only that marker as an agent-hooks abort. Preserve the existing
generic message and False return for all other sources, including custom string
sources, while retaining reason printing and ValueError propagation for the
sentinel case.
---
Outside diff comments:
In `@lib/crewai/src/crewai/utilities/agent_utils.py`:
- Around line 2045-2054: Restrict the strict Pydantic reparsing in the
hook-modified response path to the governed post-model hooks handled by the
agent-hooks governance adapter, rather than every after_llm_call hook chain.
Preserve the existing generic hook contract by bypassing this
model_validate_json validation for non-governance hooks, using the surrounding
hook identification or governance adapter symbol to distinguish the paths.
---
Nitpick comments:
In `@lib/crewai/tests/tools/test_tool_usage.py`:
- Around line 278-294: Replace the duplicated manual cleanup around the
synchronous and asynchronous tool execution tests with an autouse fixture that
clears both before-tool-call and after-tool-call hook registries before and
after every test. Remove the local clear/try/finally blocks while preserving the
existing test execution paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d2337f25-444a-4f06-99ea-e1685872d7ca
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
docs/docs.jsondocs/edge/en/learn/agent-hooks.mdxlib/crewai/pyproject.tomllib/crewai/src/crewai/agents/crew_agent_executor.pylib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/src/crewai/hooks/__init__.pylib/crewai/src/crewai/hooks/agent_hooks_engine.pylib/crewai/src/crewai/hooks/dispatch.pylib/crewai/src/crewai/hooks/llm_hooks.pylib/crewai/src/crewai/hooks/tool_hooks.pylib/crewai/src/crewai/llm.pylib/crewai/src/crewai/llms/base_llm.pylib/crewai/src/crewai/tools/tool_usage.pylib/crewai/src/crewai/utilities/agent_utils.pylib/crewai/src/crewai/utilities/tool_utils.pylib/crewai/tests/agents/test_native_tool_calling.pylib/crewai/tests/hooks/test_agent_hooks_engine.pylib/crewai/tests/tools/test_tool_usage.pylib/crewai/tests/utilities/test_agent_utils.py
438dc75 to
113a2bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/crewai/src/crewai/llms/base_llm.py (1)
1107-1134: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale docstring example after dropping the
strgate.The example still shows
if from_agent is None and isinstance(result, str):, but the method now accepts and governs any response type. Update it so providers don't reintroduce the gate that would skip POST_MODEL_CALL governance for structured responses.📝 Proposed docstring fix
Example: >>> # In a native provider's call() method: - >>> if from_agent is None and isinstance(result, str): - ... result = self._invoke_after_llm_call_hooks( - ... messages, result, from_agent - ... ) + >>> result = self._invoke_after_llm_call_hooks(messages, result, from_agent)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/llms/base_llm.py` around lines 1107 - 1134, The docstring example for _invoke_after_llm_call_hooks still conditionally invokes the hook only for string responses. Update the example to gate solely on from_agent being None and invoke the method for any response type, preserving structured-response governance.Source: Coding guidelines
lib/crewai/src/crewai/utilities/agent_utils.py (1)
1913-1922: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the new
request_idparameter.Both
Args:blocks now omitrequest_id, which is the correlation identifier the whole governance audit trail keys on.📝 Proposed docstring additions
Args: executor_context: The executor context to setup the hooks for. printer: Printer instance for error logging. + request_id: Correlation identifier shared by this model call's pre/post hooks. verbose: Whether to print output.Args: executor_context: The executor context to setup the hooks for. answer: The LLM response (string or Pydantic model). printer: Printer instance for error logging. + request_id: Correlation identifier shared by this model call's pre/post hooks. verbose: Whether to print output.As per coding guidelines: "Document public APIs and complex logic in Python code."
Also applies to: 2005-2015
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/agent_utils.py` around lines 1913 - 1922, Update the docstrings for the affected hook setup/invocation methods around the executor context to include the new request_id parameter in their Args sections, describing it as the correlation identifier used by the governance audit trail. Apply this consistently to both documented methods, including the one around lines 2005–2015.Source: Coding guidelines
lib/crewai/src/crewai/llm.py (1)
1843-1885: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve
ToolExecutionFailedErrorin the direct tool-call path.
available_functionscan contain wrappers that callhandle_tool_failure(...), which raises viaToolFailurePolicy.RAISE. The currentexcept Exception as econverts that deliberate stop into a governed result/None; re-raiseToolExecutionFailedErrorunchanged alongsideHookAborted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/llm.py` around lines 1843 - 1885, Update the direct tool-call exception handling to re-raise ToolExecutionFailedError unchanged alongside HookAborted before the general except Exception branch. Keep the existing governed-result and event-emission behavior for other tool execution exceptions.Source: Learnings
🧹 Nitpick comments (3)
lib/crewai/tests/hooks/test_agent_hooks_engine.py (1)
1757-1777: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBound the barrier waits so a failing thread can't hang the other one.
threading.Barrier.wait()without a timeout blocks forever if the sibling thread dies before reaching the barrier; thejoin(timeout=2.0)assertions then fail while a non-daemon thread leaks for the rest of the session.♻️ Proposed change
def run(label: str) -> None: session_id = engine._begin_session(crew=owner) - sessions_started.wait() + sessions_started.wait(timeout=5.0) observed[label] = ( session_id, engine._current_session_id(crew=owner), ) - sessions_observed.wait() + sessions_observed.wait(timeout=5.0) engine._finish_session(crew=owner) - first = threading.Thread(target=run, args=("first",)) - second = threading.Thread(target=run, args=("second",)) + first = threading.Thread(target=run, args=("first",), daemon=True) + second = threading.Thread(target=run, args=("second",), daemon=True)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/hooks/test_agent_hooks_engine.py` around lines 1757 - 1777, Bound both sessions_started.wait() and sessions_observed.wait() in the run helper with a finite timeout so a missing sibling releases the test instead of blocking indefinitely. Preserve the existing thread joins and session cleanup behavior around the run function.lib/crewai/src/crewai/hooks/agent_hooks_engine.py (1)
397-427: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTool-call normalization is duplicated verbatim across the engine and the executor seam. Both sites walk a provider tool-call list, call
extract_tool_call_info,json.loadsstring args, and wrap non-dict parses as{"raw": ...}/{"value": ...}. Since policy decisions depend on this exact shape, the two copies must never drift — extract one helper (e.g.normalize_tool_calls_for_governanceinagent_utils) and call it from both.
lib/crewai/src/crewai/hooks/agent_hooks_engine.py#L397-L427: replace the inline loop in_model_response_partswith a call to the shared helper (it already imports fromagent_utilshere).lib/crewai/src/crewai/utilities/agent_utils.py#L2039-L2061: move this loop into the shared helper and call it from_setup_after_llm_call_hooks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/hooks/agent_hooks_engine.py` around lines 397 - 427, Extract the duplicated tool-call normalization into a shared agent_utils helper, such as normalize_tool_calls_for_governance, preserving the existing extract_tool_call_info, JSON parsing, and argument-wrapping behavior. In lib/crewai/src/crewai/hooks/agent_hooks_engine.py lines 397-427, replace the inline loop in _model_response_parts with the helper call; in lib/crewai/src/crewai/utilities/agent_utils.py lines 2039-2061, move the normalization loop into the helper and have _setup_after_llm_call_hooks call it.lib/crewai/src/crewai/llms/base_llm.py (1)
740-797: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThe governed tool-hook scaffolding is near-identical in two tool-execution seams. Both generate a
call_id, trackbefore_hook_started/post_hook_attempted, build matching before/afterToolCallHookContextobjects, re-raiseHookAborted, and run an error-path after-hook with the samegoverned_result != error_msgreturn rule. Any future contract change has to be applied twice, and a miss silently weakens governance on one path.
lib/crewai/src/crewai/llms/base_llm.py#L740-L797: extract the shared before/after-hook wrapper (e.g. aBaseLLM._governed_tool_invocationhelper or a small context manager) and use it here.lib/crewai/src/crewai/llm.py#L1761-L1822: reuse that helper in_handle_tool_callinstead of the parallel copy, keeping only the LiteLLM-specific argument parsing and event emission local.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/llms/base_llm.py` around lines 740 - 797, The governed tool-hook logic is duplicated across two execution paths. In lib/crewai/src/crewai/llms/base_llm.py:740-797, extract the shared before/after-hook wrapper into a reusable BaseLLM._governed_tool_invocation helper or context manager, preserving call ID tracking, HookAborted propagation, context construction, and error-path behavior; in lib/crewai/src/crewai/llm.py:1761-1822, update _handle_tool_call to reuse that helper while keeping LiteLLM argument parsing and event emission local.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/hooks/agent_hooks_engine.py`:
- Around line 1071-1088: Wrap the POST_MODEL_CALL call to
mark_post_model_blocked inside its own try/except Exception within the
adapter-error handling in guarded, logging any marking failure with
logger.exception. Ensure exceptions from mark_post_model_blocked do not escape
the fail-closed path, so the blocked-result return remains authoritative.
In `@lib/crewai/tests/hooks/test_agent_hooks_engine.py`:
- Around line 404-465: Update the timeout assertions in
test_timeout_cancels_and_releases_admission and
test_cancellation_resistant_timeout_keeps_admission_bounded to expect
concurrent.futures.TimeoutError (or a file-level alias), matching
_EmitterLoop.run’s FutureTimeoutError behavior on Python 3.10 while preserving
the existing admission assertion.
---
Outside diff comments:
In `@lib/crewai/src/crewai/llm.py`:
- Around line 1843-1885: Update the direct tool-call exception handling to
re-raise ToolExecutionFailedError unchanged alongside HookAborted before the
general except Exception branch. Keep the existing governed-result and
event-emission behavior for other tool execution exceptions.
In `@lib/crewai/src/crewai/llms/base_llm.py`:
- Around line 1107-1134: The docstring example for _invoke_after_llm_call_hooks
still conditionally invokes the hook only for string responses. Update the
example to gate solely on from_agent being None and invoke the method for any
response type, preserving structured-response governance.
In `@lib/crewai/src/crewai/utilities/agent_utils.py`:
- Around line 1913-1922: Update the docstrings for the affected hook
setup/invocation methods around the executor context to include the new
request_id parameter in their Args sections, describing it as the correlation
identifier used by the governance audit trail. Apply this consistently to both
documented methods, including the one around lines 2005–2015.
---
Nitpick comments:
In `@lib/crewai/src/crewai/hooks/agent_hooks_engine.py`:
- Around line 397-427: Extract the duplicated tool-call normalization into a
shared agent_utils helper, such as normalize_tool_calls_for_governance,
preserving the existing extract_tool_call_info, JSON parsing, and
argument-wrapping behavior. In lib/crewai/src/crewai/hooks/agent_hooks_engine.py
lines 397-427, replace the inline loop in _model_response_parts with the helper
call; in lib/crewai/src/crewai/utilities/agent_utils.py lines 2039-2061, move
the normalization loop into the helper and have _setup_after_llm_call_hooks call
it.
In `@lib/crewai/src/crewai/llms/base_llm.py`:
- Around line 740-797: The governed tool-hook logic is duplicated across two
execution paths. In lib/crewai/src/crewai/llms/base_llm.py:740-797, extract the
shared before/after-hook wrapper into a reusable
BaseLLM._governed_tool_invocation helper or context manager, preserving call ID
tracking, HookAborted propagation, context construction, and error-path
behavior; in lib/crewai/src/crewai/llm.py:1761-1822, update _handle_tool_call to
reuse that helper while keeping LiteLLM argument parsing and event emission
local.
In `@lib/crewai/tests/hooks/test_agent_hooks_engine.py`:
- Around line 1757-1777: Bound both sessions_started.wait() and
sessions_observed.wait() in the run helper with a finite timeout so a missing
sibling releases the test instead of blocking indefinitely. Preserve the
existing thread joins and session cleanup behavior around the run function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ca52160-6f72-4549-bc6c-45e93cd91b26
📒 Files selected for processing (11)
docs/edge/en/learn/agent-hooks.mdxlib/crewai/src/crewai/agents/crew_agent_executor.pylib/crewai/src/crewai/hooks/agent_hooks_engine.pylib/crewai/src/crewai/hooks/dispatch.pylib/crewai/src/crewai/hooks/llm_hooks.pylib/crewai/src/crewai/llm.pylib/crewai/src/crewai/llms/base_llm.pylib/crewai/src/crewai/utilities/agent_utils.pylib/crewai/tests/agents/test_native_tool_calling.pylib/crewai/tests/hooks/test_agent_hooks_engine.pylib/crewai/tests/hooks/test_llm_hooks.py
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/edge/en/learn/agent-hooks.mdx
- lib/crewai/src/crewai/hooks/dispatch.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/crewai/src/crewai/utilities/agent_utils.py (1)
2039-2079: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve transformations for native tool-call lists.
For list responses,
structured_responseis a newly created dictionary. If the governed hook mutates it in place, the identity check still succeeds and the originalanswerlist is returned, silently discarding the transformation. If the hook replaces the response, this returns the normalized dictionary instead of the provider-native list, potentially bypassing downstream native-tool handling. Reconcile the governed response back into the native list shape, or update the downstream contract, and test both mutation styles.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/agent_utils.py` around lines 2039 - 2079, Update the native tool-call list handling around structured_response and the final return so POST_MODEL_CALL hook transformations are preserved without changing the provider-native list contract. Reconcile both in-place mutations and replaced hook_context.response values back into the native answer list shape, rather than returning the unchanged answer or normalized dictionary incorrectly. Add coverage for both hook mutation styles.lib/crewai/src/crewai/hooks/agent_hooks_engine.py (1)
1270-1277: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
_execution_endleaks the session whenself._builder(...)raises.Unlike
_execution_start(Line 1255), which builds inside thetryso_finish_sessionalways runs on failure,_execution_endcallsself._builder(...)(Line 1272) before thetry. If it raises — e.g._current_session_id's"multiple active agent-hooks sessions lack an execution-local session identifier"RuntimeError, or any other builder-construction failure —_finish_sessionis skipped entirely, leaving a stale entry in_active_sessions/_buildersfor an owner that is still alive. That stale entry can make the ambiguity check misfire on the next run of the same crew/flow, compounding failures over time.🐛 Proposed fix
def _execution_end(self, ctx: ExecutionEndContext) -> None: """Adapt ``EXECUTION_END`` -> ``agent_shutdown``: records the run's end.""" - builder = self._builder(crew=ctx.crew, flow=ctx.flow) reason = "completed" if ctx.status == "completed" else "error" try: + builder = self._builder(crew=ctx.crew, flow=ctx.flow) self._decide(builder.agent_shutdown(reason=reason)) finally: self._finish_session(crew=ctx.crew, flow=ctx.flow)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/hooks/agent_hooks_engine.py` around lines 1270 - 1277, Move the self._builder(crew=ctx.crew, flow=ctx.flow) call inside the try block in _execution_end, keeping _finish_session(crew=ctx.crew, flow=ctx.flow) in the finally block so session cleanup runs even when builder construction raises. Preserve the existing reason selection and agent_shutdown decision flow.
🧹 Nitpick comments (3)
lib/crewai/tests/hooks/test_agent_hooks_engine.py (1)
2017-2027: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert which records survived to actually cover oldest-first eviction.
The test varies
call_idbut never checks eviction order, so a LIFO/random eviction bug would still pass.♻️ Strengthen the assertion
assert len(engine.records) == 2 assert engine.records_dropped == 1 + # oldest-first eviction: "call-1" is the record that was dropped + assert [record.interception_point.value for record in engine.records] == [ + "pre_tool_call", + "pre_tool_call", + ] assert DEFAULT_MAX_RECORDS > 2Prefer asserting the retained
call_ids directly if the record exposes them (e.g. via its context/correlation fields).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/hooks/test_agent_hooks_engine.py` around lines 2017 - 2027, Strengthen test_record_buffer_evicts_oldest_and_counts_drops by asserting the retained records’ call_id values, using each record’s exposed context or correlation field. Verify call-2 and call-3 remain after inserting call-1, call-2, and call-3, while preserving the existing length and records_dropped assertions.lib/crewai/src/crewai/utilities/agent_utils.py (1)
2000-2004: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new
request_idparameter.The docstring documents the other parameters but omits
request_id, which is part of the model-hook correlation contract. Add it toArgsand describe its optional, host-owned semantics.As per coding guidelines, Python code should document public APIs and complex logic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/agent_utils.py` around lines 2000 - 2004, Update the docstring for the function containing the `answer`, `printer`, and `request_id` parameters to add `request_id` under `Args`; describe it as an optional correlation identifier owned and supplied by the host for model-hook correlation, while preserving the existing documentation for the other parameters.Source: Coding guidelines
lib/crewai/src/crewai/hooks/dispatch.py (1)
293-323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new
final_hookparameter.
final_hookwas added to the signature (Line 300) but theArgsdocstring section (Lines 308-315) doesn't mention it, and the deferred-abort/finalization semantics (a regular-hook abort is deferred until the final hook runs) are non-obvious from the code alone.As per coding guidelines, "Document public APIs and complex logic in Python code."
📝 Proposed docstring addition
reducer: Maps each hook's return value onto ``ctx``. Defaults to :func:`_default_reducer` (payload replacement). May raise :class:`HookAborted`. verbose: Whether to print swallowed-hook-error warnings. + final_hook: Optional hook run after ``hooks``, even if ``hooks`` is + empty or an earlier hook in ``hooks`` aborts. If a regular hook + aborts, that abort is deferred until ``final_hook`` completes and + is then re-raised (unless ``final_hook`` itself raises first).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/hooks/dispatch.py` around lines 293 - 323, Update the run_hooks docstring to document the final_hook argument and explain that regular-hook HookAborted failures are deferred until final_hook executes, after which the abort is propagated. Keep the existing parameter and exception documentation intact.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/crewai/src/crewai/hooks/agent_hooks_engine.py`:
- Around line 1270-1277: Move the self._builder(crew=ctx.crew, flow=ctx.flow)
call inside the try block in _execution_end, keeping
_finish_session(crew=ctx.crew, flow=ctx.flow) in the finally block so session
cleanup runs even when builder construction raises. Preserve the existing reason
selection and agent_shutdown decision flow.
In `@lib/crewai/src/crewai/utilities/agent_utils.py`:
- Around line 2039-2079: Update the native tool-call list handling around
structured_response and the final return so POST_MODEL_CALL hook transformations
are preserved without changing the provider-native list contract. Reconcile both
in-place mutations and replaced hook_context.response values back into the
native answer list shape, rather than returning the unchanged answer or
normalized dictionary incorrectly. Add coverage for both hook mutation styles.
---
Nitpick comments:
In `@lib/crewai/src/crewai/hooks/dispatch.py`:
- Around line 293-323: Update the run_hooks docstring to document the final_hook
argument and explain that regular-hook HookAborted failures are deferred until
final_hook executes, after which the abort is propagated. Keep the existing
parameter and exception documentation intact.
In `@lib/crewai/src/crewai/utilities/agent_utils.py`:
- Around line 2000-2004: Update the docstring for the function containing the
`answer`, `printer`, and `request_id` parameters to add `request_id` under
`Args`; describe it as an optional correlation identifier owned and supplied by
the host for model-hook correlation, while preserving the existing documentation
for the other parameters.
In `@lib/crewai/tests/hooks/test_agent_hooks_engine.py`:
- Around line 2017-2027: Strengthen
test_record_buffer_evicts_oldest_and_counts_drops by asserting the retained
records’ call_id values, using each record’s exposed context or correlation
field. Verify call-2 and call-3 remain after inserting call-1, call-2, and
call-3, while preserving the existing length and records_dropped assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d5906841-8628-4135-ac46-8bc7129c9aaf
📒 Files selected for processing (5)
lib/crewai/src/crewai/hooks/agent_hooks_engine.pylib/crewai/src/crewai/hooks/dispatch.pylib/crewai/src/crewai/utilities/agent_utils.pylib/crewai/tests/hooks/test_agent_hooks_engine.pylib/crewai/tests/hooks/test_llm_hooks.py
Delegate crewAI's dispatch lifecycle points to the framework-neutral agent-hooks contract via a dependency-injected governor, so policy engines, content filters, and egress guards written once as agent-hooks Interceptors govern the tool, model, and execution-boundary seams. - hooks/agent_hooks_engine.py: AgentHooksEngine + use_agent_hooks(), lazy optional import, fail-closed emission on a dedicated event loop - hooks/dispatch.py: set/clear/get_governor + governed_hook() injection - utilities/agent_utils.py: govern the executor pre/post model-call seam - hooks/__init__.py: lazy engine exports; docs + tests
…aths The executor model-call seam now appends the same governor via governed_hook(), so PRE_MODEL_CALL / POST_MODEL_CALL are governed on both the dispatch and agent-executor (sync/async) paths. Flip the coverage table to governed, replace the stale partial-coverage warning with an accurate note, and clarify the injection in 'How it works'.
Declare `agent-hooks` as an optional-dependency extra scoped to Linux x86_64 (the only platform with a published wheel) so CI's `uv sync --all-extras` installs it and the governance tests run instead of skipping. Locks agent-hooks-sdk==0.1.0a3 and updates the install docs to `pip install "crewai[agent-hooks]"`.
…ation - Break reference cycles in _json_safe with a <cycle> marker so a cyclic tool input cannot raise RecursionError before the decision (fail-closed). - Wrap every per-point adapter so a context-build error fails closed (raise / blocked-result) instead of escaping dispatch and proceeding ungoverned. - Stamp the acting agent on each emission and derive a stable pre/post tool-call id so the two contexts correlate. - Surface model tool_calls/finish_reason at post_model_call. - Drain pending emissions on _EmitterLoop.close() to release a blocked thread. Add regression tests.
Carry host-owned correlation IDs through model and tool seams, fail closed on invalid transforms, govern async/native/structured response paths, and make session lifecycle records schema-conformant. Pin the optional SDK and add regression coverage for the review findings. Signed-off-by: Prayag Upadhyay <prayag.upd@gmail.com>
Prevent a terminal governance decision from silently degrading into a weaker tool-calling path, and keep block provenance host-owned so untrusted adapter output cannot forge or mask a policy denial. - Introduce PostModelCallBlockedError as a terminal executor result so a governed post-model block cannot downgrade into native or text tool-calling fallback on either the sync or async loop. - Record block provenance in host-owned trusted context via mark_post_model_blocked / raise_if_post_model_blocked, keyed on the hook context in a WeakKeyDictionary; adapter errors fail closed as blocks instead of returning an SDK record. - Bound the agent-hooks emitter loop with an admission queue and a close()/join lifecycle, and track per-owner sessions with weakref cleanup plus a dropped-records counter. - Set post-hook error status when a native tool call cannot be resolved. - Add tests for fail-closed no-fallback behavior and unresolved-tool error status. Signed-off-by: Prayag Upadhyay <prayag.upd@gmail.com>
- Run the agent-hooks governor's EXECUTION_END as a final hook so shutdown and audit finalization still fire when a prior hook aborts; the pending HookAborted is re-raised afterward. - Log host/adapter emission failures with logger.error, not logger.exception, so fail-closed logs stay payload-free (bounded correlation ids only). - Guard mark_post_model_blocked so a provenance-recording failure still returns the blocked result instead of raising. - Restore the original model on a non-governed post-model reparse failure with a warning instead of raising ValueError; governed denies still fail closed via raise_if_post_model_blocked. - Add tests covering each path. Signed-off-by: Prayag Upadhyay <prayag.upd@gmail.com>
Build inside the try so _finish_session runs on failure, matching _execution_start. Add a regression test; document final_hook and request_id. Signed-off-by: Prayag Upadhyay <prayag.upd@gmail.com>
229e681 to
6610681
Compare
1 participant
Summary
Adds
agent-hooksas an optional, framework-neutral control engine for CrewAI. Policies, content filters, approval gates, and egress guards can be registered once and applied across execution, model, and tool lifecycle points.agent-hooks-sdkthrough theagent-hooksextra: See: https://github.com/responsibleai/agent-hooks/blob/main/spec/AGENT-HOOKS-0.1.mdArchitecture
flowchart LR subgraph Runtime["CrewAI runtime"] B["Crew and flow boundaries"] M["Model call paths"] T["Tool call paths"] H["CrewAI hook seams"] N["Global and scoped hooks"] G["Injected governor<br/>authoritative final hook"] end subgraph Engine["Optional agent-hooks control engine"] A["Point adapter<br/>schema-valid context and correlation"] L["Dedicated async emitter loop"] I["Registered interceptors<br/>policy, filter, approval, egress"] V{"Combined verdict"} R["Interception records<br/>buffer or record sink"] end B --> H M --> H T --> H H --> N --> G G --> A --> L --> I --> V V -->|allow| P["Proceed unchanged"] V -->|transform| X["Apply effective target"] V -->|deny, timeout, or error| D["Fail closed"] V --> RHow it works
use_agent_hooks(...)lazily loads the optional SDK, configures the emitter and interceptors, and installs the engine as the process governor.engine.records,take_records(), or a configured record sink.Coverage
The engine maps CrewAI execution start/end, input/output, model pre/post, and tool pre/post points to the eight
agent-hookscontrol points. CrewAIPRE_STEPandPOST_STEPremain native. Direct provider-specific async calls and direct native-provider Pydantic responses are not yet uniformly governed and are documented as exclusions.Validation
uv run mypy lib/with and withoutagent-hooks-sdkinstalleduv run ruff format --checkanduv run ruff check --no-fixon changed sourcesuv lock --checkuv run --package crewai --with "agent-hooks-sdk==0.1.0a3" pytest lib/crewai/tests -q