forked from HAL9000/cleveragents-core
554d6889cc
## Summary Add the missing `--skill` repeatable flag to `actor run` and `actor-run` CLI commands, aligning the implementation with the specification (CLI Synopsis line 277). The flag enables ad-hoc skill injection at runtime without modifying YAML configuration. Closes #887 ## Changes ### DI Container - **`container.py`**: Added `_build_skill_service()` factory and `skill_service` Singleton provider, following the established `_build_*` pattern. Falls back to in-memory `SkillService()` when the database is unavailable. Exception handling narrowed to `(ImportError, OperationalError, DatabaseError, OSError)` with `exc_info=True` for traceability. ### CLI Layer - **`actor.py`**: Added `--skill` Typer option (`list[str] | None`, repeatable, `metavar="NAME"`). Help text notes that skills only augment tool-bearing agents. Wrapped constructor in the existing `try/except` block so `CleverAgentsException` from skill resolution is properly caught. - **`actor_run.py`**: Same `--skill` option with `metavar="NAME"`. Exception handler catches `CleverAgentsException` (matching master — not broadened to `CleverAgentsError`). - **`skill.py`**: Removed module-level `_service` cache. `_get_skill_service()` now always delegates to `get_container().skill_service()` so that `reset_container()` correctly invalidates the cached instance. `_reset_skill_service()` now overrides the container's provider via `providers.Object()`. Removed dead `validate_skill_names()` function. ### Runtime Layer - **`application.py`** (438 lines, down from 625): `ReactiveCleverAgentsApp` gains `skill_names` parameter with automatic deduplication via `dict.fromkeys`. `_resolve_skills()` obtains `SkillService` from the DI container (no CLI layer import). Separate `except KeyError` and `except ValueError` produce distinct error messages (`"not found in registry"` vs `"resolution failed: {exc}"`). Skill tools are only injected into agents that already have tools (`if self._resolved_skill_tools and tools:`), preventing LLM agents from being converted to pass-through `SimpleToolAgent` instances. When skill tools are skipped for tool-less agents, `logger.debug` emits a diagnostic message. `_sanitize_skill_name()` validates skill name format with tightened regex: `^[\w.-]{1,127}/[\w.-]{1,127}$` with `re.ASCII` flag. Zero-tool skill warning now uses `logger.warning` (not `print(stderr)`), ensuring structured log output and proper log-level filtering. - **`graph_executor.py`** (334 lines): Extracted graph execution logic. Type annotations improved. ### Tests - 24+ Behave scenarios across feature files covering: single/multiple/unknown skill flags, skill+context combined, duplicate deduplication, skill resolution, ValueError path, zero-tool resolution, error handling, tool merging, default behavior, overrides, LLM agent guard, `_sanitize_skill_name` edge cases (empty string, too-long name, ANSI escape codes, disallowed characters), `_build_skill_service` happy+fallback paths, `_get_skill_service` container delegation. - CLI "unknown skill" tests for **both** `actor.py` and `actor_run.py` exercise the real error chain (mock only `get_container()`, not the entire `ReactiveCleverAgentsApp`), testing `_resolve_skills()` → `CleverAgentsException` → `except CleverAgentsException` → exit code 2 end-to-end. - Combined skill+context tests assert `ContextManager` was instantiated and `exists()` was called in dedicated **Then** steps. - `@coverage` tags added to all new scenarios. - **Robot Framework smoke tests** added (`robot/skill_actor_run.robot` + `robot/helper_skill_actor_run.py`): unknown-skill error path and valid-skill acceptance path. ### Changelog - Added entry under `## Unreleased` in `CHANGELOG.md`. ## Review Fixes Applied (Brent Edwards, Rounds 1 & 2) | # | Finding | Resolution | |---|---------|------------| | **P1-1** | `print(stderr)` for zero-tool skill warning | **Fixed** — replaced with `logger.warning("Skill '%s' resolved to zero tools", name)`, removed unused `import sys` | | **P2-2** | Skill tools silently skipped for tool-less agents | **Fixed** — added `logger.debug` when skipped; updated `--skill` help text to note "only augments tool-bearing agents" | | **P2-3** | `container.py` at 739 lines | **Acknowledged** — pre-existing growth (+59 lines for `_build_skill_service`); extracting factories is a separate refactoring task | | **P2-4↑** | `CleverAgentsException` → `CleverAgentsError` broadens catch scope | **Fixed** — reverted `actor_run.py` to `except CleverAgentsException` matching master | | **P3-5** | No Robot Framework smoke test for `--skill` | **Fixed** — added `skill_actor_run.robot` with 2 test cases (unknown-skill error, valid-skill acceptance) | | **P3-6** | `GraphExecutor._follow_chained_edges` static-calling-static | **Acknowledged** — cosmetic pattern that doesn't affect correctness; can address in a follow-up | ## Known Limitations / Deferred Items | Item | Reason | |------|--------| | `actor.py` at 679 lines (500-line guideline) | Pre-existing (670 on master), +9 lines for `--skill`. Refactoring the shared `_execute()` closure is a separate task. | | `container.py` at 739 lines (500-line guideline) | Was 680 lines on master, +59 lines for `_build_skill_service()` and `skill_service` provider. Refactoring into sub-modules is a separate task. | | Code duplication between `actor.py` and `actor_run.py` `run()` | ~47 lines identical code. Coupled with the line-count issue above — both require extracting shared execution logic into a helper module. | | `SimpleToolAgent` only executes `tools[0]` | Deferred to #974. Pre-existing architectural limitation, not introduced by this PR. | | `GraphExecutor._follow_chained_edges` static-calling-static pattern | Cosmetic, doesn't affect behavior. | ## Quality Gates - `nox -s lint`: ✅ PASS - `nox -s typecheck`: ✅ PASS (0 errors) - `nox -s unit_tests`: ✅ PASS (11,130 scenarios, 0 failures) - `nox -s integration_tests`: ✅ PASS (1,559 tests, 0 failures) - `nox -s coverage_report`: ✅ 97% (meets threshold) - Branch rebased onto latest `master` (`ab1fd19b`) Reviewed-on: cleveragents/cleveragents-core#971 Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
841 lines
31 KiB
Python
841 lines
31 KiB
Python
"""Behave steps for reactive application coverage boost - targeting uncovered lines.
|
|
|
|
Uncovered lines targeted:
|
|
73-75: _configure_logging handler creation when root logger has no handlers
|
|
123-125: _ensure_agent_registered creates agent from config when not yet registered
|
|
285-286: _initialize_graph_context generator in any() for partial stage_order
|
|
291: _initialize_graph_context resets writing_stage when value is invalid
|
|
356: _follow_chained_edges returns (msg, True) when next_node is "end"
|
|
370: _follow_chained_edges returns (msg, False) when next_node becomes falsy
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from typing import Any
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
from behave import given, then, when
|
|
from behave.runner import Context
|
|
|
|
from cleveragents.reactive.application import ReactiveCleverAgentsApp
|
|
from cleveragents.reactive.config_parser import AgentConfig, ReactiveConfig
|
|
from cleveragents.reactive.route import RouteConfig, RouteType
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Logging adds a handler when root logger has no handlers
|
|
# Targets lines 73-75
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with all root logger handlers removed")
|
|
def step_app_with_no_handlers(context: Context) -> None:
|
|
root_logger = logging.getLogger()
|
|
context.original_handlers = list(root_logger.handlers)
|
|
context.original_level = root_logger.level
|
|
for handler in list(root_logger.handlers):
|
|
root_logger.removeHandler(handler)
|
|
context.app = ReactiveCleverAgentsApp()
|
|
|
|
|
|
@when("I configure logging with verbose level {level:d}")
|
|
def step_configure_logging_level(context: Context, level: int) -> None:
|
|
context.app._configure_logging(level) # pylint: disable=protected-access
|
|
|
|
|
|
@then("the root logger should have at least one handler")
|
|
def step_root_logger_has_handler(context: Context) -> None:
|
|
root_logger = logging.getLogger()
|
|
assert len(root_logger.handlers) >= 1, (
|
|
"Expected root logger to have at least one handler"
|
|
)
|
|
|
|
|
|
@then("the handler format should include the module name pattern")
|
|
def step_handler_format_contains_name(context: Context) -> None:
|
|
root_logger = logging.getLogger()
|
|
found = False
|
|
for handler in root_logger.handlers:
|
|
fmt = handler.formatter
|
|
if fmt and "%(name)s" in fmt._fmt:
|
|
found = True
|
|
break
|
|
assert found, "Expected at least one handler with '[%(name)s] %(message)s' format"
|
|
# Restore original handlers to avoid side-effects on other tests
|
|
for handler in list(root_logger.handlers):
|
|
root_logger.removeHandler(handler)
|
|
for handler in context.original_handlers:
|
|
root_logger.addHandler(handler)
|
|
root_logger.setLevel(context.original_level)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: _ensure_agent_registered creates instance from config
|
|
# Targets lines 123-125
|
|
#
|
|
# The first loop in _register_agents_from_config iterates config.agents and
|
|
# registers each. The second loop iterates routes and calls the nested
|
|
# _ensure_agent_registered for agents/operators. To hit lines 123-125 we
|
|
# need an agent that is NOT in stream_router.agents during the route loop
|
|
# but IS found via config.agents.get(). We achieve this with a custom dict
|
|
# whose items() adds a new key only after its first iteration (the first
|
|
# loop), making it invisible to the first loop but present for the second.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class _LazyAgentsDict(dict):
|
|
"""Dict that injects a new agent key after its first items() call."""
|
|
|
|
def __init__(self, initial: dict[str, Any], deferred_key: str, deferred_value: Any):
|
|
super().__init__(initial)
|
|
self._deferred_key = deferred_key
|
|
self._deferred_value = deferred_value
|
|
self._first_iteration_done = False
|
|
|
|
def items(self):
|
|
result = list(super().items())
|
|
if not self._first_iteration_done:
|
|
self._first_iteration_done = True
|
|
self[self._deferred_key] = self._deferred_value
|
|
return result
|
|
|
|
|
|
@given(
|
|
"a reactive app with a route referencing an agent not yet registered but present in config"
|
|
)
|
|
def step_app_with_unregistered_config_agent(context: Context) -> None:
|
|
context.app = ReactiveCleverAgentsApp()
|
|
|
|
|
|
@when("I register agents from config triggering deferred registration")
|
|
def step_register_triggering_deferred(context: Context) -> None:
|
|
lazy_agents = _LazyAgentsDict(
|
|
initial={
|
|
"dummy_actor": AgentConfig(name="dummy_actor", type="custom", config={}),
|
|
},
|
|
deferred_key="target_actor",
|
|
deferred_value=AgentConfig(name="target_actor", type="custom", config={}),
|
|
)
|
|
|
|
app = ReactiveCleverAgentsApp()
|
|
route = RouteConfig(
|
|
name="trigger_route",
|
|
type=RouteType.STREAM,
|
|
agents=["target_actor"],
|
|
operators=[{"type": "map", "params": {"actor": "target_actor"}}],
|
|
)
|
|
app.config = ReactiveConfig(
|
|
agents=lazy_agents, # type: ignore[arg-type]
|
|
routes={"trigger_route": route},
|
|
)
|
|
object.__setattr__(app.config, "agents", lazy_agents)
|
|
|
|
app._register_agents_from_config() # pylint: disable=protected-access
|
|
|
|
context.test_app = app
|
|
context.target_agent_name = "target_actor"
|
|
|
|
|
|
@then("the deferred config agent should be registered in the stream router")
|
|
def step_deferred_agent_registered(context: Context) -> None:
|
|
assert context.target_agent_name in context.test_app.stream_router.agents, (
|
|
f"Expected '{context.target_agent_name}' in stream_router.agents"
|
|
)
|
|
|
|
|
|
@then("the deferred config agent alias should also be registered")
|
|
def step_deferred_agent_alias_registered(context: Context) -> None:
|
|
alias = "target_agent"
|
|
assert alias in context.test_app.stream_router.agents, (
|
|
f"Expected alias '{alias}' in stream_router.agents"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: _initialize_graph_context refreshes stage_order when partial
|
|
# Targets lines 285-286
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with a global context that has a partial stage order list")
|
|
def step_app_partial_stage_order(context: Context) -> None:
|
|
context.app = ReactiveCleverAgentsApp()
|
|
context.app.config = ReactiveConfig(
|
|
global_context={
|
|
"stage_order": ["intro", "discovery", "brainstorming"],
|
|
"writing_stage": "intro",
|
|
"paper_details": {"topic": "test"},
|
|
}
|
|
)
|
|
|
|
|
|
@when("I initialize the graph context")
|
|
def step_initialize_graph_context(context: Context) -> None:
|
|
context.graph_context = context.app._initialize_graph_context() # pylint: disable=protected-access
|
|
|
|
|
|
@then("the stage order should be refreshed to the full default list")
|
|
def step_stage_order_refreshed(context: Context) -> None:
|
|
expected = [
|
|
"intro",
|
|
"discovery",
|
|
"brainstorming",
|
|
"vetting",
|
|
"structure",
|
|
"section_writing",
|
|
"paper_review",
|
|
"latex_generation",
|
|
]
|
|
assert context.graph_context["stage_order"] == expected, (
|
|
f"Expected default stage_order but got {context.graph_context['stage_order']}"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: _initialize_graph_context resets writing_stage when invalid
|
|
# Targets line 291
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with a global context that has an invalid writing stage")
|
|
def step_app_invalid_writing_stage(context: Context) -> None:
|
|
context.app = ReactiveCleverAgentsApp()
|
|
full_stages = [
|
|
"intro",
|
|
"discovery",
|
|
"brainstorming",
|
|
"vetting",
|
|
"structure",
|
|
"section_writing",
|
|
"paper_review",
|
|
"latex_generation",
|
|
]
|
|
context.app.config = ReactiveConfig(
|
|
global_context={
|
|
"stage_order": full_stages,
|
|
"writing_stage": "nonexistent_stage",
|
|
"paper_details": {"topic": "test"},
|
|
}
|
|
)
|
|
|
|
|
|
@then("the writing stage should be reset to intro")
|
|
def step_writing_stage_reset(context: Context) -> None:
|
|
assert context.graph_context["writing_stage"] == "intro", (
|
|
f"Expected writing_stage='intro' but got '{context.graph_context['writing_stage']}'"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: _follow_chained_edges returns when next_node is "end"
|
|
# Targets line 356
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured for chained edge traversal to end node")
|
|
def step_app_chained_to_end(context: Context) -> None:
|
|
context.app = ReactiveCleverAgentsApp()
|
|
context.app.config = ReactiveConfig()
|
|
|
|
|
|
@when("I follow chained edges starting from the end node")
|
|
def step_follow_chained_to_end(context: Context) -> None:
|
|
result_msg, should_return = context.app._follow_chained_edges( # pylint: disable=protected-access
|
|
next_targets=["end"],
|
|
current_message="final_output",
|
|
context={},
|
|
node_actor_map={},
|
|
agents={},
|
|
router_node=None,
|
|
select_targets_fn=lambda _: [],
|
|
)
|
|
context.chained_message = result_msg
|
|
context.chained_should_return = should_return
|
|
|
|
|
|
@then("the chained edge result should be the current message with should_return true")
|
|
def step_chained_end_result(context: Context) -> None:
|
|
assert context.chained_message == "final_output", (
|
|
f"Expected 'final_output' but got '{context.chained_message}'"
|
|
)
|
|
assert context.chained_should_return is True, "Expected should_return=True"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: _follow_chained_edges falls through when next_node becomes falsy
|
|
# Targets line 370
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured for chained edge traversal to a falsy node")
|
|
def step_app_chained_to_falsy(context: Context) -> None:
|
|
context.app = ReactiveCleverAgentsApp()
|
|
context.app.config = ReactiveConfig()
|
|
|
|
|
|
@when("I follow chained edges starting from a node that chains to a falsy node")
|
|
def step_follow_chained_to_falsy(context: Context) -> None:
|
|
call_count = {"n": 0}
|
|
|
|
def select_targets(node: str) -> list[str]:
|
|
call_count["n"] += 1
|
|
if call_count["n"] == 1:
|
|
return [""] # empty string is falsy -> while loop exits
|
|
return []
|
|
|
|
result_msg, should_return = context.app._follow_chained_edges( # pylint: disable=protected-access
|
|
next_targets=["some_node"],
|
|
current_message="test_message",
|
|
context={},
|
|
node_actor_map={},
|
|
agents={},
|
|
router_node=None,
|
|
select_targets_fn=select_targets,
|
|
)
|
|
context.chained_message = result_msg
|
|
context.chained_should_return = should_return
|
|
|
|
|
|
@then("the chained edge result should be the current message with should_return false")
|
|
def step_chained_falsy_result(context: Context) -> None:
|
|
assert context.chained_message == "test_message", (
|
|
f"Expected 'test_message' but got '{context.chained_message}'"
|
|
)
|
|
assert context.chained_should_return is False, "Expected should_return=False"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app resolves skill names and stores tools
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with skill names resolved via a mock skill service")
|
|
def step_app_with_skill_names_resolved(context: Context) -> None:
|
|
from cleveragents.domain.models.core.skill import ResolvedToolEntry
|
|
|
|
mock_service = MagicMock()
|
|
mock_skill = MagicMock()
|
|
mock_entries = [
|
|
ResolvedToolEntry(
|
|
name="tool-a",
|
|
source_skill="local/web-tools",
|
|
is_inline=False,
|
|
),
|
|
ResolvedToolEntry(
|
|
name="tool-b",
|
|
source_skill="local/web-tools",
|
|
is_inline=True,
|
|
),
|
|
]
|
|
mock_service.resolve_tools.return_value = (mock_skill, mock_entries)
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = mock_service
|
|
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
context.app = ReactiveCleverAgentsApp(
|
|
skill_names=["local/web-tools"],
|
|
)
|
|
context.mock_service = mock_service
|
|
|
|
|
|
@when("I check the resolved skill tools")
|
|
def step_check_resolved_skill_tools(context: Context) -> None:
|
|
context.resolved_tools = context.app.resolved_skill_tools
|
|
context.skill_names_result = context.app.skill_names
|
|
|
|
|
|
@then("the app should have resolved skill tool entries")
|
|
def step_app_has_resolved_tools(context: Context) -> None:
|
|
assert len(context.resolved_tools) == 2, (
|
|
f"Expected 2 resolved tools but got {len(context.resolved_tools)}"
|
|
)
|
|
assert context.resolved_tools[0]["name"] == "tool-a"
|
|
assert context.resolved_tools[0]["source_skill"] == "local/web-tools"
|
|
assert context.resolved_tools[0]["is_inline"] is False
|
|
assert context.resolved_tools[0]["operation"] == "identity"
|
|
assert context.resolved_tools[0]["skill_source"] == "local/web-tools"
|
|
assert context.resolved_tools[1]["name"] == "tool-b"
|
|
assert context.resolved_tools[1]["is_inline"] is True
|
|
assert context.resolved_tools[1]["skill_source"] == "local/web-tools"
|
|
|
|
|
|
@then("the skill names property should match the input")
|
|
def step_skill_names_match(context: Context) -> None:
|
|
assert context.skill_names_result == ["local/web-tools"]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app raises error for unknown skill name
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured with an unknown skill name")
|
|
def step_app_with_unknown_skill(context: Context) -> None:
|
|
mock_service = MagicMock()
|
|
mock_service.resolve_tools.side_effect = KeyError(
|
|
"Skill 'local/bad' is not registered"
|
|
)
|
|
context.mock_service = mock_service
|
|
|
|
|
|
@when("I attempt to create the app with the unknown skill")
|
|
def step_attempt_create_app_with_unknown_skill(context: Context) -> None:
|
|
from cleveragents.core.exceptions import CleverAgentsException
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = context.mock_service
|
|
|
|
context.raised_exception = None
|
|
try:
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
ReactiveCleverAgentsApp(skill_names=["local/bad"])
|
|
except CleverAgentsException as exc:
|
|
context.raised_exception = exc
|
|
|
|
|
|
@then("a CleverAgentsException should be raised with skill not found message")
|
|
def step_exception_raised_with_message(context: Context) -> None:
|
|
assert context.raised_exception is not None, "Expected CleverAgentsException"
|
|
assert "local/bad" in str(context.raised_exception)
|
|
assert "not found" in str(context.raised_exception)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app merges skill tools into agent tool list
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with skill tools resolved and a config with agents")
|
|
def step_app_with_skill_tools_and_config(context: Context) -> None:
|
|
from cleveragents.domain.models.core.skill import ResolvedToolEntry
|
|
|
|
mock_service = MagicMock()
|
|
mock_skill = MagicMock()
|
|
mock_entries = [
|
|
ResolvedToolEntry(
|
|
name="skill-tool-x",
|
|
source_skill="local/test-skill",
|
|
is_inline=False,
|
|
),
|
|
]
|
|
mock_service.resolve_tools.return_value = (mock_skill, mock_entries)
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = mock_service
|
|
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
context.app = ReactiveCleverAgentsApp(
|
|
skill_names=["local/test-skill"],
|
|
)
|
|
|
|
|
|
@when("agents are registered from config with skill tools")
|
|
def step_register_agents_with_skill_tools(context: Context) -> None:
|
|
agent_cfg = AgentConfig(
|
|
name="test_actor",
|
|
type="custom",
|
|
config={"tools": [{"operation": "uppercase"}]},
|
|
)
|
|
context.app.config = ReactiveConfig(
|
|
agents={"test_actor": agent_cfg},
|
|
routes={},
|
|
)
|
|
context.app._register_agents_from_config() # pylint: disable=protected-access
|
|
|
|
|
|
@then("the agents should have skill tools merged into their tool list")
|
|
def step_agents_have_merged_tools(context: Context) -> None:
|
|
from cleveragents.reactive.stream_router import SimpleToolAgent
|
|
|
|
agent = context.app.stream_router.agents.get("test_actor")
|
|
assert agent is not None, "Expected test_actor to be registered"
|
|
assert isinstance(agent, SimpleToolAgent), "Expected SimpleToolAgent instance"
|
|
# Original tool + 1 skill tool = 2
|
|
assert len(agent.tools) == 2, f"Expected 2 tools but got {len(agent.tools)}"
|
|
assert agent.tools[0] == {"operation": "uppercase"}
|
|
assert agent.tools[1]["name"] == "skill-tool-x"
|
|
assert agent.tools[1]["skill_source"] == "local/test-skill"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app works without skill names
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app created without any skill names")
|
|
def step_app_without_skills(context: Context) -> None:
|
|
context.app = ReactiveCleverAgentsApp()
|
|
|
|
|
|
@when("I check the skill related properties")
|
|
def step_check_skill_properties(context: Context) -> None:
|
|
context.skill_names_result = context.app.skill_names
|
|
context.resolved_tools_result = context.app.resolved_skill_tools
|
|
|
|
|
|
@then("the skill names should be empty")
|
|
def step_skill_names_empty(context: Context) -> None:
|
|
assert context.skill_names_result == [], (
|
|
f"Expected empty skill names but got {context.skill_names_result}"
|
|
)
|
|
|
|
|
|
@then("the resolved skill tools should be empty")
|
|
def step_resolved_tools_empty(context: Context) -> None:
|
|
assert context.resolved_tools_result == [], (
|
|
f"Expected empty resolved tools but got {context.resolved_tools_result}"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app resolves skill with tool overrides
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with skill names resolved including overrides")
|
|
def step_app_with_overrides(context: Context) -> None:
|
|
from cleveragents.domain.models.core.skill import ResolvedToolEntry
|
|
|
|
mock_service = MagicMock()
|
|
mock_skill = MagicMock()
|
|
mock_entries = [
|
|
ResolvedToolEntry(
|
|
name="tool-with-overrides",
|
|
source_skill="local/override-skill",
|
|
is_inline=False,
|
|
overrides={"timeout": 600, "retries": 3},
|
|
),
|
|
]
|
|
mock_service.resolve_tools.return_value = (mock_skill, mock_entries)
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = mock_service
|
|
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
context.app = ReactiveCleverAgentsApp(
|
|
skill_names=["local/override-skill"],
|
|
)
|
|
|
|
|
|
@then("the resolved tool entry should include overrides")
|
|
def step_resolved_tool_has_overrides(context: Context) -> None:
|
|
assert len(context.resolved_tools) == 1, (
|
|
f"Expected 1 resolved tool but got {len(context.resolved_tools)}"
|
|
)
|
|
tool = context.resolved_tools[0]
|
|
assert tool["name"] == "tool-with-overrides"
|
|
assert tool["skill_source"] == "local/override-skill"
|
|
assert "overrides" in tool, "Expected 'overrides' key in tool dict"
|
|
assert tool["overrides"] == {"timeout": 600, "retries": 3}
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app deduplicates skill names (M7)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with duplicate skill names resolved via a mock skill service")
|
|
def step_app_with_duplicate_skills(context: Context) -> None:
|
|
from cleveragents.domain.models.core.skill import ResolvedToolEntry
|
|
|
|
mock_service = MagicMock()
|
|
mock_skill = MagicMock()
|
|
mock_entries = [
|
|
ResolvedToolEntry(
|
|
name="tool-a",
|
|
source_skill="local/web-tools",
|
|
is_inline=False,
|
|
),
|
|
]
|
|
mock_service.resolve_tools.return_value = (mock_skill, mock_entries)
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = mock_service
|
|
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
context.app = ReactiveCleverAgentsApp(
|
|
skill_names=["local/web-tools", "local/web-tools", "local/web-tools"],
|
|
)
|
|
context.mock_service = mock_service
|
|
|
|
|
|
@then("the skill names should be deduplicated")
|
|
def step_skill_names_deduplicated(context: Context) -> None:
|
|
assert context.app.skill_names == ["local/web-tools"], (
|
|
f"Expected deduplicated list but got {context.app.skill_names}"
|
|
)
|
|
|
|
|
|
@then("resolve_tools should be called once per unique skill")
|
|
def step_resolve_called_once(context: Context) -> None:
|
|
context.mock_service.resolve_tools.assert_called_once_with("local/web-tools")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app raises error for empty string skill name (m3)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured with an empty string skill name")
|
|
def step_app_with_empty_string_skill(context: Context) -> None:
|
|
# No mock service needed — the empty string is rejected by
|
|
# _sanitize_skill_name before _resolve_skills is even called.
|
|
pass
|
|
|
|
|
|
@when("I attempt to create the app with the empty skill")
|
|
def step_attempt_create_app_with_empty_skill(context: Context) -> None:
|
|
from cleveragents.core.exceptions import CleverAgentsException
|
|
|
|
context.raised_exception = None
|
|
try:
|
|
ReactiveCleverAgentsApp(skill_names=[""])
|
|
except CleverAgentsException as exc:
|
|
context.raised_exception = exc
|
|
|
|
|
|
@then("a CleverAgentsException should be raised for resolution failure")
|
|
def step_exception_raised_for_resolution_failure(context: Context) -> None:
|
|
assert context.raised_exception is not None, "Expected CleverAgentsException"
|
|
msg = str(context.raised_exception)
|
|
assert "Invalid skill name" in msg, f"Expected 'Invalid skill name' in: {msg}"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app skill injection skips LLM agents (C1)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with skill tools and a config with an LLM agent")
|
|
def step_app_with_skill_tools_and_llm_agent(context: Context) -> None:
|
|
from cleveragents.domain.models.core.skill import ResolvedToolEntry
|
|
|
|
mock_service = MagicMock()
|
|
mock_skill = MagicMock()
|
|
mock_entries = [
|
|
ResolvedToolEntry(
|
|
name="skill-tool-x",
|
|
source_skill="local/test-skill",
|
|
is_inline=False,
|
|
),
|
|
]
|
|
mock_service.resolve_tools.return_value = (mock_skill, mock_entries)
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = mock_service
|
|
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
context.app = ReactiveCleverAgentsApp(
|
|
skill_names=["local/test-skill"],
|
|
)
|
|
|
|
|
|
@when("agents are registered from config with skill tools present")
|
|
def step_register_agents_with_skill_tools_and_llm(context: Context) -> None:
|
|
llm_agent_cfg = AgentConfig(
|
|
name="llm_actor",
|
|
type="llm",
|
|
config={"system_prompt": "You are helpful.", "model": "test"},
|
|
)
|
|
tool_agent_cfg = AgentConfig(
|
|
name="tool_actor",
|
|
type="custom",
|
|
config={"tools": [{"operation": "uppercase"}]},
|
|
)
|
|
context.app.config = ReactiveConfig(
|
|
agents={"llm_actor": llm_agent_cfg, "tool_actor": tool_agent_cfg},
|
|
routes={},
|
|
)
|
|
context.app._register_agents_from_config() # pylint: disable=protected-access
|
|
|
|
|
|
@then("the LLM agent should remain a SimpleLLMAgent not a SimpleToolAgent")
|
|
def step_llm_agent_not_converted(context: Context) -> None:
|
|
from cleveragents.reactive.stream_router import SimpleLLMAgent, SimpleToolAgent
|
|
|
|
llm_agent = context.app.stream_router.agents.get("llm_actor")
|
|
assert llm_agent is not None, "Expected llm_actor to be registered"
|
|
assert isinstance(llm_agent, SimpleLLMAgent), (
|
|
f"Expected SimpleLLMAgent but got {type(llm_agent).__name__}"
|
|
)
|
|
|
|
tool_agent = context.app.stream_router.agents.get("tool_actor")
|
|
assert tool_agent is not None, "Expected tool_actor to be registered"
|
|
assert isinstance(tool_agent, SimpleToolAgent), (
|
|
f"Expected SimpleToolAgent but got {type(tool_agent).__name__}"
|
|
)
|
|
# Tool agent should have original tool + skill tool
|
|
assert len(tool_agent.tools) == 2, (
|
|
f"Expected 2 tools but got {len(tool_agent.tools)}"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app raises error for skill resolution ValueError
|
|
# Targets: except ValueError branch in _resolve_skills()
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured with a skill that triggers a ValueError")
|
|
def step_app_with_valueerror_skill(context: Context) -> None:
|
|
mock_service = MagicMock()
|
|
mock_service.resolve_tools.side_effect = ValueError(
|
|
"Cycle detected in skill includes"
|
|
)
|
|
context.mock_service = mock_service
|
|
|
|
|
|
@when("I attempt to create the app with the ValueError skill")
|
|
def step_attempt_create_app_with_valueerror_skill(context: Context) -> None:
|
|
from cleveragents.core.exceptions import CleverAgentsException
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = context.mock_service
|
|
|
|
context.raised_exception = None
|
|
try:
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
ReactiveCleverAgentsApp(skill_names=["local/cycle-skill"])
|
|
except CleverAgentsException as exc:
|
|
context.raised_exception = exc
|
|
|
|
|
|
@then("a CleverAgentsException should be raised with resolution failed message")
|
|
def step_exception_raised_with_resolution_failed(context: Context) -> None:
|
|
assert context.raised_exception is not None, "Expected CleverAgentsException"
|
|
msg = str(context.raised_exception)
|
|
assert "resolution failed" in msg, f"Expected 'resolution failed' in: {msg}"
|
|
assert "local/cycle-skill" in msg, f"Expected 'local/cycle-skill' in: {msg}"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app handles skill that resolves to zero tools
|
|
# Targets: zero-tool warning path (lines 116-124)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app with a skill that resolves to zero tools")
|
|
def step_app_with_zero_tool_skill(context: Context) -> None:
|
|
mock_service = MagicMock()
|
|
mock_skill = MagicMock()
|
|
mock_service.resolve_tools.return_value = (mock_skill, [])
|
|
|
|
mock_container = MagicMock()
|
|
mock_container.skill_service.return_value = mock_service
|
|
|
|
with patch(
|
|
"cleveragents.reactive.application.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
context.app = ReactiveCleverAgentsApp(
|
|
skill_names=["local/empty-skill"],
|
|
)
|
|
|
|
|
|
@when("I check the resolved skill tools after zero tool resolution")
|
|
def step_check_resolved_skill_tools_zero(context: Context) -> None:
|
|
context.resolved_tools_result = context.app.resolved_skill_tools
|
|
context.skill_names_result = context.app.skill_names
|
|
|
|
|
|
@then("the skill names should contain the zero-tool skill")
|
|
def step_skill_names_contain_zero_tool(context: Context) -> None:
|
|
assert context.skill_names_result == ["local/empty-skill"], (
|
|
f"Expected ['local/empty-skill'] but got {context.skill_names_result}"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app rejects skill name exceeding max length
|
|
# Targets: _sanitize_skill_name edge case — name > 127+1+127 = 255 chars
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured with an overly long skill name")
|
|
def step_app_with_long_skill_name(context: Context) -> None:
|
|
# 128 chars in namespace segment exceeds 127 limit
|
|
context.long_name = "a" * 128 + "/" + "b" * 10
|
|
|
|
|
|
@when("I attempt to create the app with the long skill name")
|
|
def step_attempt_create_app_with_long_skill(context: Context) -> None:
|
|
from cleveragents.core.exceptions import CleverAgentsException
|
|
|
|
context.raised_exception = None
|
|
try:
|
|
ReactiveCleverAgentsApp(skill_names=[context.long_name])
|
|
except CleverAgentsException as exc:
|
|
context.raised_exception = exc
|
|
|
|
|
|
@then("a CleverAgentsException should be raised for invalid name format")
|
|
def step_exception_raised_for_invalid_name(context: Context) -> None:
|
|
assert context.raised_exception is not None, "Expected CleverAgentsException"
|
|
msg = str(context.raised_exception)
|
|
assert "Invalid skill name" in msg, f"Expected 'Invalid skill name' in: {msg}"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app strips ANSI escape codes from skill names
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured with a skill name containing ANSI codes")
|
|
def step_app_with_ansi_skill_name(context: Context) -> None:
|
|
# ANSI escape code \x1b[31m is a control character sequence
|
|
context.ansi_name = "\x1b[31mlocal/bad\x1b[0m"
|
|
|
|
|
|
@when("I attempt to create the app with the ANSI skill name")
|
|
def step_attempt_create_app_with_ansi_skill(context: Context) -> None:
|
|
from cleveragents.core.exceptions import CleverAgentsException
|
|
|
|
context.raised_exception = None
|
|
try:
|
|
ReactiveCleverAgentsApp(skill_names=[context.ansi_name])
|
|
except CleverAgentsException as exc:
|
|
context.raised_exception = exc
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Scenario: Reactive app rejects skill name with disallowed characters
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("a reactive app configured with a skill name containing special characters")
|
|
def step_app_with_special_char_skill_name(context: Context) -> None:
|
|
context.special_name = "local/bad@skill!"
|
|
|
|
|
|
@when("I attempt to create the app with the special char skill name")
|
|
def step_attempt_create_app_with_special_char_skill(context: Context) -> None:
|
|
from cleveragents.core.exceptions import CleverAgentsException
|
|
|
|
context.raised_exception = None
|
|
try:
|
|
ReactiveCleverAgentsApp(skill_names=[context.special_name])
|
|
except CleverAgentsException as exc:
|
|
context.raised_exception = exc
|