mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-22 07:15:10 +00:00
fix: honor scoped hooks on direct llm calls and register @on crew methods
Direct agent-less LLM calls short-circuited on the empty global hook list, so hooks registered only for the current `scoped_hooks()` context never ran; the direct-call helpers now defer to `dispatch`, which resolves scoped hooks behind its own no-op fast path. `CrewBase` likewise only scanned the legacy `is_*_hook` markers, so `@on(InterceptionPoint.X)` methods were silently dropped — it now registers them on the dispatcher with filters applied and `self` bound. Also tightens result typing across the tool-call seams so `mypy` stays green.
This commit is contained in:
@@ -1020,7 +1020,9 @@ class CrewAgentExecutor(BaseAgentExecutor):
|
||||
tool_result=result,
|
||||
raw_tool_result=raw_tool_result,
|
||||
)
|
||||
result = run_after_tool_call_hooks(after_hook_context)
|
||||
modified_result = run_after_tool_call_hooks(after_hook_context)
|
||||
if modified_result is not None:
|
||||
result = modified_result
|
||||
|
||||
if not error_event_emitted:
|
||||
crewai_event_bus.emit(
|
||||
|
||||
@@ -2047,7 +2047,9 @@ class AgentExecutor(Flow[AgentExecutorState], BaseAgentExecutor):
|
||||
tool_result=result,
|
||||
raw_tool_result=raw_tool_result,
|
||||
)
|
||||
result = run_after_tool_call_hooks(after_hook_context)
|
||||
modified_result = run_after_tool_call_hooks(after_hook_context)
|
||||
if modified_result is not None:
|
||||
result = modified_result
|
||||
|
||||
if not error_event_emitted:
|
||||
crewai_event_bus.emit(
|
||||
|
||||
@@ -209,7 +209,7 @@ def _source_name(source: Any) -> str | None:
|
||||
if isinstance(source, str):
|
||||
return source
|
||||
name = getattr(source, "__name__", None)
|
||||
if name:
|
||||
if isinstance(name, str):
|
||||
return name
|
||||
return type(source).__name__
|
||||
|
||||
@@ -231,7 +231,7 @@ def _emit_telemetry(
|
||||
_TELEMETRY_SOURCE,
|
||||
event=HookDispatchedEvent(
|
||||
interception_point=point.value,
|
||||
outcome=outcome, # type: ignore[arg-type]
|
||||
outcome=outcome,
|
||||
hook_count=hook_count,
|
||||
duration_ms=duration_ms,
|
||||
abort_reason=abort_reason,
|
||||
|
||||
@@ -154,7 +154,7 @@ def after_tool_call_reducer(context: ToolCallHookContext, result: object) -> boo
|
||||
|
||||
A non-None return replaces the tool result on the context.
|
||||
"""
|
||||
if result is not None:
|
||||
if isinstance(result, str):
|
||||
context.tool_result = result
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -1011,12 +1011,10 @@ class BaseLLM(BaseModel, ABC):
|
||||
from crewai.hooks.llm_hooks import (
|
||||
LLMCallHookContext,
|
||||
before_llm_call_reducer,
|
||||
get_before_llm_call_hooks,
|
||||
)
|
||||
|
||||
if not get_before_llm_call_hooks():
|
||||
return True
|
||||
|
||||
# No early global-list guard: dispatch resolves global + execution-scoped
|
||||
# hooks and has its own no-op fast path, so scoped hooks still run here.
|
||||
hook_context = LLMCallHookContext(
|
||||
executor=None,
|
||||
messages=messages,
|
||||
@@ -1074,12 +1072,10 @@ class BaseLLM(BaseModel, ABC):
|
||||
from crewai.hooks.llm_hooks import (
|
||||
LLMCallHookContext,
|
||||
after_llm_call_reducer,
|
||||
get_after_llm_call_hooks,
|
||||
)
|
||||
|
||||
if not get_after_llm_call_hooks():
|
||||
return response
|
||||
|
||||
# No early global-list guard: dispatch resolves global + execution-scoped
|
||||
# hooks and has its own no-op fast path, so scoped hooks still run here.
|
||||
hook_context = LLMCallHookContext(
|
||||
executor=None,
|
||||
messages=messages,
|
||||
|
||||
@@ -452,7 +452,15 @@ def _register_crew_hooks(instance: CrewInstance, cls: type) -> None:
|
||||
)
|
||||
}
|
||||
|
||||
if not hook_methods:
|
||||
# Methods decorated with @on(InterceptionPoint.X) carry ``_interception_point``
|
||||
# instead of the legacy markers above.
|
||||
on_methods = {
|
||||
name: method
|
||||
for name, method in cls.__dict__.items()
|
||||
if hasattr(method, "_interception_point")
|
||||
}
|
||||
|
||||
if not hook_methods and not on_methods:
|
||||
return
|
||||
|
||||
from crewai.hooks import (
|
||||
@@ -588,6 +596,25 @@ def _register_crew_hooks(instance: CrewInstance, cls: type) -> None:
|
||||
("after_tool_call", after_tool_hook)
|
||||
)
|
||||
|
||||
if on_methods:
|
||||
from crewai.hooks.dispatch import (
|
||||
_wrap_with_filters,
|
||||
register as register_interception_hook,
|
||||
)
|
||||
|
||||
for on_method in on_methods.values():
|
||||
point = on_method._interception_point
|
||||
bound_hook = on_method.__get__(instance, cls)
|
||||
tools_filter = getattr(on_method, "_filter_tools", None)
|
||||
agents_filter = getattr(on_method, "_filter_agents", None)
|
||||
hook = (
|
||||
_wrap_with_filters(bound_hook, agents_filter, tools_filter)
|
||||
if (tools_filter or agents_filter)
|
||||
else bound_hook
|
||||
)
|
||||
register_interception_hook(point, hook)
|
||||
instance._registered_hook_functions.append((point.value, hook))
|
||||
|
||||
instance._hooks_being_registered = False
|
||||
|
||||
|
||||
|
||||
@@ -1580,7 +1580,9 @@ def execute_single_native_tool_call(
|
||||
tool_result=result,
|
||||
raw_tool_result=raw_tool_result,
|
||||
)
|
||||
result = run_after_tool_call_hooks(after_hook_context)
|
||||
modified_result = run_after_tool_call_hooks(after_hook_context)
|
||||
if modified_result is not None:
|
||||
result = modified_result
|
||||
|
||||
if not error_event_emitted:
|
||||
crewai_event_bus.emit(
|
||||
|
||||
@@ -117,7 +117,10 @@ async def aexecute_tool_and_check_finality(
|
||||
raw_tool_result=blocked_message,
|
||||
)
|
||||
modified_result = run_after_tool_call_hooks(blocked_hook_context)
|
||||
return ToolResult(modified_result, False)
|
||||
return ToolResult(
|
||||
modified_result if modified_result is not None else blocked_message,
|
||||
False,
|
||||
)
|
||||
|
||||
tool_result = await tool_usage.ause(tool_calling, agent_action.text)
|
||||
raw_tool_result = tool_usage.get_last_raw_result(tool_result)
|
||||
@@ -135,7 +138,10 @@ async def aexecute_tool_and_check_finality(
|
||||
|
||||
modified_result = run_after_tool_call_hooks(after_hook_context)
|
||||
|
||||
return ToolResult(modified_result, tool.result_as_answer)
|
||||
return ToolResult(
|
||||
modified_result if modified_result is not None else tool_result,
|
||||
tool.result_as_answer,
|
||||
)
|
||||
|
||||
tool_result = I18N_DEFAULT.errors("wrong_tool_name").format(
|
||||
tool=sanitized_tool_name,
|
||||
@@ -233,7 +239,10 @@ def execute_tool_and_check_finality(
|
||||
raw_tool_result=blocked_message,
|
||||
)
|
||||
modified_result = run_after_tool_call_hooks(blocked_hook_context)
|
||||
return ToolResult(modified_result, False)
|
||||
return ToolResult(
|
||||
modified_result if modified_result is not None else blocked_message,
|
||||
False,
|
||||
)
|
||||
|
||||
tool_result = tool_usage.use(tool_calling, agent_action.text)
|
||||
raw_tool_result = tool_usage.get_last_raw_result(tool_result)
|
||||
@@ -251,7 +260,10 @@ def execute_tool_and_check_finality(
|
||||
|
||||
modified_result = run_after_tool_call_hooks(after_hook_context)
|
||||
|
||||
return ToolResult(modified_result, tool.result_as_answer)
|
||||
return ToolResult(
|
||||
modified_result if modified_result is not None else tool_result,
|
||||
tool.result_as_answer,
|
||||
)
|
||||
|
||||
tool_result = I18N_DEFAULT.errors("wrong_tool_name").format(
|
||||
tool=sanitized_tool_name,
|
||||
|
||||
@@ -306,6 +306,62 @@ class TestCrewScopedHooks:
|
||||
assert len(execution_log) == 1
|
||||
|
||||
|
||||
class TestCrewOnDecoratedMethods:
|
||||
"""@on(InterceptionPoint.X) methods inside @CrewBase must register.
|
||||
|
||||
Regression: CrewBase only scanned the legacy ``is_*_hook`` markers, so
|
||||
methods decorated with the generic ``@on`` decorator (which sets
|
||||
``_interception_point``) were silently dropped and never ran.
|
||||
"""
|
||||
|
||||
def test_on_decorated_method_registers_and_binds_self(self):
|
||||
from crewai.hooks import InterceptionPoint, on
|
||||
from crewai.hooks.dispatch import _resolve_hooks
|
||||
|
||||
execution_log = []
|
||||
|
||||
@CrewBase
|
||||
class TestCrew:
|
||||
def __init__(self):
|
||||
self.name = "on-crew"
|
||||
|
||||
@on(InterceptionPoint.PRE_MODEL_CALL)
|
||||
def on_pre_model(self, context):
|
||||
execution_log.append(self.name)
|
||||
|
||||
@agent
|
||||
def researcher(self):
|
||||
return Agent(role="Researcher", goal="Research", backstory="Expert")
|
||||
|
||||
@crew
|
||||
def crew(self):
|
||||
return Crew(agents=self.agents, tasks=[], verbose=False)
|
||||
|
||||
before = len(_resolve_hooks(InterceptionPoint.PRE_MODEL_CALL))
|
||||
|
||||
instance = TestCrew()
|
||||
|
||||
hooks = _resolve_hooks(InterceptionPoint.PRE_MODEL_CALL)
|
||||
assert len(hooks) == before + 1
|
||||
|
||||
assert (
|
||||
InterceptionPoint.PRE_MODEL_CALL.value,
|
||||
hooks[-1],
|
||||
) in instance._registered_hook_functions
|
||||
|
||||
mock_executor = Mock()
|
||||
mock_executor.messages = []
|
||||
mock_executor.agent = Mock(role="Test")
|
||||
mock_executor.task = Mock()
|
||||
mock_executor.crew = Mock()
|
||||
mock_executor.llm = Mock()
|
||||
mock_executor.iterations = 0
|
||||
|
||||
hooks[-1](LLMCallHookContext(executor=mock_executor))
|
||||
|
||||
assert execution_log == ["on-crew"]
|
||||
|
||||
|
||||
class TestCrewScopedHookAttributes:
|
||||
"""Test that crew-scoped hooks have correct attributes set."""
|
||||
|
||||
|
||||
@@ -463,3 +463,77 @@ class TestLLMHooksIntegration:
|
||||
finally:
|
||||
unregister_before_llm_call_hook(before_hook)
|
||||
unregister_after_llm_call_hook(after_hook)
|
||||
|
||||
|
||||
class TestDirectLLMScopedHooks:
|
||||
"""Direct (agent-less) LLM calls must honor execution-scoped hooks.
|
||||
|
||||
Regression: the direct-call helpers used to short-circuit when the global
|
||||
hook list was empty, so hooks registered only for the current
|
||||
``scoped_hooks()`` context never ran on this path.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _stub_llm():
|
||||
from crewai.llms.base_llm import BaseLLM
|
||||
|
||||
class _StubLLM(BaseLLM):
|
||||
def call(self, *args: object, **kwargs: object) -> str:
|
||||
return ""
|
||||
|
||||
return _StubLLM(model="stub")
|
||||
|
||||
def test_scoped_before_hook_runs_on_direct_call(self):
|
||||
from crewai.hooks import InterceptionPoint
|
||||
from crewai.hooks.dispatch import register_scoped, scoped_hooks
|
||||
|
||||
llm = self._stub_llm()
|
||||
seen: list[int] = []
|
||||
|
||||
with scoped_hooks():
|
||||
register_scoped(
|
||||
InterceptionPoint.PRE_MODEL_CALL,
|
||||
lambda ctx: seen.append(len(ctx.messages)),
|
||||
)
|
||||
proceed = llm._invoke_before_llm_call_hooks(
|
||||
[{"role": "user", "content": "hi"}], from_agent=None
|
||||
)
|
||||
|
||||
assert proceed is True
|
||||
assert seen == [1]
|
||||
|
||||
def test_scoped_before_hook_can_block_direct_call(self):
|
||||
from crewai.hooks import InterceptionPoint
|
||||
from crewai.hooks.dispatch import HookAborted, register_scoped, scoped_hooks
|
||||
|
||||
llm = self._stub_llm()
|
||||
|
||||
def block(ctx: LLMCallHookContext) -> None:
|
||||
raise HookAborted(reason="blocked by scoped hook")
|
||||
|
||||
with scoped_hooks():
|
||||
register_scoped(InterceptionPoint.PRE_MODEL_CALL, block)
|
||||
proceed = llm._invoke_before_llm_call_hooks(
|
||||
[{"role": "user", "content": "hi"}], from_agent=None
|
||||
)
|
||||
|
||||
assert proceed is False
|
||||
|
||||
def test_scoped_after_hook_modifies_direct_response(self):
|
||||
from crewai.hooks import InterceptionPoint
|
||||
from crewai.hooks.dispatch import register_scoped, scoped_hooks
|
||||
|
||||
llm = self._stub_llm()
|
||||
|
||||
def redact(ctx: LLMCallHookContext) -> str:
|
||||
return ctx.response.replace("SECRET", "[REDACTED]")
|
||||
|
||||
with scoped_hooks():
|
||||
register_scoped(InterceptionPoint.POST_MODEL_CALL, redact)
|
||||
result = llm._invoke_after_llm_call_hooks(
|
||||
[{"role": "user", "content": "hi"}],
|
||||
"contains SECRET",
|
||||
from_agent=None,
|
||||
)
|
||||
|
||||
assert result == "contains [REDACTED]"
|
||||
|
||||
Reference in New Issue
Block a user