hurui200320
9ec578f229
CI / lint (pull_request) Successful in 44s
CI / typecheck (pull_request) Successful in 49s
CI / security (pull_request) Successful in 47s
CI / quality (pull_request) Successful in 33s
CI / unit_tests (pull_request) Successful in 3m4s
CI / integration_tests (pull_request) Successful in 1m2s
CI / build (pull_request) Successful in 33s
CI / coverage (pull_request) Successful in 3m5s
CI / status-check (pull_request) Successful in 2s
CI / lint (push) Successful in 31s
CI / typecheck (push) Successful in 46s
CI / security (push) Successful in 46s
CI / quality (push) Successful in 31s
CI / unit_tests (push) Successful in 3m2s
CI / integration_tests (push) Successful in 1m2s
CI / build (push) Successful in 31s
CI / coverage (push) Successful in 3m4s
CI / status-check (push) Successful in 6s
Extends AgentFactory.__init__ and create_executor() with a keyword-only
tool_agent_class: type[ToolAgent] = ToolAgent parameter so callers can
inject a custom ToolAgent subclass at runtime without monkey-patching
module globals (resolves the five-module setattr loop in cleveragents-webapp).
**AgentFactory** (agents/factory.py):
- Accepts tool_agent_class keyword argument; validates it is a ToolAgent
subclass at construction time (fail-fast, CONTRIBUTING.md §Argument
Validation).
- Stores as self._tool_agent_class and uses it to populate
self.agent_types['tool'], so every create_agent('tool') call
instantiates the supplied subclass.
- Forwards tool_agent_class to LLMAgent when creating 'llm'-typed agents
so the LLM tool-calling loop honours the injected subclass.
- Default ToolAgent is preserved, so all existing callers are unaffected.
**LLMAgent** (agents/llm.py):
- Accepts a keyword-only tool_agent_class: type[ToolAgent] = ToolAgent
argument, validated as a ToolAgent subclass at construction time.
- The multi-turn tool-call loop (_execute_tool_loop) constructs its
ephemeral tool executors via self._tool_agent_class(...) across all
three dispatch sites (regular loop, budget-exhaustion synthesis round,
and stuck-model synthesis round), so the LLM tool-calling path no
longer falls back to the module-level ToolAgent. This closes the gap
that previously prevented removing the webapp monkey-patch for the LLM
tool-calling path.
**Executor / create_executor** (runtime.py):
- Executor.__init__ accepts and validates tool_agent_class; stores as
self.tool_agent_class.
- create_executor() accepts tool_agent_class and forwards it to Executor.
**Dispatch paths** (runtime_dispatch.py):
- _execute_tool: uses executor.tool_agent_class(...) instead of the
module-level ToolAgent(...) so the direct-construction path (bypassing
AgentFactory) also honours the injected subclass.
- _execute_graph and _execute_graph_stream: pass
tool_agent_class=executor.tool_agent_class to AgentFactory so graph
nodes using tool-typed agents pick up the subclass.
- _execute_llm and _execute_llm_stream: pass
tool_agent_class=executor.tool_agent_class to AgentFactory so the
single-LLM actor's internally-created LLMAgent (and its tool-calling
loop) honours the injected subclass. Without this, a single LLM actor
that makes tool calls silently fell back to the base ToolAgent.
- _execute_multi_actor: passes 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. Without this, all
dispatch paths reachable through a multi-actor bundle fell back to the
base ToolAgent.
- Removed the now-unused module-level ToolAgent import; the _execute_tool
docstring cross-reference is fully qualified so it still resolves.
All six dispatch paths that construct or dispatch tool agents now
forward the injected subclass.
**ReactiveCleverAgentsApp** (core/application.py):
- Removed the redundant register_agent_type('tool', ToolAgent) call from
load_configuration and the dead type_map re-registration block in
_create_agents; AgentFactory.__init__ already owns the 'tool'
registration. This drops the last hardcoded module-level ToolAgent
references flagged in the issue #73 audit.
**Tests** (features/tool_agent_class_injection.feature + steps):
- 17 BDD scenarios covering: default class when parameter omitted;
custom subclass stored on factory and executor; invalid class rejected
with ConfigurationError (factory, executor, and LLMAgent); factory
create_agent returns custom instance; _execute_tool dispatch
instantiates custom subclass; the LLM tool loop instantiates the
injected subclass; isinstance LSP compatibility through the factory;
the _execute_llm dispatch path forwarding tool_agent_class to
AgentFactory (single-LLM actor making a tool call); and the
_execute_multi_actor dispatch path forwarding tool_agent_class to the
sub-Executor. The two new dispatch-path scenarios were verified to
fail without the source fixes (instantiation_count=0), confirming
they guard the injection contract for the single-LLM and multi-actor
paths.
**Updated tests** (features/steps/runtime_coverage_steps.py,
features/steps/runtime_extended_coverage_steps.py):
- Removed the now-dead patch('cleveractors.runtime_dispatch.ToolAgent')
mocks (and their mock instances) that no longer intercept _execute_tool
after it switched to executor.tool_agent_class. The tool-actor scenario
continues to pass via the real ToolAgent; the normal-coverage branch
uses patch.object(ToolAgent, 'process_message', ...) to stub tool
execution.
**Updated test** (features/steps/credential_executor_paths_steps.py):
- The _execute_multi_actor credentials-forwarding scenario intercepts
Executor.__init__ with a capturing_init to assert the credentials
dict reaches the sub-Executor. Now that _execute_multi_actor forwards
tool_agent_class to the sub-Executor, capturing_init accepts and
forwards that keyword so the assertion continues to pass.
**Removed obsolete test** (features/application_coverage_gaps.feature):
- Dropped the '_create_agents registers built-in agent types' scenario
and its step definitions; the behaviour it asserted (redundant
re-registration) was removed from _create_agents.
ISSUES CLOSED: #73
242 lines
12 KiB
Gherkin
242 lines
12 KiB
Gherkin
Feature: Application Error Handling, Stream Routing, and Tool Execution
|
|
As a developer
|
|
I want the application to handle logger setup errors, stream route edge cases, tool command processing, and disposal correctly
|
|
So that application components degrade gracefully and tools execute safely
|
|
|
|
Background:
|
|
Given a fresh application gaps test context (apg)
|
|
|
|
Scenario: Logger handler setup when no handlers exist
|
|
Given a fresh application gaps test context (apg)
|
|
When I create a ReactiveCleverAgentsApp with verbose=4 (apg)
|
|
Then a StreamHandler should be added to the root logger (apg)
|
|
|
|
Scenario: load_configuration wraps non-CleverAgentsException errors
|
|
Given a fresh application gaps test context (apg)
|
|
When load_configuration raises a ValueError (apg)
|
|
Then the error should be wrapped in CleverAgentsException (apg)
|
|
|
|
Scenario: load_configuration re-raises CleverAgentsException directly
|
|
Given a fresh application gaps test context (apg)
|
|
When load_configuration raises a CleverAgentsException (apg)
|
|
Then the original exception should be re-raised unchanged (apg)
|
|
|
|
Scenario: run_single_shot uses pure graph mode
|
|
Given an app with a pure graph "main_graph" (apg)
|
|
When I call run_single_shot with prompt "hello" (apg)
|
|
Then the pure graph should be executed (apg)
|
|
And the result should be returned from graph execution (apg)
|
|
|
|
Scenario: run_single_shot pure graph with context_manager
|
|
Given an app with a pure graph "ctx_graph" and a context_manager (apg)
|
|
When I call run_single_shot with prompt "hello context" and a context_manager (apg)
|
|
Then the context_manager should be passed to the pure graph (apg)
|
|
|
|
Scenario: run_single_shot pure graph with conversation history
|
|
Given an app with a pure graph "hist_graph" and a context_manager with history (apg)
|
|
When I call run_single_shot with prompt "continue" (apg)
|
|
Then conversation_history should be built from context_manager messages (apg)
|
|
|
|
Scenario: _run_pure_graph_single_shot raises when no graphs
|
|
Given an app with empty pure_graphs dict (apg)
|
|
When I call _run_pure_graph_single_shot (apg)
|
|
Then a CleverAgentsException should be raised about missing graph (apg)
|
|
|
|
Scenario: _run_pure_graph_single_shot updates global context after execution
|
|
Given an app with a pure graph "ctx_update_graph" and config (apg)
|
|
When I call _run_pure_graph_single_shot and graph returns context (apg)
|
|
Then the config global_context should be updated with graph context (apg)
|
|
|
|
Scenario: _run_pure_graph_single_shot error wraps in CleverAgentsException
|
|
Given an app with a failing pure graph "fail_graph" (apg)
|
|
When I call _run_pure_graph_single_shot (apg)
|
|
Then the graph error should be wrapped in CleverAgentsException (apg)
|
|
|
|
Scenario: _check_pure_graph_mode returns True when pure_graphs populated
|
|
Given an app with a pure_graph "check_graph" stored (apg)
|
|
When I check pure graph mode (apg)
|
|
Then _check_pure_graph_mode should return True (apg)
|
|
|
|
Scenario: _check_pure_graph_mode returns True when _use_pure_graphs is True
|
|
Given an app with _use_pure_graphs set to True (apg)
|
|
When I check pure graph mode (apg)
|
|
Then _check_pure_graph_mode should return True (apg)
|
|
|
|
Scenario: _check_pure_graph_mode returns False when neither condition met
|
|
Given an app with no pure_graphs and _use_pure_graphs not set (apg)
|
|
When I check pure graph mode (apg)
|
|
Then _check_pure_graph_mode should return False (apg)
|
|
|
|
Scenario: _has_rxpy_stream_routes returns True when stream route present
|
|
Given an app with config having a stream route "input_stream" (apg)
|
|
When I check for RxPY stream routes (apg)
|
|
Then _has_rxpy_stream_routes should return True (apg)
|
|
|
|
Scenario: _has_rxpy_stream_routes returns False when no stream routes
|
|
Given an app with config having only graph routes (apg)
|
|
When I check for RxPY stream routes (apg)
|
|
Then _has_rxpy_stream_routes should return False (apg)
|
|
|
|
Scenario: _has_rxpy_stream_routes returns False when no config
|
|
Given an app with no config (apg)
|
|
When I check for RxPY stream routes (apg)
|
|
Then _has_rxpy_stream_routes should return False (apg)
|
|
|
|
Scenario: _detect_pure_graph_mode sets flag for all-graph routes
|
|
Given an app with config where all routes are GRAPH type (apg)
|
|
When I call _detect_pure_graph_mode (apg)
|
|
Then _use_pure_graphs should be set to True (apg)
|
|
|
|
Scenario: _detect_pure_graph_mode does not set flag for mixed routes
|
|
Given an app with config having a STREAM and a GRAPH route (apg)
|
|
When I call _detect_pure_graph_mode (apg)
|
|
Then _use_pure_graphs should be set to False (apg)
|
|
|
|
Scenario: Stream error callback logs error
|
|
Given an app with a stream_router set up (apg)
|
|
When the on_error callback receives an error message (apg)
|
|
Then the error should be logged (apg)
|
|
|
|
Scenario: Pure graph execution uses config global_context when no context_manager
|
|
Given an app with a pure graph "ctx_graph" without context_manager (apg)
|
|
When graph execution is called without a context_manager (apg)
|
|
Then config.global_context should be used as initial context (apg)
|
|
|
|
# -- New scenarios targeting uncovered lines in application.py --
|
|
|
|
Scenario: on_error callback logs stream error in single-shot
|
|
Given a fresh application gaps test context (apg)
|
|
When the on_error stream callback receives an error message in single-shot (apg)
|
|
Then the single-shot error should be logged via logger.error (apg)
|
|
|
|
Scenario: Interactive on_output handles empty string content
|
|
Given an app for interactive on_output testing (apg)
|
|
When simulate_on_output receives an empty string (apg)
|
|
Then the debug empty output message should be printed (apg)
|
|
|
|
Scenario: Interactive on_output handles whitespace-only content
|
|
Given an app for interactive on_output testing (apg)
|
|
When simulate_on_output receives a whitespace-only string (apg)
|
|
Then the debug empty output message should be printed (apg)
|
|
|
|
Scenario: Interactive on_output processes non-empty content with tool commands
|
|
Given an app for interactive on_output testing (apg)
|
|
When simulate_on_output receives non-empty content (apg)
|
|
Then the content should be processed through tool commands and printed (apg)
|
|
|
|
Scenario: User input saved to context_manager in interactive session
|
|
Given an app with a context_manager (apg)
|
|
When user input "hello world" is saved to context_manager (apg)
|
|
Then context_manager.add_message should be called with role user and message "hello world" (apg)
|
|
|
|
Scenario: Pure graph execution in interactive mode returns result
|
|
Given an app with pure_graphs and _use_pure_graphs equals True (apg)
|
|
When interactive pure graph processes message "hello graph" (apg)
|
|
Then the graph should execute and return a result (apg)
|
|
And config global_context should be updated from graph context (apg)
|
|
|
|
Scenario: Pure graph in interactive saves assistant response to context_manager
|
|
Given an app with pure_graphs, context_manager, and _use_pure_graphs equals True (apg)
|
|
When interactive pure graph processes message "save this" (apg)
|
|
Then context_manager.add_message should be called with role assistant and the result (apg)
|
|
|
|
Scenario: Pure graph fault in interactive handled gracefully
|
|
Given an app with a failing pure graph and _use_pure_graphs equals True (apg)
|
|
When interactive pure graph processes message "fail" (apg)
|
|
Then the error should be printed and not terminate the session (apg)
|
|
|
|
Scenario: Interactive stream timeout prints warning
|
|
Given an app with config and stream router set up (apg)
|
|
When a stream message times out in interactive session (apg)
|
|
Then a timeout warning should be printed (apg)
|
|
And context_manager.add_message for assistant should NOT be called (apg)
|
|
|
|
Scenario: Interactive KeyboardInterrupt prints exit hint
|
|
Given an app with config (apg)
|
|
When a KeyboardInterrupt is raised during interactive input (apg)
|
|
Then the Use exit to quit message should be printed (apg)
|
|
|
|
Scenario: Interactive EOFError breaks session loop
|
|
Given an app with config (apg)
|
|
When an EOFError is raised during interactive input (apg)
|
|
Then the session loop should break without error (apg)
|
|
|
|
Scenario: Interactive outer error wraps non-CleverAgentsException
|
|
Given an app with config (apg)
|
|
When start_interactive_session encounters a RuntimeError (apg)
|
|
Then the error should be wrapped in CleverAgentsException with session message (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)
|
|
Then the input should be returned unchanged (apg)
|
|
|
|
Scenario: _setup_routes pure graph creation failure logs and re-raises
|
|
Given an app with a graph route that will fail pure graph creation (apg)
|
|
When _setup_routes is called with _use_pure_graphs equals True (apg)
|
|
Then the creation error should be logged and re-raised (apg)
|
|
|
|
Scenario: _enforce_unsafe_flag raises when config requires unsafe and not enabled
|
|
Given an app with config requiring unsafe mode but app not unsafe (apg)
|
|
When _enforce_unsafe_flag is called (apg)
|
|
Then UnsafeConfigurationError should be raised (apg)
|
|
|
|
Scenario: _execute_single_tool agent not found with verbose
|
|
Given an app with empty agents and verbose=1 (apg)
|
|
When _execute_single_tool is called for unknown tool "no_tool" (apg)
|
|
Then an error message with agent not available detail should be returned (apg)
|
|
|
|
Scenario: _execute_single_tool agent not found without verbose
|
|
Given an app with empty agents and verbose=0 (apg)
|
|
When _execute_single_tool is called for unknown tool "no_tool" (apg)
|
|
Then a generic tool execution failed message should be returned (apg)
|
|
|
|
Scenario: _execute_single_tool None result returned as error
|
|
Given an app with an agent returning None from process_message (apg)
|
|
When _execute_single_tool is called for tool "null_tool" (apg)
|
|
Then an error indicating no result was produced should be returned (apg)
|
|
|
|
Scenario: _execute_single_tool exception with verbose includes details
|
|
Given an app with an agent that raises on process_message (apg)
|
|
When _execute_single_tool is called with verbose=1 and tool "crash_tool" (apg)
|
|
Then error details should include the exception message (apg)
|
|
|
|
Scenario: _execute_single_tool exception without verbose shows generic message
|
|
Given an app with an agent that raises on process_message (apg)
|
|
When _execute_single_tool is called with verbose=0 and tool "crash_tool" (apg)
|
|
Then error should suggest using -v flag for details (apg)
|
|
|
|
Scenario: _execute_single_tool global context merge exception is handled
|
|
Given an app with a tool agent and a broken global_context (apg)
|
|
When _execute_single_tool is called for tool "merge_tool" (apg)
|
|
Then the merge exception should be logged at debug level (apg)
|
|
|
|
Scenario: _execute_single_tool uses ThreadPoolExecutor in async context
|
|
Given an app with a tool agent and a running async loop (apg)
|
|
When _execute_single_tool is called from within an async context (apg)
|
|
Then the tool should execute via ThreadPoolExecutor (apg)
|
|
|
|
Scenario: _process_tool_commands bad JSON params with verbose
|
|
Given an app with verbose=1 (apg)
|
|
When _process_tool_commands receives content with invalid JSON params (apg)
|
|
Then invalid tool parameters format error should be returned (apg)
|
|
|
|
Scenario: _handle_stream_command accepts valid stream and message
|
|
Given an app with a stream named "test_stream" (apg)
|
|
When _handle_stream_command is called with "test_stream hello" (apg)
|
|
Then the message should be sent to the stream (apg)
|
|
|
|
Scenario: _handle_stream_command shows error for missing stream
|
|
Given an app with no "missing" stream (apg)
|
|
When _handle_stream_command is called with "missing greeting" (apg)
|
|
Then the stream not found error should be printed (apg)
|
|
|
|
Scenario: _handle_graph_command rejects missing graph route
|
|
Given an app with no "ghost" graph route (apg)
|
|
When _handle_graph_command is called with "ghost hello" (apg)
|
|
Then the graph route not found error should be printed (apg)
|
|
|
|
Scenario: _handle_graph_command executes valid graph route
|
|
Given an app with a graph route "my_graph" (apg)
|
|
When _handle_graph_command is called with "my_graph hello graph" (apg)
|
|
Then the graph should be executed and result displayed (apg) |