mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-07-22 07:15:10 +00:00
test: pin per-hook fail-open at the LLM and tool seams
The dispatcher swallows a hook's exception per hook rather than around the whole loop, so one buggy hook no longer silently skips every hook registered after it. These seam-level tests pin that behavior through `_setup_before_llm_call_hooks` and `run_before/after_tool_call_hooks`, and confirm an intentional `return False` block still short-circuits later hooks.
This commit is contained in:
@@ -303,6 +303,60 @@ class TestLLMHooksIntegration:
|
||||
hooks = get_before_llm_call_hooks()
|
||||
assert len(hooks) == 0
|
||||
|
||||
def test_raising_before_hook_does_not_skip_later_hooks(self, mock_executor):
|
||||
"""Fail-open is per-hook: a crashing hook must not disable its neighbors.
|
||||
|
||||
Regression guard for the dispatcher migration: previously the
|
||||
``except Exception`` wrapped the whole hook loop, so a raising hook
|
||||
silently skipped every hook registered after it. Now swallowing is
|
||||
per-hook — later hooks still run and the LLM call still proceeds.
|
||||
"""
|
||||
from crewai.utilities.agent_utils import _setup_before_llm_call_hooks
|
||||
|
||||
ran: list[str] = []
|
||||
|
||||
def crashing_hook(context):
|
||||
ran.append("crashing")
|
||||
raise ValueError("bug in user hook")
|
||||
|
||||
def later_hook(context):
|
||||
ran.append("later")
|
||||
|
||||
register_before_llm_call_hook(crashing_hook)
|
||||
register_before_llm_call_hook(later_hook)
|
||||
mock_executor.before_llm_call_hooks = get_before_llm_call_hooks()
|
||||
|
||||
proceed = _setup_before_llm_call_hooks(
|
||||
mock_executor, printer=Mock(), verbose=False
|
||||
)
|
||||
|
||||
assert ran == ["crashing", "later"]
|
||||
assert proceed is True
|
||||
|
||||
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
|
||||
|
||||
ran: list[str] = []
|
||||
|
||||
def blocking_hook(context):
|
||||
ran.append("blocking")
|
||||
return False
|
||||
|
||||
def later_hook(context):
|
||||
ran.append("later")
|
||||
|
||||
register_before_llm_call_hook(blocking_hook)
|
||||
register_before_llm_call_hook(later_hook)
|
||||
mock_executor.before_llm_call_hooks = get_before_llm_call_hooks()
|
||||
|
||||
proceed = _setup_before_llm_call_hooks(
|
||||
mock_executor, printer=Mock(), verbose=False
|
||||
)
|
||||
|
||||
assert ran == ["blocking"]
|
||||
assert proceed is False
|
||||
|
||||
@pytest.mark.vcr()
|
||||
def test_lite_agent_hooks_integration_with_real_llm(self):
|
||||
"""Test that LiteAgent executes before/after LLM call hooks and prints messages correctly."""
|
||||
|
||||
@@ -576,6 +576,75 @@ class TestToolHooksIntegration:
|
||||
unregister_after_tool_call_hook(after_tool_call_hook)
|
||||
|
||||
|
||||
class TestPerHookFailOpen:
|
||||
"""Fail-open is per-hook: a crashing hook must not disable its neighbors.
|
||||
|
||||
Regression guards for the dispatcher migration: previously each seam's
|
||||
``except Exception`` wrapped the whole hook loop, so a raising hook
|
||||
silently skipped every hook registered after it.
|
||||
"""
|
||||
|
||||
def test_raising_before_hook_does_not_skip_later_hooks_or_block(
|
||||
self, mock_tool, mock_agent
|
||||
):
|
||||
from crewai.hooks.tool_hooks import run_before_tool_call_hooks
|
||||
|
||||
mock_agent.verbose = False
|
||||
ran: list[str] = []
|
||||
|
||||
def crashing_hook(context):
|
||||
ran.append("crashing")
|
||||
raise ValueError("bug in user hook")
|
||||
|
||||
def later_hook(context):
|
||||
ran.append("later")
|
||||
|
||||
register_before_tool_call_hook(crashing_hook)
|
||||
register_before_tool_call_hook(later_hook)
|
||||
|
||||
context = ToolCallHookContext(
|
||||
tool_name="test_tool",
|
||||
tool_input={"arg": "value"},
|
||||
tool=mock_tool,
|
||||
agent=mock_agent,
|
||||
)
|
||||
blocked = run_before_tool_call_hooks(context)
|
||||
|
||||
assert ran == ["crashing", "later"]
|
||||
assert blocked is False
|
||||
|
||||
def test_raising_after_hook_does_not_skip_later_result_rewrites(
|
||||
self, mock_tool, mock_agent
|
||||
):
|
||||
from crewai.hooks.tool_hooks import run_after_tool_call_hooks
|
||||
|
||||
mock_agent.verbose = False
|
||||
ran: list[str] = []
|
||||
|
||||
def crashing_hook(context):
|
||||
ran.append("crashing")
|
||||
raise ValueError("bug in user hook")
|
||||
|
||||
def rewriting_hook(context):
|
||||
ran.append("rewriting")
|
||||
return f"{context.tool_result} [rewritten]"
|
||||
|
||||
register_after_tool_call_hook(crashing_hook)
|
||||
register_after_tool_call_hook(rewriting_hook)
|
||||
|
||||
context = ToolCallHookContext(
|
||||
tool_name="test_tool",
|
||||
tool_input={"arg": "value"},
|
||||
tool=mock_tool,
|
||||
agent=mock_agent,
|
||||
tool_result="original",
|
||||
)
|
||||
result = run_after_tool_call_hooks(context)
|
||||
|
||||
assert ran == ["crashing", "rewriting"]
|
||||
assert result == "original [rewritten]"
|
||||
|
||||
|
||||
class TestNativeToolCallingHooksIntegration:
|
||||
"""Integration tests for hooks with native function calling (Agent and Crew)."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user