diff --git a/CHANGELOG.md b/CHANGELOG.md index 1088eb3..456a584 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,22 @@ and this project adheres to [Clever Semantic Versioning](https://www.w3.org/subm ### Added +- **`tool_agent_class` Injection Parameter for `AgentFactory` and `create_executor` (issue #73)** (`agents/factory.py`, `agents/llm.py`, `runtime.py`, `runtime_dispatch.py`, `core/application.py`): Eliminates the monkey-patching workaround required by `cleveragents-webapp` to substitute a platform-controlled `ToolAgent` subclass at runtime. + + `AgentFactory.__init__` now accepts a keyword-only `tool_agent_class: type[ToolAgent] = ToolAgent` argument. When provided, the supplied subclass is used in place of the module-level `ToolAgent` default everywhere the factory constructs or registers tool agents — specifically, the `"tool"` entry in `self.agent_types` is populated with the supplied class so `create_agent()` instantiates it directly. + + `Executor.__init__` and `create_executor()` accept the same parameter and store it on `self.tool_agent_class`. All six dispatch paths that construct or dispatch tool agents are updated to honour this attribute: `_execute_tool` uses `executor.tool_agent_class(...)` instead of `ToolAgent(...)`; `_execute_graph` and `_execute_graph_stream` forward `tool_agent_class=executor.tool_agent_class` to `AgentFactory` (so graph nodes with tool-typed agents pick up the subclass); `_execute_llm` and `_execute_llm_stream` forward `tool_agent_class=executor.tool_agent_class` to `AgentFactory` (so the single-LLM actor's internal tool-calling loop uses the subclass); and `_execute_multi_actor` forwards `tool_agent_class=executor.tool_agent_class` to the sub-`Executor` (so every actor inside a multi-actor bundle — tool, graph, or LLM — honours the injected subclass). + + `LLMAgent.__init__` accepts the same keyword-only `tool_agent_class` argument (validated as a `ToolAgent` subclass) and `AgentFactory._create_agent_instance` forwards it when creating `"llm"`-typed agents. The multi-turn tool-call loop in `LLMAgent._execute_tool_loop` constructs its ephemeral tool executors via `self._tool_agent_class(...)` (covering the regular loop, the budget-exhaustion synthesis round, and the stuck-model synthesis round), so the LLM tool-calling path no longer falls back to the module-level `ToolAgent`. + + `ReactiveCleverAgentsApp` no longer redundantly re-registers the module-level `ToolAgent` (the factory already owns the `"tool"` registration), and the now-unused `ToolAgent` import was removed from `runtime_dispatch.py` (the dispatch path uses `executor.tool_agent_class`). + + Both `AgentFactory`, `Executor`, and `LLMAgent` validate that the supplied class is a subclass of `ToolAgent` at construction time (fail-fast, per `CONTRIBUTING.md §Argument Validation`). + + **Backward-compatibility:** callers that do not pass `tool_agent_class` receive identical behaviour to the previous release — the default is `ToolAgent` itself. + + **Module:** `src/cleveractors/agents/factory.py` (`AgentFactory.__init__`, `_create_agent_instance`), `src/cleveractors/agents/llm.py` (`LLMAgent.__init__`, `_execute_tool_loop`), `src/cleveractors/runtime.py` (`Executor.__init__`, `create_executor`), `src/cleveractors/runtime_dispatch.py` (`_execute_tool`, `_execute_graph`, `_execute_graph_stream`, `_execute_llm`, `_execute_llm_stream`, `_execute_multi_actor`), `src/cleveractors/core/application.py` (`load_configuration`, `_create_agents`). BDD: scenarios in `features/tool_agent_class_injection.feature`. + - **LLM Agent Token-Budget Awareness and Tool Output Pruning (issue #61, #65)** (`llm.py`): Two complementary mechanisms to prevent context-window exhaustion in the multi-turn tool-call loop, with accurate token tracking for billing. **Token-budget awareness** (`token_budget_percent` config, default off): tracks actual token consumption from LLM response metadata before each invocation. Emits a warning at 75% budget consumption. When the budget ceiling is exceeded, injects a synthesis prompt, permits one final tool-call round, then forces a text-only response. Token counts from all rounds (including budget-exhaustion synthesis, stuck-model synthesis, and pruning passes) are accumulated for accurate billing. diff --git a/features/application_coverage_gaps.feature b/features/application_coverage_gaps.feature index 111be59..7c255a1 100644 --- a/features/application_coverage_gaps.feature +++ b/features/application_coverage_gaps.feature @@ -166,11 +166,6 @@ Feature: Application Error Handling, Stream Routing, and Tool Execution When start_interactive_session encounters a RuntimeError (apg) Then the error should be wrapped in CleverAgentsException with session message (apg) - Scenario: _create_agents registers built-in agent types - Given an app with config and agent_factory (apg) - When _create_agents is called (apg) - Then LLMAgent and ToolAgent types should be registered in the factory (apg) - Scenario: _sanitize_json_string returns already-valid JSON unchanged Given a fresh application gaps test context (apg) When _sanitize_json_string is called with a valid JSON string (apg) diff --git a/features/steps/application_coverage_gaps_steps.py b/features/steps/application_coverage_gaps_steps.py index 375ade3..844acd9 100644 --- a/features/steps/application_coverage_gaps_steps.py +++ b/features/steps/application_coverage_gaps_steps.py @@ -1044,28 +1044,12 @@ def step_apg_assert_session_wrap(context): # ---- _create_agents register built-in types ---- - - -@when("_create_agents is called (apg)") -def step_apg_call_create_agents(context): - app = context.apg_app - factory = app.agent_factory - with ( - patch.object(factory, "register_agent_type") as reg, - patch.object(factory, "get_agent_types", return_value=[]), - ): - app._create_agents() - context.apg_results["registered_calls"] = [ - c[0][0] for c in reg.call_args_list if c[0] - ] - - -@then("LLMAgent and ToolAgent types should be registered in the factory (apg)") -def step_apg_assert_types_registered(context): - calls = context.apg_results.get("registered_calls", []) - assert "llm" in calls - assert "tool" in calls - +# The former "_create_agents registers built-in agent types" scenario and its +# step definitions were removed: ``_create_agents`` no longer redundantly +# re-registers the built-in agent types (the factory's ``__init__`` owns that +# registration, issue #73). ``_create_agents`` itself is still exercised +# end-to-end by every scenario that loads a configuration via +# ``load_configuration``. # ---- _sanitize_json_string fast path ---- diff --git a/features/steps/credential_executor_paths_steps.py b/features/steps/credential_executor_paths_steps.py index 9409d0a..50e9d62 100644 --- a/features/steps/credential_executor_paths_steps.py +++ b/features/steps/credential_executor_paths_steps.py @@ -79,6 +79,8 @@ async def step_execute_multi_actor(context: Any, message: str) -> None: credentials: Any, limits: Any, pricing: Any, + tool_agent_class: Any = None, + **_extra_kwargs: Any, ) -> None: context._sub_executor_credentials = copy.deepcopy(credentials) # Patch execute on the sub-executor to avoid real LLM calls @@ -90,7 +92,23 @@ async def step_execute_multi_actor(context: Any, message: str) -> None: nodes=[], ) ) - original_init(self_exec, config_dict, credentials, limits, pricing) + # Forward tool_agent_class (keyword-only on Executor.__init__) so the + # sub-Executor honours the parent's injected subclass; this mirrors the + # real _execute_multi_actor call site, which forwards the attribute + # (issue #73). When the kwarg is absent, fall back to Executor's own + # default (ToolAgent) by omitting it. Other keyword-only args are + # accepted via _extra_kwargs for forward-compatibility. + if tool_agent_class is not None: + original_init( + self_exec, + config_dict, + credentials, + limits, + pricing, + tool_agent_class=tool_agent_class, + ) + else: + original_init(self_exec, config_dict, credentials, limits, pricing) try: with patch.object(Executor, "__init__", capturing_init): diff --git a/features/steps/runtime_coverage_steps.py b/features/steps/runtime_coverage_steps.py index 65c5474..64c7ec7 100644 --- a/features/steps/runtime_coverage_steps.py +++ b/features/steps/runtime_coverage_steps.py @@ -62,7 +62,6 @@ async def step_execute_actor(context: Any, msg: str) -> None: with ( patch("cleveractors.runtime_dispatch.TemplateRenderer") as mock_renderer, - patch("cleveractors.runtime_dispatch.ToolAgent") as mock_tool, patch("cleveractors.agents.llm.LLMAgent") as mock_llm, patch("cleveractors.runtime_dispatch.AgentFactory") as mock_factory, patch("cleveractors.runtime_dispatch.PureLangGraph") as mock_pure_graph, @@ -88,12 +87,6 @@ async def step_execute_actor(context: Any, msg: str) -> None: mock_llm.return_value = mock_llm_instance - mock_tool_instance = MagicMock() - mock_tool_instance.process_message = AsyncMock( - return_value="Mock tool response" - ) - mock_tool.return_value = mock_tool_instance - mock_graph_instance = MagicMock() # PureLangGraph.execute() returns a 3-tuple (AC4): (response, state, node_usages) mock_graph_instance.execute = AsyncMock( diff --git a/features/steps/runtime_extended_coverage_steps.py b/features/steps/runtime_extended_coverage_steps.py index ad2f0fc..91fd0e1 100644 --- a/features/steps/runtime_extended_coverage_steps.py +++ b/features/steps/runtime_extended_coverage_steps.py @@ -10,6 +10,7 @@ from behave import given, then, when from behave.api.async_step import async_run_until_complete from behave.runner import Context +from cleveractors.agents.tool import ToolAgent from cleveractors.core.exceptions import ConfigurationError from cleveractors.runtime import Executor, create_executor @@ -290,10 +291,6 @@ async def step_rxe_execute(context: Context, message: str) -> None: mock_llm_inst.cleanup = AsyncMock() mock_llm_inst._last_token_usage = (100, 50) - mock_tool_inst = MagicMock() - mock_tool_inst.process_message = AsyncMock(return_value="Mock tool response") - mock_tool_inst.cleanup = AsyncMock() - mock_graph_inst = MagicMock() # PureLangGraph.execute() returns a 3-tuple (AC4, issue #14) mock_graph_inst.execute = AsyncMock(return_value=("Mock graph response", {}, [])) @@ -309,14 +306,12 @@ async def step_rxe_execute(context: Context, message: str) -> None: # and check credentials. with ( patch("cleveractors.agents.llm.LLMAgent") as mock_llm, - patch("cleveractors.runtime_dispatch.ToolAgent") as mock_tool, patch("cleveractors.runtime_dispatch.PureLangGraph") as mock_pure_graph, patch( "cleveractors.langgraph.pure_graph.PureGraphConfig" ) as mock_pg_config, ): mock_llm.return_value = mock_llm_inst - mock_tool.return_value = mock_tool_inst mock_pure_graph.return_value = mock_graph_inst mock_pg_config.return_value = MagicMock() @@ -349,7 +344,15 @@ async def step_rxe_execute(context: Context, message: str) -> None: with ( patch("cleveractors.runtime_dispatch.TemplateRenderer") as mock_renderer, - patch("cleveractors.runtime_dispatch.ToolAgent") as mock_tool, + # executor.tool_agent_class is the real ToolAgent class; patch its + # process_message so tool actors return a canned response without + # executing actual tools (issue #73: module-level ToolAgent patch no + # longer intercepts construction via executor.tool_agent_class). + patch.object( + ToolAgent, + "process_message", + AsyncMock(return_value="Mock tool response"), + ), patch("cleveractors.agents.llm.LLMAgent") as mock_llm, patch("cleveractors.runtime_dispatch.AgentFactory") as mock_factory, patch("cleveractors.runtime_dispatch.PureLangGraph") as mock_pure_graph, @@ -358,7 +361,6 @@ async def step_rxe_execute(context: Context, message: str) -> None: mock_renderer_inst = MagicMock() mock_renderer.return_value = mock_renderer_inst mock_llm.return_value = mock_llm_inst - mock_tool.return_value = mock_tool_inst mock_pure_graph.return_value = mock_graph_inst mock_pg_config.return_value = MagicMock() mock_factory.return_value = mock_factory_inst diff --git a/features/steps/tool_agent_class_injection_steps.py b/features/steps/tool_agent_class_injection_steps.py new file mode 100644 index 0000000..fd97299 --- /dev/null +++ b/features/steps/tool_agent_class_injection_steps.py @@ -0,0 +1,720 @@ +"""Step definitions for tool_agent_class injection BDD tests (issue #73). + +Tests that AgentFactory and create_executor accept a ``tool_agent_class`` +keyword argument and use it in place of the module-level ``ToolAgent`` +default for all internal tool-agent construction paths. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import Mock, patch + +from behave import given, then, when # type: ignore[import-untyped] +from behave.api.async_step import ( + async_run_until_complete, # type: ignore[import-untyped] +) +from langchain_core.language_models.chat_models import BaseChatModel +from langchain_core.messages import AIMessage + +from cleveractors.agents.factory import AgentFactory +from cleveractors.agents.tool import ToolAgent +from cleveractors.core.exceptions import ConfigurationError +from cleveractors.runtime import Executor, create_executor +from cleveractors.templates.renderer import TemplateRenderer + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_tool_agent_subclass(name: str = "CustomToolAgent") -> type[ToolAgent]: + """Dynamically create a named ToolAgent subclass for test isolation.""" + return type(name, (ToolAgent,), {}) + + +def _make_minimal_factory_config(agent_name: str = "tool_agent") -> dict[str, Any]: + """Minimal AgentFactory config with a single tool-typed agent.""" + return { + "agents": { + agent_name: { + "type": "tool", + "config": {"tools": ["echo"]}, + } + } + } + + +def _make_valid_executor_config() -> dict[str, Any]: + """Minimal Executor config dict for basic validation tests.""" + return { + "type": "llm", + "provider": "openai", + "model": "gpt-4o-mini", + "system_prompt": "You are a helper.", + } + + +def _make_valid_tool_executor_config() -> dict[str, Any]: + """Minimal Executor config dict for a tool actor.""" + return { + "type": "tool", + "name": "test_tool_actor", + "tools": ["echo"], + } + + +def _make_single_llm_executor_config_with_tools() -> dict[str, Any]: + """Minimal Executor config for a single LLM actor that declares tools. + + The ``config.tools`` entry engages ``LLMAgent._execute_tool_loop`` so the + injected ``tool_agent_class`` is instantiated when the mock chat model + emits a tool call. This config drives the ``_execute_llm`` dispatch path + (issue #73): that path builds an ``AgentFactory`` internally, so the test + verifies the executor forwards ``tool_agent_class`` to the factory and on + into the LLM agent's tool loop. + """ + return { + "type": "llm", + "name": "llm_with_tools", + "provider": "openai", + "model": "gpt-4o-mini", + "system_prompt": "You are a helper.", + "config": { + "tools": [{"name": "echo"}], + }, + } + + +def _make_multi_actor_executor_config_with_tool_sub() -> dict[str, Any]: + """Minimal multi-actor bundle config whose default actor is a tool actor. + + Drives the ``_execute_multi_actor`` dispatch path (issue #73): that path + builds a sub-:class:`Executor` from the selected actor's config, so the + test verifies the parent executor forwards ``tool_agent_class`` to the + sub-executor. The tool sub-actor then runs via ``_execute_tool``, which + constructs the agent through ``executor.tool_agent_class(...)``. + """ + return { + "type": "multi_actor", + "cleveragents": {"default_actor": "tool_sub"}, + "actors": { + "tool_sub": { + "type": "tool", + "name": "tool_sub", + "tools": ["echo"], + }, + }, + } + + +def _mock_ai_message( + text: str, + tool_calls: list[dict[str, Any]] | None = None, +) -> Mock: + """Build a mock AIMessage for the LLMAgent tool-loop test. + + Mirrors the response shape used by ``llm_agent_tool_calling_steps``: + a ``Mock(spec=AIMessage)`` carrying ``content``, ``tool_calls`` and + ``usage_metadata`` so the LLMAgent billing/extraction helpers work. + """ + response = Mock(spec=AIMessage) + response.content = text + response.tool_calls = tool_calls if tool_calls is not None else None + response.usage_metadata = {"input_tokens": 10, "output_tokens": 20} + return response + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("a minimal AgentFactory config with a single tool agent") +def step_minimal_factory_config(context: Any) -> None: + context.factory_agent_name = "tool_agent" + context.factory_config = _make_minimal_factory_config("tool_agent") + + +@given('a minimal AgentFactory config with a single tool agent named "{agent_name}"') +def step_minimal_factory_config_named(context: Any, agent_name: str) -> None: + context.factory_agent_name = agent_name + context.factory_config = _make_minimal_factory_config(agent_name) + + +@given('a custom ToolAgent subclass named "{subclass_name}"') +def step_custom_tool_agent_subclass(context: Any, subclass_name: str) -> None: + context.custom_tool_agent_class = _make_tool_agent_subclass(subclass_name) + + +@given('a custom ToolAgent subclass named "{subclass_name}" that records instantiation') +def step_custom_tool_agent_subclass_recording(context: Any, subclass_name: str) -> None: + context.instantiation_count = 0 + outer = context + + class RecordingToolAgent(ToolAgent): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + outer.instantiation_count += 1 + + RecordingToolAgent.__name__ = subclass_name + RecordingToolAgent.__qualname__ = subclass_name + context.custom_tool_agent_class = RecordingToolAgent + + +@given("a valid Executor config dict") +def step_valid_executor_config(context: Any) -> None: + context.executor_config = _make_valid_executor_config() + + +@given("a valid tool Executor config dict") +def step_valid_tool_executor_config(context: Any) -> None: + context.executor_config = _make_valid_tool_executor_config() + + +@given("a single-llm Executor config with tools") +def step_single_llm_executor_config_with_tools(context: Any) -> None: + context.executor_config = _make_single_llm_executor_config_with_tools() + + +@given("a multi-actor Executor config with a tool sub-actor") +def step_multi_actor_executor_config_with_tool_sub(context: Any) -> None: + context.executor_config = _make_multi_actor_executor_config_with_tool_sub() + + +@given("an AgentFactory config with a single llm agent that has tools") +def step_llm_factory_config_with_tools(context: Any) -> None: + """AgentFactory config with one LLM agent that declares an ``echo`` tool. + + The LLM agent never makes a real provider call: the When step injects a + mock chat model via ``chat_model`` setter, so no API key / LangChain + client construction happens. ``tools`` is what matters — it engages the + multi-turn tool-call loop in ``LLMAgent._execute_tool_loop``. + """ + context.factory_agent_name = "llm_with_tools" + context.factory_config = { + "agents": { + "llm_with_tools": { + "type": "llm", + "config": { + "provider": "openai", + "model": "gpt-4o-mini", + "api_key": "test_key", + "tools": [{"name": "echo"}], + }, + } + } + } + + +# --------------------------------------------------------------------------- +# When steps — AgentFactory +# --------------------------------------------------------------------------- + + +@when("I create an AgentFactory without tool_agent_class") +def step_create_factory_default(context: Any) -> None: + context.factory_error = None + try: + context.factory = AgentFactory( + config=context.factory_config, + template_renderer=TemplateRenderer(), + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.factory_error = exc + + +@when("I create an AgentFactory with that custom tool_agent_class") +def step_create_factory_custom(context: Any) -> None: + context.factory_error = None + try: + context.factory = AgentFactory( + config=context.factory_config, + template_renderer=TemplateRenderer(), + tool_agent_class=context.custom_tool_agent_class, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.factory_error = exc + + +@when("I create an AgentFactory with a non-ToolAgent class as tool_agent_class") +def step_create_factory_invalid_class(context: Any) -> None: + context.factory_error = None + try: + context.factory = AgentFactory( + config=context.factory_config, + template_renderer=TemplateRenderer(), + tool_agent_class=str, # type: ignore[arg-type] # intentionally invalid + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.factory_error = exc + + +@when('I create the agent named "{agent_name}" from the factory') +def step_create_agent_from_factory(context: Any, agent_name: str) -> None: + context.created_agent_error = None + try: + context.created_agent = context.factory.create_agent(agent_name) + except Exception as exc: # pylint: disable=broad-exception-caught + context.created_agent_error = exc + + +# --------------------------------------------------------------------------- +# When steps — Executor / create_executor +# --------------------------------------------------------------------------- + + +@when("I create an Executor without tool_agent_class") +def step_create_executor_default(context: Any) -> None: + context.executor_error = None + try: + context.test_executor = Executor( + config_dict=context.executor_config, + credentials=None, + limits={}, + pricing={}, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.executor_error = exc + + +@when("I create an Executor with that custom tool_agent_class") +def step_create_executor_custom(context: Any) -> None: + context.executor_error = None + try: + context.test_executor = Executor( + config_dict=context.executor_config, + credentials=None, + limits={}, + pricing={}, + tool_agent_class=context.custom_tool_agent_class, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.executor_error = exc + + +@when("I create an Executor with a non-ToolAgent class as tool_agent_class") +def step_create_executor_invalid_class(context: Any) -> None: + context.executor_error = None + try: + context.test_executor = Executor( + config_dict=context.executor_config, + credentials=None, + limits={}, + pricing={}, + tool_agent_class=str, # type: ignore[arg-type] # intentionally invalid + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.executor_error = exc + + +@when("I call create_executor without tool_agent_class") +def step_call_create_executor_no_class(context: Any) -> None: + context.executor_error = None + try: + context.test_executor = create_executor( + config_dict=context.executor_config, + credentials=None, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.executor_error = exc + + +@when("I call create_executor with that custom tool_agent_class") +def step_call_create_executor_custom(context: Any) -> None: + context.executor_error = None + try: + context.test_executor = create_executor( + config_dict=context.executor_config, + credentials=None, + tool_agent_class=context.custom_tool_agent_class, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.executor_error = exc + + +@when("I execute the tool actor using the custom subclass") +@async_run_until_complete +async def step_execute_tool_custom_subclass(context: Any) -> None: + """Execute a tool actor with a custom subclass and capture instantiation. + + The custom subclass (set up in the recording Given step) increments + ``context.instantiation_count`` each time it is constructed. We patch + enough of the tool execution to avoid actual tool invocation while still + exercising the constructor call path in ``_execute_tool``. + """ + executor = create_executor( + config_dict=context.executor_config, + credentials=None, + tool_agent_class=context.custom_tool_agent_class, + ) + + # Patch process_message on the base class so it doesn't actually run tools. + # We use a module-level patch on ToolAgent.process_message (the inherited + # method) so the custom subclass picks it up without needing to be patched + # separately. The helper accepts ``**_kwargs`` (rather than a parameter + # named ``context``) so it never shadows the Behave ``context`` object that + # encloses it, while still absorbing the ``context=`` keyword that the real + # ``_execute_tool`` and the LLM tool-loop callers pass through. + original_process = ToolAgent.process_message + + async def _fake_process(self: Any, message: str, **_kwargs: Any) -> str: + return "echo ok" + + ToolAgent.process_message = _fake_process # type: ignore[method-assign] + try: + context.execute_result = await executor.execute("echo hello") + finally: + ToolAgent.process_message = original_process # type: ignore[method-assign] + + +@when( + "I execute the single-llm Executor with a mock chat model " + "that returns one tool call then text" +) +@async_run_until_complete +async def step_execute_single_llm_executor(context: Any) -> None: + """Execute a single-LLM executor and assert the injected subclass is used. + + Drives the ``_execute_llm`` dispatch path (issue #73). That path builds an + ``AgentFactory`` internally; with the fix it forwards + ``tool_agent_class=executor.tool_agent_class`` so the created ``LLMAgent`` + receives the recording subclass. We patch ``build_chat_model`` to return a + mock chat model that emits one ``echo`` tool call then a plain-text answer, + which engages ``LLMAgent._execute_tool_loop`` — the loop instantiates the + injected subclass when dispatching the tool call, incrementing + ``context.instantiation_count``. ``ToolAgent.process_message`` is patched + so the recording subclass returns a canned string without running real + tools. ``credentials=None`` is safe because ``build_chat_model`` is fully + mocked, so the lazy chat-model init never hits a real provider. + """ + executor = create_executor( + config_dict=context.executor_config, + credentials=None, + tool_agent_class=context.custom_tool_agent_class, + ) + + call_counter = [0] + + async def _mock_ainvoke(messages: Any, **invoke_kwargs: Any) -> Any: + call_counter[0] += 1 + if call_counter[0] == 1: + return _mock_ai_message( + "Thinking...", + tool_calls=[ + { + "id": "call_exec_llm_1", + "type": "function", + "function": { + "name": "echo", + "arguments": '{"text": "hello"}', + }, + } + ], + ) + return _mock_ai_message("Final answer", tool_calls=None) + + mock_model = Mock(spec=BaseChatModel) + mock_model.ainvoke = _mock_ainvoke + mock_model.temperature = 0.7 + + # Patch ToolAgent.process_message so the recording subclass (which inherits + # it) returns a canned string without executing real tools. ``**_kwargs`` + # absorbs the ``context=`` keyword the LLM tool loop passes through, and + # avoids shadowing the Behave ``context`` object enclosing this helper. + original_process = ToolAgent.process_message + + async def _fake_process(self: Any, message: str, **_kwargs: Any) -> str: + return "echo ok" + + ToolAgent.process_message = _fake_process # type: ignore[method-assign] + try: + with patch( + "cleveractors.agents.llm.build_chat_model", + return_value=mock_model, + ): + context.execute_result = await executor.execute("Do something") + finally: + ToolAgent.process_message = original_process # type: ignore[method-assign] + + +@when("I execute the multi-actor Executor using the custom subclass") +@async_run_until_complete +async def step_execute_multi_actor_executor(context: Any) -> None: + """Execute a multi-actor bundle and assert the injected subclass is used. + + Drives the ``_execute_multi_actor`` dispatch path (issue #73). That path + builds a sub-:class:`Executor` from the selected actor's config; with the + fix it forwards ``tool_agent_class=executor.tool_agent_class`` to the + sub-executor. The bundle's default actor here is a tool actor, so the + sub-executor runs ``_execute_tool``, which constructs the agent via + ``executor.tool_agent_class(...)`` — the recording subclass — incrementing + ``context.instantiation_count``. ``ToolAgent.process_message`` is patched + so the recording subclass returns a canned string without running real + tools. + """ + executor = create_executor( + config_dict=context.executor_config, + credentials=None, + tool_agent_class=context.custom_tool_agent_class, + ) + + # Same process_message patch strategy as the _execute_tool scenario above. + original_process = ToolAgent.process_message + + async def _fake_process(self: Any, message: str, **_kwargs: Any) -> str: + return "echo ok" + + ToolAgent.process_message = _fake_process # type: ignore[method-assign] + try: + context.execute_result = await executor.execute("echo hello") + finally: + ToolAgent.process_message = original_process # type: ignore[method-assign] + + +# --------------------------------------------------------------------------- +# When steps — LLM agent tool loop (issue #73) +# --------------------------------------------------------------------------- + + +@when( + "I run the llm agent tool loop with a mock chat model " + "that returns one tool call then text" +) +@async_run_until_complete +async def step_run_llm_tool_loop(context: Any) -> None: + """Drive the LLMAgent tool loop with a mock chat model. + + The agent under test was produced by ``AgentFactory(tool_agent_class=...)`` + so its internal tool-call loop must construct the injected recording + subclass when dispatching the ``echo`` tool call. ``context.created_agent`` + is the factory-built LLMAgent; we inject a mock chat model that emits one + ``echo`` tool call, then a plain-text final answer, and patch + ``ToolAgent.process_message`` so the recording subclass returns a canned + string without executing real tools. The scenario asserts via + ``context.instantiation_count`` that the subclass was instantiated. + """ + call_counter = [0] + + async def _mock_ainvoke(messages: Any, **invoke_kwargs: Any) -> Any: + call_counter[0] += 1 + if call_counter[0] == 1: + return _mock_ai_message( + "Thinking...", + tool_calls=[ + { + "id": "call_inject_1", + "type": "function", + "function": { + "name": "echo", + "arguments": '{"text": "hello"}', + }, + } + ], + ) + return _mock_ai_message("Final answer", tool_calls=None) + + mock_model = Mock(spec=BaseChatModel) + mock_model.ainvoke = _mock_ainvoke + mock_model.temperature = 0.7 + context.created_agent.chat_model = mock_model + + # Patch ToolAgent.process_message so the recording subclass (which inherits + # it) returns a canned string without executing real tools. ``**_kwargs`` + # absorbs the ``context=`` keyword the LLM tool loop passes through, and + # avoids shadowing the Behave ``context`` object enclosing this helper. + original_process = ToolAgent.process_message + + async def _fake_process(self: Any, message: str, **_kwargs: Any) -> str: + return "echo ok" + + ToolAgent.process_message = _fake_process # type: ignore[method-assign] + try: + context.tool_loop_result = await context.created_agent.process_message( + "Do something" + ) + finally: + ToolAgent.process_message = original_process # type: ignore[method-assign] + + +@when("I construct an LLMAgent directly with a non-ToolAgent tool_agent_class") +def step_construct_llm_with_invalid_tool_agent_class(context: Any) -> None: + """Construct LLMAgent directly with an invalid tool_agent_class. + + Validation runs before LangChain is touched (and before super().__init__), + so this exercises the fail-fast guard without needing LangChain installed. + """ + from cleveractors.agents.llm import LLMAgent + + context.llm_construct_error: Exception | None = None + try: + LLMAgent( + name="bad_ta_agent", + config={"provider": "openai", "model": "gpt-4o-mini"}, + template_renderer=TemplateRenderer(), + tool_agent_class=str, # type: ignore[arg-type] # intentionally invalid + ) + except Exception as exc: # pylint: disable=broad-exception-caught + context.llm_construct_error = exc + + +# --------------------------------------------------------------------------- +# Then steps — AgentFactory assertions +# --------------------------------------------------------------------------- + + +@then('the factory agent_types["tool"] entry should be the default ToolAgent') +def step_assert_factory_default_tool_agent(context: Any) -> None: + assert context.factory_error is None, ( + f"Expected no error, got: {context.factory_error}" + ) + agent_types = context.factory.agent_types + assert "tool" in agent_types, "Expected 'tool' key in agent_types" + assert agent_types["tool"] is ToolAgent, ( + f"Expected ToolAgent, got {agent_types['tool']}" + ) + + +@then('the factory agent_types["tool"] entry should be the custom subclass') +def step_assert_factory_custom_tool_agent(context: Any) -> None: + assert context.factory_error is None, ( + f"Expected no error, got: {context.factory_error}" + ) + agent_types = context.factory.agent_types + assert "tool" in agent_types, "Expected 'tool' key in agent_types" + assert agent_types["tool"] is context.custom_tool_agent_class, ( + f"Expected custom subclass, got {agent_types['tool']}" + ) + + +@then("the factory _tool_agent_class attribute should be the custom subclass") +def step_assert_factory_tool_agent_class_attr(context: Any) -> None: + assert context.factory_error is None, ( + f"Expected no error, got: {context.factory_error}" + ) + assert context.factory._tool_agent_class is context.custom_tool_agent_class, ( + f"Expected custom subclass on _tool_agent_class, " + f"got {context.factory._tool_agent_class}" + ) + + +@then('factory creation should raise ConfigurationError containing "{text}"') +def step_assert_factory_config_error(context: Any, text: str) -> None: + assert context.factory_error is not None, ( + "Expected a ConfigurationError, but no exception was raised" + ) + assert isinstance(context.factory_error, ConfigurationError), ( + f"Expected ConfigurationError, got {type(context.factory_error).__name__}: " + f"{context.factory_error}" + ) + assert text in str(context.factory_error), ( + f"Expected error message to contain {text!r}, got: {context.factory_error}" + ) + + +@then("the created agent should be an instance of the custom subclass") +def step_assert_created_agent_custom_class(context: Any) -> None: + assert context.created_agent_error is None, ( + f"Expected no error creating agent, got: {context.created_agent_error}" + ) + assert isinstance(context.created_agent, context.custom_tool_agent_class), ( + f"Expected instance of custom subclass, got {type(context.created_agent)}" + ) + + +@then("the created agent should be an instance of ToolAgent") +def step_assert_created_agent_tool_agent(context: Any) -> None: + assert context.created_agent_error is None, ( + f"Expected no error creating agent, got: {context.created_agent_error}" + ) + assert isinstance(context.created_agent, ToolAgent), ( + f"Expected instance of ToolAgent, got {type(context.created_agent)}" + ) + + +# --------------------------------------------------------------------------- +# Then steps — Executor assertions +# --------------------------------------------------------------------------- + + +@then("the executor tool_agent_class attribute should be the default ToolAgent") +def step_assert_executor_default_tool_agent(context: Any) -> None: + assert context.executor_error is None, ( + f"Expected no error, got: {context.executor_error}" + ) + assert context.test_executor.tool_agent_class is ToolAgent, ( + f"Expected ToolAgent, got {context.test_executor.tool_agent_class}" + ) + + +@then("the executor tool_agent_class attribute should be the custom subclass") +def step_assert_executor_custom_tool_agent(context: Any) -> None: + assert context.executor_error is None, ( + f"Expected no error, got: {context.executor_error}" + ) + assert context.test_executor.tool_agent_class is context.custom_tool_agent_class, ( + f"Expected custom subclass, got {context.test_executor.tool_agent_class}" + ) + + +@then('executor creation should raise ConfigurationError containing "{text}"') +def step_assert_executor_config_error(context: Any, text: str) -> None: + assert context.executor_error is not None, ( + "Expected a ConfigurationError, but no exception was raised" + ) + assert isinstance(context.executor_error, ConfigurationError), ( + f"Expected ConfigurationError, got {type(context.executor_error).__name__}: " + f"{context.executor_error}" + ) + assert text in str(context.executor_error), ( + f"Expected error message to contain {text!r}, got: {context.executor_error}" + ) + + +@then("the returned executor tool_agent_class should be the default ToolAgent") +def step_assert_returned_executor_default(context: Any) -> None: + assert context.executor_error is None, ( + f"Expected no error, got: {context.executor_error}" + ) + assert context.test_executor.tool_agent_class is ToolAgent, ( + f"Expected ToolAgent, got {context.test_executor.tool_agent_class}" + ) + + +@then("the returned executor tool_agent_class should be the custom subclass") +def step_assert_returned_executor_custom(context: Any) -> None: + assert context.executor_error is None, ( + f"Expected no error, got: {context.executor_error}" + ) + assert context.test_executor.tool_agent_class is context.custom_tool_agent_class, ( + f"Expected custom subclass, got {context.test_executor.tool_agent_class}" + ) + + +# --------------------------------------------------------------------------- +# Then steps — _execute_tool dispatch path +# --------------------------------------------------------------------------- + + +@then("the custom subclass should have been instantiated for tool execution") +def step_assert_custom_subclass_instantiated(context: Any) -> None: + assert context.instantiation_count > 0, ( + "Expected the custom ToolAgent subclass to be instantiated at least once " + f"during tool execution, but instantiation_count={context.instantiation_count}" + ) + + +@then('LLMAgent construction should raise ConfigurationError containing "{text}"') +def step_assert_llm_construct_config_error(context: Any, text: str) -> None: + assert context.llm_construct_error is not None, ( + "Expected a ConfigurationError, but no exception was raised" + ) + assert isinstance(context.llm_construct_error, ConfigurationError), ( + f"Expected ConfigurationError, got " + f"{type(context.llm_construct_error).__name__}: " + f"{context.llm_construct_error}" + ) + assert text in str(context.llm_construct_error), ( + f"Expected error message to contain {text!r}, got: " + f"{context.llm_construct_error}" + ) diff --git a/features/tool_agent_class_injection.feature b/features/tool_agent_class_injection.feature new file mode 100644 index 0000000..9842217 --- /dev/null +++ b/features/tool_agent_class_injection.feature @@ -0,0 +1,118 @@ +Feature: tool_agent_class injection in AgentFactory and create_executor + As a platform developer consuming cleveractors-core + I want to supply a custom ToolAgent subclass to AgentFactory and create_executor + So that I can extend tool agent behaviour without monkey-patching module globals + + # ── AgentFactory ──────────────────────────────────────────────────────────── + + Scenario: AgentFactory defaults to ToolAgent when tool_agent_class is omitted + Given a minimal AgentFactory config with a single tool agent + When I create an AgentFactory without tool_agent_class + Then the factory agent_types["tool"] entry should be the default ToolAgent + + Scenario: AgentFactory uses custom subclass when tool_agent_class is provided + Given a minimal AgentFactory config with a single tool agent + And a custom ToolAgent subclass named "CustomToolAgent" + When I create an AgentFactory with that custom tool_agent_class + Then the factory agent_types["tool"] entry should be the custom subclass + + Scenario: AgentFactory stores custom subclass on _tool_agent_class attribute + Given a minimal AgentFactory config with a single tool agent + And a custom ToolAgent subclass named "CustomToolAgent" + When I create an AgentFactory with that custom tool_agent_class + Then the factory _tool_agent_class attribute should be the custom subclass + + Scenario: AgentFactory rejects non-ToolAgent class as tool_agent_class + Given a minimal AgentFactory config with a single tool agent + When I create an AgentFactory with a non-ToolAgent class as tool_agent_class + Then factory creation should raise ConfigurationError containing "subclass of ToolAgent" + + Scenario: AgentFactory creates tool agent using the custom subclass + Given a minimal AgentFactory config with a single tool agent named "my_tool" + And a custom ToolAgent subclass named "CustomToolAgent" + When I create an AgentFactory with that custom tool_agent_class + And I create the agent named "my_tool" from the factory + Then the created agent should be an instance of the custom subclass + + Scenario: AgentFactory default behaviour is unchanged (no regression) + Given a minimal AgentFactory config with a single tool agent named "default_tool" + When I create an AgentFactory without tool_agent_class + And I create the agent named "default_tool" from the factory + Then the created agent should be an instance of ToolAgent + + # ── Executor and create_executor ──────────────────────────────────────────── + + Scenario: Executor defaults to ToolAgent when tool_agent_class is omitted + Given a valid Executor config dict + When I create an Executor without tool_agent_class + Then the executor tool_agent_class attribute should be the default ToolAgent + + Scenario: Executor stores custom subclass on tool_agent_class attribute + Given a valid Executor config dict + And a custom ToolAgent subclass named "CustomToolAgent" + When I create an Executor with that custom tool_agent_class + Then the executor tool_agent_class attribute should be the custom subclass + + Scenario: Executor rejects non-ToolAgent class as tool_agent_class + Given a valid Executor config dict + When I create an Executor with a non-ToolAgent class as tool_agent_class + Then executor creation should raise ConfigurationError containing "subclass of ToolAgent" + + Scenario: create_executor defaults to ToolAgent when tool_agent_class is omitted + Given a valid Executor config dict + When I call create_executor without tool_agent_class + Then the returned executor tool_agent_class should be the default ToolAgent + + Scenario: create_executor forwards custom subclass to Executor + Given a valid Executor config dict + And a custom ToolAgent subclass named "CustomToolAgent" + When I call create_executor with that custom tool_agent_class + Then the returned executor tool_agent_class should be the custom subclass + + # ── _execute_tool dispatch path ────────────────────────────────────────────── + + Scenario: _execute_tool uses custom subclass when provided via Executor + Given a valid tool Executor config dict + And a custom ToolAgent subclass named "CustomToolAgent" that records instantiation + When I execute the tool actor using the custom subclass + Then the custom subclass should have been instantiated for tool execution + + # ── _execute_llm dispatch path (issue #73: AgentFactory forwarding) ───────── + + Scenario: _execute_llm forwards tool_agent_class so the LLM tool loop uses the custom subclass + Given a single-llm Executor config with tools + And a custom ToolAgent subclass named "RecordingToolAgent" that records instantiation + When I execute the single-llm Executor with a mock chat model that returns one tool call then text + Then the custom subclass should have been instantiated for tool execution + + # ── _execute_multi_actor dispatch path (issue #73: sub-Executor forwarding) ── + + Scenario: _execute_multi_actor forwards tool_agent_class to the sub-Executor + Given a multi-actor Executor config with a tool sub-actor + And a custom ToolAgent subclass named "RecordingToolAgent" that records instantiation + When I execute the multi-actor Executor using the custom subclass + Then the custom subclass should have been instantiated for tool execution + + # ── LLM agent tool loop (issue #73: ephemeral tool executors) ─────────────── + + Scenario: LLMAgent tool loop uses the injected tool_agent_class subclass + Given an AgentFactory config with a single llm agent that has tools + And a custom ToolAgent subclass named "RecordingToolAgent" that records instantiation + When I create an AgentFactory with that custom tool_agent_class + And I create the agent named "llm_with_tools" from the factory + And I run the llm agent tool loop with a mock chat model that returns one tool call then text + Then the custom subclass should have been instantiated for tool execution + + Scenario: LLMAgent rejects a non-ToolAgent tool_agent_class at construction + Given a custom ToolAgent subclass named "CustomToolAgent" + When I construct an LLMAgent directly with a non-ToolAgent tool_agent_class + Then LLMAgent construction should raise ConfigurationError containing "subclass of ToolAgent" + + # ── isinstance compatibility ───────────────────────────────────────────────── + + Scenario: Factory-created custom tool agent is an instance of ToolAgent (LSP) + Given a minimal AgentFactory config with a single tool agent named "lsp_tool" + And a custom ToolAgent subclass named "CustomToolAgent" + When I create an AgentFactory with that custom tool_agent_class + And I create the agent named "lsp_tool" from the factory + Then the created agent should be an instance of ToolAgent diff --git a/src/cleveractors/agents/factory.py b/src/cleveractors/agents/factory.py index 7bc06d2..ea3aea8 100644 --- a/src/cleveractors/agents/factory.py +++ b/src/cleveractors/agents/factory.py @@ -39,6 +39,12 @@ class AgentFactory: dict keyed by provider name (ADR-2026). When provided, credential resolution is delegated to each ``LLMAgent`` via this dict instead of relying on environment variables. + _tool_agent_class (type[ToolAgent]): The concrete ``ToolAgent`` + subclass to use for ``"tool"``-typed agents. Defaults to + :class:`~cleveractors.agents.tool.ToolAgent`. Callers that need + to substitute a custom subclass (e.g. a platform-aware subclass + with native async handlers) supply it via the ``tool_agent_class`` + constructor argument rather than monkey-patching module globals. .. note:: @@ -62,6 +68,8 @@ class AgentFactory: stream_router: Any | None = None, langgraph_bridge: Any | None = None, credentials: dict[str, dict[str, str]] | None = None, + *, + tool_agent_class: type[ToolAgent] = ToolAgent, ) -> None: """ Initialize the AgentFactory. @@ -81,6 +89,17 @@ class AgentFactory: are not consulted. When ``None``, agents fall back to ``config["api_key"]`` or environment variables (standalone / CLI mode). + tool_agent_class: Optional ``ToolAgent`` subclass to use in + place of the default :class:`~cleveractors.agents.tool.ToolAgent`. + Must be a subclass of ``ToolAgent``. When provided, every + internal factory code path that constructs or registers a + ``ToolAgent`` uses this class instead of the module-level + default, removing the need for monkey-patching (issue #73). + + Raises: + ConfigurationError: If ``config`` is not a dict, ``template_renderer`` + is not a :class:`~cleveractors.templates.renderer.TemplateRenderer`, + or ``tool_agent_class`` is not a subclass of ``ToolAgent``. """ # Argument validation (CONTRIBUTING.md §Argument Validation) if not isinstance(config, dict): @@ -92,11 +111,20 @@ class AgentFactory: f"template_renderer must be a TemplateRenderer, " f"got {type(template_renderer).__name__}" ) + if not ( + isinstance(tool_agent_class, type) + and issubclass(tool_agent_class, ToolAgent) + ): + raise ConfigurationError( + f"tool_agent_class must be a subclass of ToolAgent, " + f"got {tool_agent_class!r}" + ) + self._tool_agent_class: type[ToolAgent] = tool_agent_class self.credentials = validate_credentials_structure(credentials) - self.agent_types = { + self.agent_types: dict[str, type[Agent]] = { "llm": LLMAgent, - "tool": ToolAgent, + "tool": self._tool_agent_class, } self.template_renderer = template_renderer self.config = config @@ -271,6 +299,14 @@ class AgentFactory: "config": final_config, "template_renderer": self.template_renderer, } + # LLMAgent owns the multi-turn tool-call loop and constructs + # ephemeral ToolAgent instances internally. Forward the injected + # tool_agent_class so the LLM tool-calling path honours a custom + # subclass (issue #73); without this the loop would fall back to + # the module-level ToolAgent and the monkey-patch could not be + # removed for the LLM path. + if agent_type == "llm": + kwargs["tool_agent_class"] = self._tool_agent_class if agent_type == "llm" and self.credentials is not None: provider = agent_config.get("provider", "openai") if not isinstance(provider, str): diff --git a/src/cleveractors/agents/llm.py b/src/cleveractors/agents/llm.py index d0fbd5c..f6fc6c2 100644 --- a/src/cleveractors/agents/llm.py +++ b/src/cleveractors/agents/llm.py @@ -40,6 +40,7 @@ from cleveractors.agents.llm_client import build_chat_model from cleveractors.agents.llm_imports import populate_langchain_globals from cleveractors.agents.llm_tools import normalize_tool_entry as _normalize_tool_entry from cleveractors.agents.retry import _get_provider_url, call_with_retry +from cleveractors.agents.tool import ToolAgent from cleveractors.core.exceptions import ( AgentCreationError, ConfigurationError, @@ -208,6 +209,8 @@ class LLMAgent(AgentWithMemory): config: dict[str, Any], template_renderer: TemplateRenderer, credentials: dict[str, str] | None = None, + *, + tool_agent_class: type[ToolAgent] = ToolAgent, ) -> None: """ Initialize a reactive LLM agent using LangChain. @@ -225,6 +228,13 @@ class LLMAgent(AgentWithMemory): is required for non-native providers, optional for native ones). When *not* supplied, the agent falls back to ``config["api_key"]`` or environment variables (standalone / CLI mode). + tool_agent_class: Optional ``ToolAgent`` subclass to use when the + multi-turn tool-call loop constructs ephemeral tool executors. + Must be a subclass of :class:`~cleveractors.agents.tool.ToolAgent`. + Defaults to ``ToolAgent`` itself, preserving pre-existing + behaviour. Supplying a subclass here lets platform consumers + override tool agent behaviour for the LLM tool-calling path + without monkey-patching module globals (issue #73). Raises: AgentCreationError: If LangChain dependencies are not available. @@ -261,6 +271,20 @@ class LLMAgent(AgentWithMemory): else: self._credentials = None + # tool_agent_class validation (CONTRIBUTING.md §Argument Validation). + # Fail fast so an invalid subclass is rejected at construction time + # rather than surfacing mid-tool-loop. This mirrors the validation + # performed by AgentFactory and Executor (issue #73). + if not ( + isinstance(tool_agent_class, type) + and issubclass(tool_agent_class, ToolAgent) + ): + raise ConfigurationError( + f"tool_agent_class must be a subclass of ToolAgent, " + f"got {tool_agent_class!r}" + ) + self._tool_agent_class: type[ToolAgent] = tool_agent_class + super().__init__(name, config, template_renderer) # Lazy import LangChain dependencies @@ -905,10 +929,6 @@ class LLMAgent(AgentWithMemory): _bq_output_prune = args.pop("output_prune", None) _bq_prune_ctx = args.pop("output_prune_context", None) try: - from cleveractors.agents.tool import ( - ToolAgent as _TA, - ) - parent_unsafe = self.config.get( "unsafe_mode", False ) @@ -923,7 +943,7 @@ class LLMAgent(AgentWithMemory): ), "timeout": self.config.get("timeout", 1), } - t_agent = _TA( + t_agent = self._tool_agent_class( name=f"_tc_synth_budget_{call_id}", config=tool_cfg, template_renderer=self.template_renderer, @@ -1063,8 +1083,6 @@ class LLMAgent(AgentWithMemory): _output_prune = args_m.pop("output_prune", None) _prune_context = args_m.pop("output_prune_context", None) try: - from cleveractors.agents.tool import ToolAgent as _TA - parent_unsafe = self.config.get("unsafe_mode", False) tool_config: dict[str, Any] = { "tools": [{"name": tool_name}], @@ -1083,7 +1101,7 @@ class LLMAgent(AgentWithMemory): ) continue - agent = _TA( + agent = self._tool_agent_class( name=f"_tc_{call_id}", config=tool_config, template_renderer=self.template_renderer, @@ -1230,10 +1248,6 @@ class LLMAgent(AgentWithMemory): elif isinstance(arguments_raw, dict): args_s = arguments_raw try: - from cleveractors.agents.tool import ( - ToolAgent as _TA, - ) - parent_unsafe = self.config.get("unsafe_mode", False) tool_config_s: dict[str, Any] = { "tools": [{"name": tool_name}], @@ -1242,7 +1256,7 @@ class LLMAgent(AgentWithMemory): "exec_python": self.config.get("exec_python", False), "timeout": self.config.get("timeout", 1), } - agent_s = _TA( + agent_s = self._tool_agent_class( name=f"_tc_synth_{call_id}", config=tool_config_s, template_renderer=self.template_renderer, diff --git a/src/cleveractors/core/application.py b/src/cleveractors/core/application.py index d885bee..9f30760 100644 --- a/src/cleveractors/core/application.py +++ b/src/cleveractors/core/application.py @@ -12,7 +12,7 @@ import logging import re import sys from pathlib import Path -from typing import Any, List, Optional, Type, Union +from typing import Any, List, Optional, Union from rx.core import Observer # type: ignore[attr-defined] from rx.scheduler.eventloop import AsyncIOScheduler # type: ignore[attr-defined] @@ -20,7 +20,6 @@ from rx.scheduler.eventloop import AsyncIOScheduler # type: ignore[attr-defined from cleveractors.agents.base import Agent from cleveractors.agents.factory import AgentFactory from cleveractors.agents.llm import LLMAgent -from cleveractors.agents.tool import ToolAgent from cleveractors.context_manager import ContextManager from cleveractors.core.exceptions import ( AgentCreationError, @@ -227,9 +226,15 @@ class ReactiveCleverAgentsApp: # pylint: disable=too-many-instance-attributes langgraph_bridge=self.langgraph_bridge, ) - # Register built-in agent types + # Register built-in agent types. + # ``AgentFactory.__init__`` already registers ``"llm"`` -> ``LLMAgent`` + # and ``"tool"`` -> its ``tool_agent_class`` (defaulting to + # ``ToolAgent``). The ``"llm"`` registration below is retained as an + # explicit, defensive no-op; the ``"tool"`` registration is intentionally + # omitted so this site no longer hardcodes the module-level ``ToolAgent`` + # symbol (issue #73 audit). Consumers needing a custom tool subclass use + # ``create_executor(tool_agent_class=...)`` directly. self.agent_factory.register_agent_type("llm", LLMAgent) - self.agent_factory.register_agent_type("tool", ToolAgent) # Validate agent configuration early to catch errors at load time self.agent_factory.validate_configuration() @@ -927,15 +932,10 @@ class ReactiveCleverAgentsApp: # pylint: disable=too-many-instance-attributes if not self.agent_factory or not self.config: raise AgentCreationError("Agent factory or configuration not initialized") - # Register built-in agent types - registered_types = self.agent_factory.get_agent_types() - type_map: dict[str, Type[Agent]] = { - "llm": LLMAgent, - "tool": ToolAgent, - } - for type_name, agent_class in type_map.items(): - if type_name not in registered_types: - self.agent_factory.register_agent_type(type_name, agent_class) + # Built-in agent types ("llm" and "tool") are registered by + # ``AgentFactory.__init__``; the redundant re-registration that used to + # live here was dead code (the "already registered" guard never fired) + # and hardcoded the module-level ``ToolAgent`` symbol (issue #73 audit). # Create agents for agent_name, agent_config in self.config.agents.items(): diff --git a/src/cleveractors/runtime.py b/src/cleveractors/runtime.py index c75d957..50ae235 100644 --- a/src/cleveractors/runtime.py +++ b/src/cleveractors/runtime.py @@ -16,6 +16,7 @@ from __future__ import annotations from collections.abc import AsyncGenerator from typing import Any +from cleveractors.agents.tool import ToolAgent from cleveractors.core.exceptions import ConfigurationError from cleveractors.result import ActorResult, NodeUsage from cleveractors.runtime_dispatch import ( @@ -47,6 +48,14 @@ class Executor: Constructed via :func:`create_executor`. Credentials are per-request; the router should construct a new executor per request (ADR-2024). + + Attributes: + tool_agent_class (type[ToolAgent]): The concrete ``ToolAgent`` + subclass used by all dispatch paths that need to construct + a ``ToolAgent`` (or register one in ``AgentFactory``). + Defaults to :class:`~cleveractors.agents.tool.ToolAgent`. + Supply a subclass to override tool agent behaviour at runtime + without monkey-patching (issue #73). """ def __init__( @@ -55,6 +64,8 @@ class Executor: credentials: dict[str, Any] | None, limits: dict[str, Any], pricing: dict[str, Any], + *, + tool_agent_class: type[ToolAgent] = ToolAgent, ): if not isinstance(config_dict, dict): raise ConfigurationError("config_dict must be a dict") @@ -64,10 +75,19 @@ class Executor: raise ConfigurationError("limits must be a dict") if not isinstance(pricing, dict): raise ConfigurationError("pricing must be a dict") + if not ( + isinstance(tool_agent_class, type) + and issubclass(tool_agent_class, ToolAgent) + ): + raise ConfigurationError( + f"tool_agent_class must be a subclass of ToolAgent, " + f"got {tool_agent_class!r}" + ) self.config = config_dict self.credentials = credentials self.limits = limits self.pricing = pricing + self.tool_agent_class: type[ToolAgent] = tool_agent_class # Populated by execute_stream() after the async iterator is exhausted. # Remains None until the stream completes (AC4, issue #16). self.last_result: ActorResult | None = None @@ -231,6 +251,8 @@ def create_executor( credentials: dict[str, Any] | None, limits: dict[str, Any] | None = None, pricing: dict[str, Any] | None = None, + *, + tool_agent_class: type[ToolAgent] = ToolAgent, ) -> Executor: """Construct an :class:`Executor` for the supplied actor configuration. @@ -241,6 +263,12 @@ def create_executor( limits: Optional execution limits (max_depth, max_model_calls, max_tool_calls, timeout_ms, max_cost_usd). pricing: Optional pricing table for cost enforcement. + tool_agent_class: Optional ``ToolAgent`` subclass to use in place of + the default :class:`~cleveractors.agents.tool.ToolAgent`. Must be + a subclass of ``ToolAgent``. When provided, every internal dispatch + path that constructs or registers a ``ToolAgent`` uses this class + instead of the module-level default, removing the need for + monkey-patching (issue #73). Returns: An :class:`Executor` instance. Call ``await executor.execute(message)`` @@ -251,4 +279,5 @@ def create_executor( credentials=credentials, limits=limits or {}, pricing=pricing or {}, + tool_agent_class=tool_agent_class, ) diff --git a/src/cleveractors/runtime_dispatch.py b/src/cleveractors/runtime_dispatch.py index 8fc8db7..030f5e4 100644 --- a/src/cleveractors/runtime_dispatch.py +++ b/src/cleveractors/runtime_dispatch.py @@ -33,7 +33,6 @@ from cleveractors.agents.llm import ( LLMAgent, last_token_usage_var, ) -from cleveractors.agents.tool import ToolAgent from cleveractors.core.exceptions import ( AgentCreationError, ConfigurationError, @@ -140,6 +139,7 @@ async def _execute_llm( config=factory_cfg, credentials=executor.credentials, template_renderer=renderer, + tool_agent_class=executor.tool_agent_class, ) # Build context dict forwarding conversation history for multi-turn support. @@ -344,6 +344,7 @@ async def _execute_graph( config=factory_config, credentials=executor.credentials, template_renderer=renderer, + tool_agent_class=executor.tool_agent_class, ) agents: dict[str, Any] = {} @@ -471,20 +472,25 @@ async def _execute_graph( async def _execute_tool(executor: Executor, message: str) -> ActorResult: """Execute a single tool actor. - Constructs a :class:`ToolAgent` from the actor config and invokes + Constructs a :class:`~cleveractors.agents.tool.ToolAgent` (via + ``executor.tool_agent_class``) from the actor config and invokes ``process_message``. ``ConfigurationError``, ``AgentCreationError`` and ``ExecutionError`` are propagated directly; all other exceptions are wrapped in ``ExecutionError("Tool execution failed: ...")``. - ToolAgent is constructed directly because tool agents do not require - credential injection; AgentFactory is unnecessary for this path. + The tool agent is constructed directly via ``executor.tool_agent_class`` + because tool agents do not require credential injection; AgentFactory is + unnecessary for this path. """ config_block: dict[str, Any] = executor.config.get("config", {}) tools: list[Any] = executor.config.get("tools", config_block.get("tools", [])) agent_config: dict[str, Any] = {"tools": tools} renderer = TemplateRenderer() - agent = ToolAgent( + # Use executor.tool_agent_class so callers that supply a custom subclass + # via create_executor(tool_agent_class=...) have it honoured here too + # (issue #73). + agent = executor.tool_agent_class( name=executor.config.get("name", "tool"), config=agent_config, template_renderer=renderer, @@ -577,6 +583,7 @@ async def _execute_multi_actor( credentials=executor.credentials, limits=executor.limits, pricing=executor.pricing, + tool_agent_class=executor.tool_agent_class, ) result = await sub_executor.execute(message, messages=messages, state=state) @@ -793,6 +800,7 @@ async def _execute_llm_stream( config=factory_cfg, credentials=executor.credentials, template_renderer=renderer, + tool_agent_class=executor.tool_agent_class, ) llm_context: dict[str, Any] | None = None @@ -1215,6 +1223,7 @@ async def _execute_graph_stream( config=factory_config, credentials=executor.credentials, template_renderer=renderer, + tool_agent_class=executor.tool_agent_class, ) agents: dict[str, Any] = {}