mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-24 08:15:10 +00:00
fix: run execution-scoped hooks on the agent executor model seams
`_setup_before/after_llm_call_hooks` only ran the executor's snapshot hook lists, so hooks registered via `scoped_hooks()` never fired on `PRE/POST_MODEL_CALL` during normal agent execution, while the tool seams (which go through `dispatch`) merged them. The seams now append the current scope's hooks after the snapshot via `get_scoped_hooks`, matching dispatch's global-then-scoped ordering, and a scoped-only registration no longer short-circuits the seam.
This commit is contained in:
@@ -165,6 +165,20 @@ def register_scoped(point: InterceptionPoint, hook: HookFn) -> None:
|
||||
scope.setdefault(point, []).append(hook)
|
||||
|
||||
|
||||
def get_scoped_hooks(point: InterceptionPoint) -> list[HookFn]:
|
||||
"""Return the hooks registered in the current execution scope for a point.
|
||||
|
||||
Used by seams that carry a pre-snapshotted hook list (e.g. the agent
|
||||
executors' per-executor LLM hook lists) so they can merge in
|
||||
execution-scoped hooks with the same snapshot-then-scoped ordering that
|
||||
:func:`dispatch` applies to global vs scoped hooks.
|
||||
"""
|
||||
scope = _scoped_hooks_var.get()
|
||||
if not scope:
|
||||
return []
|
||||
return list(scope.get(point, []))
|
||||
|
||||
|
||||
def _resolve_hooks(point: InterceptionPoint) -> list[HookFn]:
|
||||
"""Resolve the ordered hooks for a point: global first, then scoped."""
|
||||
global_hooks = _global_hooks[point]
|
||||
|
||||
@@ -1678,14 +1678,24 @@ def _setup_before_llm_call_hooks(
|
||||
Returns:
|
||||
True if LLM execution should proceed, False if blocked by a hook.
|
||||
"""
|
||||
if executor_context and executor_context.before_llm_call_hooks:
|
||||
if executor_context:
|
||||
from crewai.hooks.dispatch import (
|
||||
HookAborted,
|
||||
InterceptionPoint,
|
||||
get_scoped_hooks,
|
||||
run_hooks,
|
||||
)
|
||||
from crewai.hooks.llm_hooks import LLMCallHookContext, before_llm_call_reducer
|
||||
|
||||
# Executor snapshot first, then execution-scoped hooks — the same
|
||||
# ordering dispatch() applies to global vs scoped hooks.
|
||||
hooks: list[Any] = [
|
||||
*executor_context.before_llm_call_hooks,
|
||||
*get_scoped_hooks(InterceptionPoint.PRE_MODEL_CALL),
|
||||
]
|
||||
if not hooks:
|
||||
return True
|
||||
|
||||
original_messages = executor_context.messages
|
||||
|
||||
hook_context = LLMCallHookContext(executor_context)
|
||||
@@ -1693,7 +1703,7 @@ def _setup_before_llm_call_hooks(
|
||||
run_hooks(
|
||||
InterceptionPoint.PRE_MODEL_CALL,
|
||||
hook_context,
|
||||
executor_context.before_llm_call_hooks,
|
||||
hooks,
|
||||
reducer=before_llm_call_reducer,
|
||||
verbose=verbose,
|
||||
)
|
||||
@@ -1740,10 +1750,19 @@ def _setup_after_llm_call_hooks(
|
||||
Returns:
|
||||
The potentially modified response (string or Pydantic model).
|
||||
"""
|
||||
if executor_context and executor_context.after_llm_call_hooks:
|
||||
from crewai.hooks.dispatch import InterceptionPoint, run_hooks
|
||||
if executor_context:
|
||||
from crewai.hooks.dispatch import InterceptionPoint, get_scoped_hooks, run_hooks
|
||||
from crewai.hooks.llm_hooks import LLMCallHookContext, after_llm_call_reducer
|
||||
|
||||
# Executor snapshot first, then execution-scoped hooks — the same
|
||||
# ordering dispatch() applies to global vs scoped hooks.
|
||||
hooks: list[Any] = [
|
||||
*executor_context.after_llm_call_hooks,
|
||||
*get_scoped_hooks(InterceptionPoint.POST_MODEL_CALL),
|
||||
]
|
||||
if not hooks:
|
||||
return answer
|
||||
|
||||
original_messages = executor_context.messages
|
||||
|
||||
if isinstance(answer, BaseModel):
|
||||
@@ -1758,7 +1777,7 @@ def _setup_after_llm_call_hooks(
|
||||
run_hooks(
|
||||
InterceptionPoint.POST_MODEL_CALL,
|
||||
hook_context,
|
||||
executor_context.after_llm_call_hooks,
|
||||
hooks,
|
||||
reducer=after_llm_call_reducer,
|
||||
verbose=verbose,
|
||||
)
|
||||
|
||||
@@ -333,6 +333,51 @@ class TestLLMHooksIntegration:
|
||||
assert ran == ["crashing", "later"]
|
||||
assert proceed is True
|
||||
|
||||
def test_scoped_hooks_fire_on_agent_executor_llm_seams(self, mock_executor):
|
||||
"""register_scoped hooks must run on the executor model seams.
|
||||
|
||||
Regression: `_setup_before/after_llm_call_hooks` only ran the
|
||||
executor's snapshot lists, so execution-scoped hooks never fired on
|
||||
PRE/POST_MODEL_CALL during normal agent execution (while tool seams,
|
||||
which go through `dispatch`, merged them). Scoped hooks run after the
|
||||
snapshot, matching dispatch's global-then-scoped ordering.
|
||||
"""
|
||||
from crewai.hooks import InterceptionPoint
|
||||
from crewai.hooks.dispatch import register_scoped, scoped_hooks
|
||||
from crewai.utilities.agent_utils import (
|
||||
_setup_after_llm_call_hooks,
|
||||
_setup_before_llm_call_hooks,
|
||||
)
|
||||
|
||||
order: list[str] = []
|
||||
|
||||
def snapshot_hook(context):
|
||||
order.append("snapshot")
|
||||
|
||||
mock_executor.before_llm_call_hooks = [snapshot_hook]
|
||||
mock_executor.after_llm_call_hooks = []
|
||||
|
||||
with scoped_hooks():
|
||||
register_scoped(
|
||||
InterceptionPoint.PRE_MODEL_CALL,
|
||||
lambda ctx: order.append("scoped_pre"),
|
||||
)
|
||||
register_scoped(
|
||||
InterceptionPoint.POST_MODEL_CALL,
|
||||
lambda ctx: order.append("scoped_post"),
|
||||
)
|
||||
|
||||
proceed = _setup_before_llm_call_hooks(
|
||||
mock_executor, printer=Mock(), verbose=False
|
||||
)
|
||||
answer = _setup_after_llm_call_hooks(
|
||||
mock_executor, "answer", printer=Mock(), verbose=False
|
||||
)
|
||||
|
||||
assert order == ["snapshot", "scoped_pre", "scoped_post"]
|
||||
assert proceed is True
|
||||
assert answer == "answer"
|
||||
|
||||
def test_intentional_block_still_short_circuits_later_hooks(self, mock_executor):
|
||||
"""A hook returning False blocks the call and skips later hooks (unchanged)."""
|
||||
from crewai.utilities.agent_utils import _setup_before_llm_call_hooks
|
||||
|
||||
Reference in New Issue
Block a user