From a860789dfd44466b667f54c868696994ad1e05c1 Mon Sep 17 00:00:00 2001 From: Lucas Gomide Date: Mon, 13 Jul 2026 15:38:16 -0300 Subject: [PATCH] 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. --- lib/crewai/tests/hooks/test_llm_hooks.py | 54 ++++++++++++++++++ lib/crewai/tests/hooks/test_tool_hooks.py | 69 +++++++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/lib/crewai/tests/hooks/test_llm_hooks.py b/lib/crewai/tests/hooks/test_llm_hooks.py index 4b1a75403..bd09afcf3 100644 --- a/lib/crewai/tests/hooks/test_llm_hooks.py +++ b/lib/crewai/tests/hooks/test_llm_hooks.py @@ -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.""" diff --git a/lib/crewai/tests/hooks/test_tool_hooks.py b/lib/crewai/tests/hooks/test_tool_hooks.py index 14c6848a8..4978f14b8 100644 --- a/lib/crewai/tests/hooks/test_tool_hooks.py +++ b/lib/crewai/tests/hooks/test_tool_hooks.py @@ -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)."""