From c47e6445d079db175495fc7bd71c4287c5d9247d Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Tue, 24 Feb 2026 06:39:34 +0000 Subject: [PATCH] feat(actor): compile hierarchical actor configs to LangGraph Add ActorCompiler module that translates GRAPH-type ActorConfigSchema definitions into LangGraph NodeConfig/Edge structures with LSP binding metadata. Includes subgraph resolution with cross-actor cycle detection, entry/exit validation, and CompilationMetadata for diagnostics. New files: - src/cleveragents/actor/compiler.py: Core compiler with compile_actor() - features/actor_compiler.feature: 13 Behave scenarios - features/steps/actor_compiler_steps.py: Step definitions - robot/actor_compiler.robot: 4 Robot smoke tests - benchmarks/actor_compiler_bench.py: ASV performance benchmarks - docs/reference/actor_compiler.md: Compilation pipeline reference Modified: - src/cleveragents/actor/__init__.py: Export compiler types - vulture_whitelist.py: Whitelist new public API ISSUES CLOSED: #158 --- benchmarks/actor_compiler_bench.py | 167 ++++ benchmarks/agent_skills_registry_bench.py | 112 +++ benchmarks/skill_flatten_bench.py | 150 ++++ docs/reference/actor_compiler.md | 106 +++ docs/reference/skill_registry.md | 107 +++ features/actor_compiler.feature | 102 +++ features/actor_compiler_coverage.feature | 192 +++++ features/skill_discovery.feature | 176 +++++ features/skill_flatten.feature | 168 ++++ .../steps/actor_compiler_coverage_steps.py | 746 ++++++++++++++++++ features/steps/actor_compiler_steps.py | 536 +++++++++++++ features/steps/skill_discovery_steps.py | 573 ++++++++++++++ features/steps/skill_flatten_steps.py | 506 ++++++++++++ robot/actor_compiler.robot | 44 ++ robot/helper_actor_compiler.py | 74 ++ robot/helper_skill_discovery.py | 147 ++++ robot/helper_skill_flatten.py | 365 +++++++++ robot/skill_discovery.robot | 68 ++ robot/skill_flatten.robot | 97 +++ src/cleveragents/actor/__init__.py | 24 +- src/cleveragents/actor/compiler.py | 345 ++++++++ .../application/services/config_service.py | 12 + .../services/skill_registry_service.py | 113 ++- src/cleveragents/cli/commands/skill.py | 47 ++ src/cleveragents/domain/models/core/skill.py | 84 +- src/cleveragents/skills/__init__.py | 18 + src/cleveragents/skills/discovery.py | 340 ++++++++ src/cleveragents/skills/registry.py | 81 ++ src/cleveragents/tool/registry.py | 6 + src/cleveragents/tool/runtime.py | 18 + vulture_whitelist.py | 10 + 31 files changed, 5530 insertions(+), 4 deletions(-) create mode 100644 benchmarks/actor_compiler_bench.py create mode 100644 benchmarks/agent_skills_registry_bench.py create mode 100644 benchmarks/skill_flatten_bench.py create mode 100644 docs/reference/actor_compiler.md create mode 100644 features/actor_compiler.feature create mode 100644 features/actor_compiler_coverage.feature create mode 100644 features/skill_discovery.feature create mode 100644 features/skill_flatten.feature create mode 100644 features/steps/actor_compiler_coverage_steps.py create mode 100644 features/steps/actor_compiler_steps.py create mode 100644 features/steps/skill_discovery_steps.py create mode 100644 features/steps/skill_flatten_steps.py create mode 100644 robot/actor_compiler.robot create mode 100644 robot/helper_actor_compiler.py create mode 100644 robot/helper_skill_discovery.py create mode 100644 robot/helper_skill_flatten.py create mode 100644 robot/skill_discovery.robot create mode 100644 robot/skill_flatten.robot create mode 100644 src/cleveragents/actor/compiler.py create mode 100644 src/cleveragents/skills/discovery.py diff --git a/benchmarks/actor_compiler_bench.py b/benchmarks/actor_compiler_bench.py new file mode 100644 index 0000000000..b3885073ba --- /dev/null +++ b/benchmarks/actor_compiler_bench.py @@ -0,0 +1,167 @@ +"""ASV benchmarks for actor compiler throughput. + +Measures the performance of: +- compile_actor() on a simple two-node graph +- compile_actor() on a larger multi-node graph +- Subgraph cycle detection overhead +- Compilation metadata serialization +""" + +from __future__ import annotations + +import importlib +import sys +from pathlib import Path + +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +import cleveragents # noqa: E402 + +importlib.reload(cleveragents) + +from cleveragents.actor.compiler import compile_actor # noqa: E402 +from cleveragents.actor.schema import ( # noqa: E402 + ActorConfigSchema, + ActorType, + EdgeDefinition, + NodeDefinition, + NodeType, + RouteDefinition, +) + + +def _make_graph(node_count: int) -> ActorConfigSchema: + """Build a linear graph with *node_count* agent nodes.""" + nodes: list[NodeDefinition] = [] + edges: list[EdgeDefinition] = [] + for i in range(node_count): + nodes.append( + NodeDefinition( + id=f"node-{i}", + type=NodeType.AGENT, + name=f"Node {i}", + description=f"Agent node {i}", + config={"agent": f"agent_{i}"}, + ) + ) + if i > 0: + edges.append(EdgeDefinition(from_node=f"node-{i - 1}", to_node=f"node-{i}")) + route = RouteDefinition( + nodes=nodes, + edges=edges, + entry_node="node-0", + exit_nodes=[f"node-{node_count - 1}"], + ) + return ActorConfigSchema( + name="bench/graph", + type=ActorType.GRAPH, + description="Benchmark graph", + model="gpt-4", + route=route, + ) + + +class CompileSimpleSuite: + """Benchmark compile_actor on a simple 2-node graph.""" + + timeout = 60 + + def setup(self) -> None: + self.config = _make_graph(2) + + def time_compile_simple(self) -> None: + compile_actor(self.config) + + +class CompileLargeSuite: + """Benchmark compile_actor on a 20-node linear graph.""" + + timeout = 60 + + def setup(self) -> None: + self.config = _make_graph(20) + + def time_compile_large(self) -> None: + compile_actor(self.config) + + +class MetadataSerializationSuite: + """Benchmark compilation metadata serialization.""" + + timeout = 60 + + def setup(self) -> None: + self.config = _make_graph(10) + self.compiled = compile_actor(self.config) + + def time_metadata_dump(self) -> None: + self.compiled.metadata.model_dump(mode="json") + + +class SubgraphResolutionSuite: + """Benchmark compile_actor with subgraph resolution.""" + + timeout = 60 + + def setup(self) -> None: + inner_nodes = [ + NodeDefinition( + id="inner", + type=NodeType.AGENT, + name="Inner", + description="Inner agent", + config={"agent": "inner"}, + ), + ] + inner_route = RouteDefinition( + nodes=inner_nodes, + edges=[], + entry_node="inner", + exit_nodes=["inner"], + ) + self.inner_config = ActorConfigSchema( + name="bench/inner", + type=ActorType.GRAPH, + description="Inner graph", + model="gpt-4", + route=inner_route, + ) + + outer_nodes = [ + NodeDefinition( + id="main", + type=NodeType.AGENT, + name="Main", + description="Main agent", + config={"agent": "main"}, + ), + NodeDefinition( + id="sub", + type=NodeType.SUBGRAPH, + name="Sub", + description="Subgraph ref", + config={"actor_ref": "bench/inner"}, + ), + ] + outer_edges = [EdgeDefinition(from_node="main", to_node="sub")] + outer_route = RouteDefinition( + nodes=outer_nodes, + edges=outer_edges, + entry_node="main", + exit_nodes=["sub"], + ) + self.outer_config = ActorConfigSchema( + name="bench/outer", + type=ActorType.GRAPH, + description="Outer graph", + model="gpt-4", + route=outer_route, + ) + self.resolver = lambda name: ( + self.inner_config if name == "bench/inner" else None + ) + + def time_compile_with_subgraph(self) -> None: + compile_actor(self.outer_config, actor_resolver=self.resolver) diff --git a/benchmarks/agent_skills_registry_bench.py b/benchmarks/agent_skills_registry_bench.py new file mode 100644 index 0000000000..c74f09ebc2 --- /dev/null +++ b/benchmarks/agent_skills_registry_bench.py @@ -0,0 +1,112 @@ +"""ASV benchmarks for Agent Skills discovery scan overhead. + +Measures the performance of: +- ``parse_agent_skills_paths`` path parsing +- ``scan_directory`` filesystem scanning +- ``build_tool_spec`` ToolSpec construction +- ``register_discovered_skills`` registration in ToolRegistry +""" + +from __future__ import annotations + +import shutil +import sys +import tempfile +from pathlib import Path + +try: + from cleveragents.skills.discovery import ( + DiscoveredAgentSkill, + build_tool_spec, + parse_agent_skills_paths, + register_discovered_skills, + scan_directory, + ) + from cleveragents.tool.registry import ToolRegistry +except ModuleNotFoundError: + sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) + from cleveragents.skills.discovery import ( + DiscoveredAgentSkill, + build_tool_spec, + parse_agent_skills_paths, + register_discovered_skills, + scan_directory, + ) + from cleveragents.tool.registry import ToolRegistry + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _create_skill_md(folder: Path, name: str) -> None: + """Create a SKILL.md with minimal front-matter.""" + folder.mkdir(parents=True, exist_ok=True) + content = ( + f"---\nname: {name}\ndescription: Benchmark skill {name}\n---\n\n# {name}\n" + ) + (folder / "SKILL.md").write_text(content, encoding="utf-8") + + +def _make_discovered(name: str) -> DiscoveredAgentSkill: + return DiscoveredAgentSkill( + name=name, + description=f"Benchmark skill {name}", + path=f"/tmp/bench/{name}", + ) + + +# --------------------------------------------------------------------------- +# Benchmark suite +# --------------------------------------------------------------------------- + + +class ParsePathsSuite: + """Benchmark path string parsing.""" + + def setup(self) -> None: + self.paths_str = ",".join(f"/opt/skills/dir-{i}" for i in range(20)) + + def time_parse_20_paths(self) -> None: + parse_agent_skills_paths(self.paths_str) + + +class ScanDirectorySuite: + """Benchmark filesystem scanning.""" + + def setup(self) -> None: + self.tmpdir = Path(tempfile.mkdtemp()) + for i in range(50): + _create_skill_md(self.tmpdir / f"skill-{i}", f"skill-{i}") + + def teardown(self) -> None: + shutil.rmtree(str(self.tmpdir), ignore_errors=True) + + def time_scan_50_skills(self) -> None: + scan_directory(self.tmpdir) + + +class BuildToolSpecSuite: + """Benchmark ToolSpec construction from discovered skills.""" + + def setup(self) -> None: + self.skills = [_make_discovered(f"tool-{i}") for i in range(100)] + + def time_build_100_toolspecs(self) -> None: + for skill in self.skills: + build_tool_spec(skill) + + +class RegisterSuite: + """Benchmark tool registration with conflict handling.""" + + params: list[int] = [10, 50, 100] + param_names: list[str] = ["count"] + + def setup(self, count: int) -> None: + self.skills = [_make_discovered(f"tool-{i}") for i in range(count)] + + def time_register(self, count: int) -> None: + registry = ToolRegistry() + register_discovered_skills(self.skills, registry, on_conflict="skip") diff --git a/benchmarks/skill_flatten_bench.py b/benchmarks/skill_flatten_bench.py new file mode 100644 index 0000000000..becd1f2a3e --- /dev/null +++ b/benchmarks/skill_flatten_bench.py @@ -0,0 +1,150 @@ +"""ASV benchmarks for Skill flattening and capability summaries. + +Measures the performance of: +- FlattenSimple: Flatten a skill with 5 tool refs +- FlattenDeep: Flatten a skill with 3 levels of includes +- FlattenWide: Flatten a skill including 10 other skills +- ComputeSummary: Compute capability summary for 20 inline tools +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +try: + from cleveragents.domain.models.core.skill import ( + Skill, + SkillInclude, + SkillInlineTool, + SkillResolver, + ) + from cleveragents.domain.models.core.tool import ( + ToolCapability, + ToolSource, + ) +except ModuleNotFoundError: + sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src")) + from cleveragents.domain.models.core.skill import ( + Skill, + SkillInclude, + SkillInlineTool, + SkillResolver, + ) + from cleveragents.domain.models.core.tool import ( + ToolCapability, + ToolSource, + ) + + +class FlattenSimpleSuite: + """Benchmark flattening a skill with 5 named tool refs.""" + + def setup(self) -> None: + """Prepare skill and resolver.""" + self.skill = Skill( + name="bench/simple", + description="Simple benchmark skill", + tool_refs=[f"bench/tool-{i}" for i in range(5)], + ) + self.resolver = SkillResolver() + + def time_flatten_simple(self) -> None: + """Benchmark resolve_tools for a skill with 5 refs.""" + self.resolver.resolve_tools(self.skill, {}) + + +class FlattenDeepSuite: + """Benchmark flattening a skill with 3 levels of includes.""" + + def setup(self) -> None: + """Build a 3-level include chain.""" + self.base = Skill( + name="bench/deep-base", + description="Base", + tool_refs=["bench/base-tool"], + ) + self.mid = Skill( + name="bench/deep-mid", + description="Mid", + tool_refs=["bench/mid-tool"], + includes=[SkillInclude(name="bench/deep-base")], + ) + self.top = Skill( + name="bench/deep-top", + description="Top", + tool_refs=["bench/top-tool"], + includes=[SkillInclude(name="bench/deep-mid")], + ) + self.registry = { + "bench/deep-base": self.base, + "bench/deep-mid": self.mid, + "bench/deep-top": self.top, + } + self.resolver = SkillResolver() + + def time_flatten_deep(self) -> None: + """Benchmark resolve_tools for a 3-level include chain.""" + self.resolver.resolve_tools(self.top, self.registry) + + +class FlattenWideSuite: + """Benchmark flattening a skill including 10 other skills.""" + + def setup(self) -> None: + """Build a skill with 10 child includes.""" + self.registry: dict[str, Skill] = {} + includes: list[SkillInclude] = [] + for i in range(10): + child_name = f"bench/wide-child-{i}" + self.registry[child_name] = Skill( + name=child_name, + description=f"Child {i}", + tool_refs=[f"bench/wide-tool-{i}"], + ) + includes.append(SkillInclude(name=child_name)) + self.top = Skill( + name="bench/wide-top", + description="Wide skill", + includes=includes, + ) + self.registry["bench/wide-top"] = self.top + self.resolver = SkillResolver() + + def time_flatten_wide(self) -> None: + """Benchmark resolve_tools for a wide include fan-out.""" + self.resolver.resolve_tools(self.top, self.registry) + + +class ComputeSummarySuite: + """Benchmark compute_capability_summary for 20 inline tools.""" + + def setup(self) -> None: + """Create a skill with 20 inline tools of varying capabilities.""" + inlines: list[SkillInlineTool] = [] + for i in range(20): + if i % 3 == 0: + cap = ToolCapability(read_only=True) + elif i % 3 == 1: + cap = ToolCapability(writes=True, side_effects=["fs"]) + else: + cap = ToolCapability(checkpointable=True) + inlines.append( + SkillInlineTool( + description=f"Tool {i}", + source=ToolSource.CUSTOM, + code=f"return {i}", + capability=cap, + ) + ) + self.skill = Skill( + name="bench/summary", + description="Summary benchmark", + anonymous_tools=inlines, + ) + self.resolver = SkillResolver() + self.resolved = self.resolver.resolve_tools(self.skill, {}) + + def time_compute_summary(self) -> None: + """Benchmark compute_capability_summary with 20 tools.""" + self.resolver.compute_capability_summary(self.skill, self.resolved) diff --git a/docs/reference/actor_compiler.md b/docs/reference/actor_compiler.md new file mode 100644 index 0000000000..ca438463b9 --- /dev/null +++ b/docs/reference/actor_compiler.md @@ -0,0 +1,106 @@ +# Actor Compiler + +The actor compiler translates hierarchical YAML-defined GRAPH actors into +LangGraph `StateGraph` node/edge structures that can be executed by the +CleverAgents runtime. + +## Compilation Pipeline + +1. **Input validation** — The compiler accepts an `ActorConfigSchema` with + `type=GRAPH` and a populated `route` field. Non-GRAPH types are rejected + with `ActorCompilationError`. + +2. **Reference validation** — All node IDs referenced in edges, entry, and + exit points are checked against the declared node set. + +3. **Intra-graph cycle detection** — The route's `detect_cycles()` method + verifies the node graph is acyclic. + +4. **Cross-actor subgraph cycle detection** — When an optional + `actor_resolver` is provided, the compiler follows `SUBGRAPH` node + references recursively and detects cycles across actor boundaries + (e.g. actor A → actor B → actor A). + +5. **Node mapping** — Each `NodeDefinition` is mapped to a LangGraph + `NodeConfig` with the appropriate `NodeType` (AGENT, TOOL, CONDITIONAL, + SUBGRAPH). + +6. **Edge mapping** — Each `EdgeDefinition` is mapped to a LangGraph `Edge`. + Conditional expressions are preserved in `edge.condition`. + +7. **LSP binding extraction** — Per-node `lsp_bindings` config entries are + extracted into `LspBinding` objects and stored in compilation metadata. + +8. **Metadata assembly** — The compiler returns a `CompiledActor` containing + the node map, edge list, entry point, and a `CompilationMetadata` object + for diagnostics and CLI inspection. + +## Node Binding + +| Actor Node Type | LangGraph NodeType | Notes | +|---|---|---| +| `agent` | `AGENT` | LLM invocation node | +| `tool` | `TOOL` | Tool execution node | +| `conditional` | `CONDITIONAL` | Routing node | +| `subgraph` | `SUBGRAPH` | Nested actor reference | + +LSP bindings are declared per-node in the `config.lsp_bindings` list: + +```yaml +nodes: + - id: coder + type: agent + name: Code Writer + description: Writes Python code + config: + agent: coder_agent + lsp_bindings: + - lsp_server_name: local/pyright + languages: [python] + auto_detect: true +``` + +## Error Modes + +| Error | Class | When | +|---|---|---| +| Non-GRAPH type | `ActorCompilationError` | `config.type != GRAPH` | +| Missing route | `ActorCompilationError` | `config.route is None` | +| Missing node | `MissingNodeError` | Edge references unknown node | +| Invalid entry/exit | `InvalidEntryExitError` | Entry/exit node not in graph | +| Intra-graph cycle | `SubgraphCycleError` | Nodes form a cycle | +| Cross-actor cycle | `SubgraphCycleError` | Subgraph refs form a cycle | + +## API Reference + +### `compile_actor(config, *, actor_resolver=None) -> CompiledActor` + +Compile an `ActorConfigSchema` into a LangGraph-ready bundle. + +**Parameters:** +- `config` — The actor configuration (must be `ActorType.GRAPH`). +- `actor_resolver` — Optional callable `(name: str) -> ActorConfigSchema | None` + for resolving subgraph references. + +**Returns:** `CompiledActor` with nodes, edges, and metadata. + +### `CompiledActor` + +| Field | Type | Description | +|---|---|---| +| `name` | `str` | Actor name | +| `nodes` | `dict[str, NodeConfig]` | LangGraph node configs | +| `edges` | `list[Edge]` | LangGraph edges | +| `entry_point` | `str` | Entry node ID | +| `metadata` | `CompilationMetadata` | Diagnostic metadata | + +### `CompilationMetadata` + +| Field | Type | Description | +|---|---|---| +| `node_ids` | `list[str]` | All node IDs (sorted) | +| `tool_nodes` | `list[str]` | Tool-type node IDs | +| `lsp_bindings` | `list[LspBinding]` | Per-node LSP bindings | +| `subgraph_refs` | `dict[str, str]` | Subgraph node → actor name | +| `entry_node` | `str` | Entry point node ID | +| `exit_nodes` | `list[str]` | Exit point node IDs | diff --git a/docs/reference/skill_registry.md b/docs/reference/skill_registry.md index a7ad50b3de..81093f3d08 100644 --- a/docs/reference/skill_registry.md +++ b/docs/reference/skill_registry.md @@ -89,3 +89,110 @@ svc.remove_skill("local/code-tools") | item_config | Text | Optional JSON configuration | | item_order | Integer | Stable ordering index | | created_at | String(30) | ISO-8601 creation timestamp | + +## Flattening Rules + +The `SkillResolver` flattens a skill's tool set using the following +deterministic algorithm: + +### Include Order + +Tools are collected via **depth-first traversal** of includes: + +1. For each include (left-to-right), recurse into the included skill + first, processing *its* includes before its own `tool_refs`. +2. After all includes are resolved, add the current skill's `tool_refs` + in definition order. +3. Add inline tools as `{skill_name}/_anon_{index}`. +4. Add MCP tools as `mcp:{server}/{tool_name}`. +5. Add agent skills as `agent_skill:{path}`. + +### De-Duplication + +When the same tool name appears multiple times (from overlapping +includes), the **first occurrence** determines the position in the +output list, but the **last occurrence's entry metadata wins** +(last-wins semantics). + +### Override Merging + +Overrides are applied at two levels: + +1. **Skill-level overrides** (`skill.overrides` dict): Applied to + matching `tool_refs` during resolution. +2. **Per-include overrides** (`SkillInclude.overrides`): Applied to + *all* tool entries contributed by the included skill. Uses + **shallow merge** -- include overrides are merged on top of any + existing entry overrides. + +### Non-Overridable Fields + +The following `ResolvedToolEntry` fields cannot be overridden: + +- `name` +- `source_skill` +- `is_inline` + +Attempting to override these raises `ValueError` with the skill name +and include path for traceability. + +## Capability Summary + +The `SkillCapabilitySummary` aggregates capability metadata across all +resolved tools: + +| Field | Description | +|-----------------------|-------------------------------------------------| +| `total_tools` | Total number of resolved tool entries | +| `read_only_tools` | Count of inline tools with `read_only=True` | +| `write_tools` | Count of inline tools with `writes=True` | +| `checkpointable_tools`| Count of inline tools with `checkpointable=True` | +| `has_side_effects` | `True` if any inline tool has side effects | +| `mcp_sources` | Number of MCP server sources on the root skill | +| `agent_skill_sources` | Number of agent skill sources on the root skill | + +## `validate_plan()` Usage + +The `SkillRegistry.validate_plan()` method checks that a plan's skill +references are satisfiable: + +```python +from cleveragents.skills.registry import SkillRegistry + +registry = SkillRegistry() +# ... register skills ... + +errors = registry.validate_plan({"skills": ["local/code-tools", "local/deploy"]}) +if errors: + for err in errors: + print(f"Plan validation error: {err}") +``` + +### Checks Performed + +1. All referenced skills exist in the registry. +2. All includes are resolvable (no missing skills in include chains). +3. No cycles in include chains. +4. Tool refs are valid (if a tool registry is configured). + +### Error Messages + +| Condition | Error message pattern | +|------------------------|--------------------------------------------------------------| +| Missing skill | `Skill '{name}' referenced in plan is not registered` | +| Missing include | `Skill '{name}': Included skill '{inc}' not found ...` | +| Cycle detected | `Skill '{name}': Cycle detected in skill includes: A -> B` | +| Non-overridable field | `Non-overridable field(s) ... in overrides from skill ...` | + +## `tools()` Method + +The `SkillRegistry.tools()` method combines `resolve_tools()` and +`compute_capability_summary()` into a single call: + +```python +entries, summary = registry.tools("local/code-tools") +print(f"Total tools: {summary.total_tools}") +print(f"Has writes: {summary.write_tools > 0}") +``` + +Returns a 2-tuple of `(list[ResolvedToolEntry], SkillCapabilitySummary)`. diff --git a/features/actor_compiler.feature b/features/actor_compiler.feature new file mode 100644 index 0000000000..227378ff16 --- /dev/null +++ b/features/actor_compiler.feature @@ -0,0 +1,102 @@ +Feature: Actor compiler translates hierarchical actor configs to LangGraph + As a CleverAgents developer + I want to compile GRAPH actor configs into LangGraph node/edge structures + So that actor YAML definitions can be executed as LangGraph StateGraphs + + # ──────────────────────────────────────────────────────────── + # Successful compilation scenarios + # ──────────────────────────────────────────────────────────── + + Scenario: Compile a simple two-node graph actor + Given a GRAPH actor config with a linear two-node topology + When I compile the actor config + Then the compilation should succeed + And the compiled actor should have 2 nodes + And the compiled actor entry point should be "planner" + And the compilation metadata should list node IDs "executor" and "planner" + + Scenario: Compile a graph with a tool node + Given a GRAPH actor config with an agent and a tool node + When I compile the actor config + Then the compilation should succeed + And the compilation metadata should have 1 tool node + + Scenario: Compile a graph with LSP bindings on a node + Given a GRAPH actor config with LSP bindings on the agent node + When I compile the actor config + Then the compilation should succeed + And the compilation metadata should have 1 LSP binding + And the LSP binding should reference server "local/pyright" + + Scenario: Compile a graph with a conditional node + Given a GRAPH actor config with a conditional routing node + When I compile the actor config + Then the compilation should succeed + And the compiled actor should have 3 nodes + + Scenario: Compile a graph with a subgraph reference + Given a GRAPH actor config with a subgraph node referencing "workflows/inner" + When I compile the actor config with a resolver + Then the compilation should succeed + And the compilation metadata subgraph refs should map "sub" to "workflows/inner" + + Scenario: Compile a single-node graph actor + Given a GRAPH actor config with a single node + When I compile the actor config + Then the compilation should succeed + And the compiled actor should have 1 nodes + + # ──────────────────────────────────────────────────────────── + # Subgraph resolution and cycle detection + # ──────────────────────────────────────────────────────────── + + Scenario: Detect cross-actor subgraph cycle + Given a GRAPH actor config "workflows/outer" referencing "workflows/inner" + And a resolver where "workflows/inner" references "workflows/outer" + When I compile the actor config with a resolver + Then the compilation should fail with SubgraphCycleError + And the compiler error message should contain "cycle" + + Scenario: Accept acyclic subgraph chain + Given a GRAPH actor config "workflows/top" referencing "workflows/mid" + And a resolver where "workflows/mid" has no subgraph nodes + When I compile the actor config with a resolver + Then the compilation should succeed + + # ──────────────────────────────────────────────────────────── + # Missing node and invalid entry/exit errors + # ──────────────────────────────────────────────────────────── + + Scenario: Reject non-GRAPH actor type + Given an LLM actor config + When I attempt to compile the non-graph actor + Then the compilation should fail with ActorCompilationError + And the compiler error message should contain "GRAPH" + + Scenario: Reject GRAPH actor without route + Given a GRAPH actor config with route set to None + When I attempt to compile the routeless actor + Then the compilation should fail with ActorCompilationError + And the compiler error message should contain "no route" + + # ──────────────────────────────────────────────────────────── + # Metadata inspection + # ──────────────────────────────────────────────────────────── + + Scenario: Metadata lists all node IDs sorted + Given a GRAPH actor config with nodes "alpha", "beta", and "gamma" + When I compile the actor config + Then the compilation should succeed + And the metadata node IDs should be sorted alphabetically + + Scenario: Metadata records entry and exit nodes + Given a GRAPH actor config with entry "start_node" and exit "end_node" + When I compile the actor config + Then the compilation should succeed + And the metadata entry node should be "start_node" + And the metadata exit nodes should contain "end_node" + + Scenario: Compilation metadata is JSON-serializable + Given a GRAPH actor config with LSP bindings on the agent node + When I compile the actor config + Then the compilation metadata should be JSON-serializable diff --git a/features/actor_compiler_coverage.feature b/features/actor_compiler_coverage.feature new file mode 100644 index 0000000000..4822046c3e --- /dev/null +++ b/features/actor_compiler_coverage.feature @@ -0,0 +1,192 @@ +Feature: Actor compiler full coverage for node mapping, edge mapping, LSP bindings, and error paths + As a CleverAgents developer + I want every code path in the actor compiler module to be tested + So that the compiler is reliable and regressions are caught early + + # ──────────────────────────────────────────────────────────── + # Node type mapping (_map_node and _NODE_TYPE_MAP) + # ──────────────────────────────────────────────────────────── + + Scenario: Compile a graph with all four node types mapped correctly + Given a GRAPH actor config containing AGENT, TOOL, CONDITIONAL, and SUBGRAPH nodes + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled node "agent_node" should have LangGraph type "agent" + And the compiled node "tool_node" should have LangGraph type "tool" + And the compiled node "cond_node" should have LangGraph type "conditional" + And the compiled node "sub_node" should have LangGraph type "subgraph" + + Scenario: Agent node config maps agent field to NodeConfig + Given a GRAPH actor config with an agent node configured as "my_agent" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled node "agent_node" should have agent set to "my_agent" + + Scenario: Conditional node config maps function field to NodeConfig + Given a GRAPH actor config with a conditional node configured with function "route_fn" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled node "cond_node" should have function set to "route_fn" + + Scenario: Tool node config maps tools list to NodeConfig + Given a GRAPH actor config with a tool node configured with tools "lint,format" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled node "tool_node" should have tools "lint" and "format" + + Scenario: Subgraph node config maps actor_ref to NodeConfig subgraph field + Given a GRAPH actor config with a subgraph node referencing actor "workflows/child" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled node "sub_node" should have subgraph set to "workflows/child" + + # ──────────────────────────────────────────────────────────── + # Edge mapping (_map_edge) + # ──────────────────────────────────────────────────────────── + + Scenario: Edge without condition compiles to null condition field + Given a GRAPH actor config with an unconditional edge from "a" to "b" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled edge from "a" to "b" should have a null condition + + Scenario: Edge with condition compiles to expression dict + Given a GRAPH actor config with a conditional edge from "a" to "b" with condition "state.get('done')" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled edge from "a" to "b" should have condition expression "state.get('done')" + + Scenario: Edge priority is recorded in compiled edge metadata + Given a GRAPH actor config with an edge from "a" to "b" with priority 5 + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled edge from "a" to "b" should have metadata priority 5 + + # ──────────────────────────────────────────────────────────── + # LSP binding extraction edge cases (_extract_lsp_bindings) + # ──────────────────────────────────────────────────────────── + + Scenario: Non-list LSP bindings config value is safely ignored + Given a GRAPH actor config where lsp_bindings is a string instead of a list + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compilation metadata should have 0 LSP bindings + + Scenario: Non-dict entry in LSP bindings list is skipped + Given a GRAPH actor config where lsp_bindings contains a non-dict entry + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compilation metadata should have 0 LSP bindings + + Scenario: LSP binding with empty server name is skipped + Given a GRAPH actor config where an LSP binding has an empty server name + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compilation metadata should have 0 LSP bindings + + Scenario: LSP binding with non-string server name is skipped + Given a GRAPH actor config where an LSP binding has a numeric server name + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compilation metadata should have 0 LSP bindings + + Scenario: LSP binding with missing server name key is skipped + Given a GRAPH actor config where an LSP binding is missing the server name key + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compilation metadata should have 0 LSP bindings + + Scenario: Multiple valid LSP bindings are extracted from a single node + Given a GRAPH actor config with two valid LSP bindings on one node + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compilation metadata should have 2 LSP bindings + And the LSP binding servers should include "local/pyright" and "local/ts" + + # ──────────────────────────────────────────────────────────── + # Subgraph cycle detection edge cases (_detect_subgraph_cycles) + # ──────────────────────────────────────────────────────────── + + Scenario: Subgraph node with empty actor_ref is skipped during cycle detection + Given a GRAPH actor config with a subgraph node that has an empty actor_ref + When I compile the full-coverage actor config with a resolver + Then the compilation should succeed without errors + + Scenario: Resolver returning None for a referenced actor skips that node + Given a GRAPH actor config with a subgraph referencing actor name "workflows/missing" + And a cycle-detection resolver that returns None for all names + When I compile the full-coverage actor config with a resolver + Then the compilation should succeed without errors + + Scenario: Referenced actor that is not GRAPH type is skipped during cycle detection + Given a GRAPH actor config with a subgraph referencing actor name "tools/helper" + And a cycle-detection resolver where "tools/helper" is an LLM type actor + When I compile the full-coverage actor config with a resolver + Then the compilation should succeed without errors + + Scenario: Referenced actor with no route definition is skipped during cycle detection + Given a GRAPH actor config with a subgraph referencing actor name "workflows/no-route" + And a cycle-detection resolver where "workflows/no-route" is a GRAPH actor with no route + When I compile the full-coverage actor config with a resolver + Then the compilation should succeed without errors + + Scenario: Deep three-level subgraph cycle is detected + Given a GRAPH actor config named "workflows/alpha" with subgraph ref to "workflows/beta" + And a cycle-detection resolver where "workflows/beta" references "workflows/gamma" and "workflows/gamma" references "workflows/alpha" + When I compile the full-coverage actor config with a resolver + Then the compilation should fail with a SubgraphCycleError + And the compilation error message should mention "cycle" + + # ──────────────────────────────────────────────────────────── + # compile_actor error paths + # ──────────────────────────────────────────────────────────── + + Scenario: Intra-graph cycle raises SubgraphCycleError during compilation + Given a GRAPH actor config with a cyclic route that bypasses schema validation + When I compile the full-coverage actor config + Then the compilation should fail with a SubgraphCycleError + And the compilation error message should mention "intra-graph" + + Scenario: Edge referencing missing from_node raises MissingNodeError + Given a GRAPH actor config with an edge whose from_node does not exist in the node list + When I compile the full-coverage actor config + Then the compilation should fail with a MissingNodeError + And the compilation error message should mention "from_node" + + Scenario: Edge referencing missing to_node raises MissingNodeError + Given a GRAPH actor config with an edge whose to_node does not exist in the node list + When I compile the full-coverage actor config + Then the compilation should fail with a MissingNodeError + And the compilation error message should mention "to_node" + + Scenario: Invalid entry node raises InvalidEntryExitError + Given a GRAPH actor config whose entry node does not match any compiled node + When I compile the full-coverage actor config + Then the compilation should fail with an InvalidEntryExitError + And the compilation error message should mention "Entry node" + + Scenario: Invalid exit node raises InvalidEntryExitError + Given a GRAPH actor config whose exit node does not match any compiled node + When I compile the full-coverage actor config + Then the compilation should fail with an InvalidEntryExitError + And the compilation error message should mention "Exit node" + + # ──────────────────────────────────────────────────────────── + # Model defaults and result bundle + # ──────────────────────────────────────────────────────────── + + Scenario: CompilationMetadata defaults to empty collections + Given a freshly constructed CompilationMetadata with no arguments + Then the metadata node_ids should be an empty list + And the metadata tool_nodes should be an empty list + And the metadata lsp_bindings should be an empty list + And the metadata subgraph_refs should be an empty dict + And the metadata entry_node should be an empty string + And the metadata exit_nodes should be an empty list + + Scenario: CompiledActor preserves actor name and entry point from config + Given a GRAPH actor config named "workflows/test" with entry "start" + When I compile the full-coverage actor config + Then the compilation should succeed without errors + And the compiled actor name should be "workflows/test" + And the compiled actor entry_point field should be "start" diff --git a/features/skill_discovery.feature b/features/skill_discovery.feature new file mode 100644 index 0000000000..1468752afb --- /dev/null +++ b/features/skill_discovery.feature @@ -0,0 +1,176 @@ +Feature: Agent Skills Discovery + As a developer using CleverAgents + I want to discover Agent Skills from configured filesystem paths + So that external tool bundles are automatically registered in the ToolRegistry + + Background: + Given the agent skills discovery module is available + + # --- Config key --- + + Scenario: Config key skills.agent_skills_paths is registered + When I check the config registry for "skills.agent_skills_paths" + Then the config entry should exist + And the config entry type should be "str" + And the config entry env var should be "CLEVERAGENTS_SKILLS_AGENT_SKILLS_PATHS" + + # --- Path parsing --- + + Scenario: Parse single agent skills path + When I parse agent skills paths "~/.cleveragents/agent_skills" + Then the result should contain 1 path + + Scenario: Parse multiple comma-separated paths + When I parse agent skills paths "/opt/skills,/home/user/skills" + Then the result should contain 2 paths + + Scenario: Parse empty agent skills paths + When I parse an empty agent skills path string + Then the result should contain 0 paths + + Scenario: Parse paths with whitespace around commas + When I parse agent skills paths "/opt/skills , /home/user/skills" + Then the result should contain 2 paths + + # --- Discovery --- + + Scenario: Discover agent skills from directory with SKILL.md + Given a temporary directory with agent skill folders + | folder_name | skill_name | description | + | my-tool | my-tool | A test agent skill | + When I scan the directory for agent skills + Then I should discover 1 agent skill + And the discovered skill "my-tool" should have description "A test agent skill" + + Scenario: Discover multiple agent skills + Given a temporary directory with agent skill folders + | folder_name | skill_name | description | + | tool-a | tool-a | First tool | + | tool-b | tool-b | Second tool | + | tool-c | tool-c | Third tool | + When I scan the directory for agent skills + Then I should discover 3 agent skills + + Scenario: Skip directories without SKILL.md + Given a temporary directory with some folders missing SKILL.md + When I scan the directory for agent skills + Then I should discover 0 agent skills + + Scenario: Handle non-existent directory gracefully + When I run discovery on a non-existent directory + Then the discovery result should have 0 discovered skills + And the discovery result should have errors mentioning "does not exist" + + Scenario: Handle SKILL.md without front-matter + Given a temporary directory with a SKILL.md that has no front-matter + When I scan the directory for agent skills + Then I should discover 0 agent skills + + Scenario: Fall back to folder name when SKILL.md has no name field + Given a temporary directory with agent skill folders + | folder_name | skill_name | description | + | fallback-tool | _none_ | Some description | + When I scan the directory for agent skills + Then I should discover 1 agent skill + And the discovered skill should use folder name as name + + # --- ToolSpec building --- + + Scenario: Build ToolSpec from discovered agent skill + Given a discovered agent skill named "my-tool" at "/opt/skills/my-tool" + When I build a ToolSpec from the discovered skill + Then the ToolSpec name should be "agent_skills/my-tool" + And the ToolSpec source should be "agent_skills" + And the ToolSpec source_metadata should contain path "/opt/skills/my-tool" + + # --- Registration --- + + Scenario: Register discovered agent skills in ToolRegistry + Given a ToolRegistry with no existing tools + And a list of 2 discovered agent skills + When I register discovered skills with "skip" conflict strategy + Then 2 tools should be registered in the ToolRegistry + And 0 conflicts should be reported + + Scenario: Skip registration on name collision with skip strategy + Given a ToolRegistry with an existing tool "agent_skills/collider" + And a discovered agent skill named "collider" + When I register discovered skills with "skip" conflict strategy + Then 0 tools should be registered in the ToolRegistry + And 1 conflict should be reported with tool name "agent_skills/collider" + + Scenario: Error on name collision with error strategy + Given a ToolRegistry with an existing tool "agent_skills/collider" + And a discovered agent skill named "collider" + When I register discovered skills with "error" conflict strategy + Then a discovery ValueError should be raised mentioning "name collision" + + Scenario: Replace existing tool on collision with replace strategy + Given a ToolRegistry with an existing tool "agent_skills/collider" + And a discovered agent skill named "collider" + When I register discovered skills with "replace" conflict strategy + Then 1 tool should be registered in the ToolRegistry + And the tool "agent_skills/collider" should have source "agent_skills" + + # --- Refresh --- + + Scenario: Refresh removes old agent skills and re-discovers + Given a SkillRegistryService with a ToolRegistry + And agent skills paths pointing to a directory with 2 skills + When I call discover_and_register + Then 2 agent skills should be registered + When the directory now has 3 skills and I call refresh_agent_skills + Then 3 agent skills should be registered + + # --- Edge cases --- + + Scenario: Register empty discovered list returns empty + Given a ToolRegistry with no existing tools + When I register an empty discovered skills list + Then the registration should return 0 specs and 0 conflicts + + Scenario: Scan non-directory path returns empty + When I scan a non-directory path for agent skills + Then I should discover 0 agent skills + + Scenario: Discover with path that is a file not a directory + When I run discovery on a path that is a file + Then the discovery result should have 0 discovered skills + And the discovery result should have errors mentioning "not a directory" + + Scenario: SKILL.md with invalid YAML front-matter + Given a temporary directory with a SKILL.md that has invalid YAML + When I scan the directory for agent skills + Then I should discover 0 agent skills + + Scenario: SKILL.md with empty YAML block + Given a temporary directory with a SKILL.md that has empty front-matter + When I scan the directory for agent skills + Then I should discover 0 agent skills + + Scenario: SKILL.md with non-dict YAML + Given a temporary directory with a SKILL.md that has non-dict YAML + When I scan the directory for agent skills + Then I should discover 0 agent skills + + Scenario: Skill with non-string description in front-matter + Given a temporary directory with a SKILL.md that has numeric description + When I scan the directory for agent skills + Then I should discover 1 agent skill + + Scenario: Skill with empty description falls back to default + Given a temporary directory with a SKILL.md that has empty description + When I scan the directory for agent skills + Then I should discover 1 agent skill + And the first discovered skill description should contain "Agent skill from" + + Scenario: Noop handler returns expected placeholder + When I call the noop handler + Then the noop handler should return status "agent_skill_placeholder" + + # --- Source metadata --- + + Scenario: Source metadata is included when resolving skill tools + Given a registered skill "local/test-skill" with an agent_skill source + When I resolve tools for "local/test-skill" + Then the resolved tools should include agent_skill source entries diff --git a/features/skill_flatten.feature b/features/skill_flatten.feature new file mode 100644 index 0000000000..7b1bda7507 --- /dev/null +++ b/features/skill_flatten.feature @@ -0,0 +1,168 @@ +@phase1 @domain @skill_flatten +Feature: Skill Registry Flattening and Capability Summaries + As a developer using skill composition + I want skills to be flattened with deterministic ordering and capability summaries + So that actors receive a predictable, well-documented tool set + + # --------------------------------------------------------------------------- + # Flattening with named tool refs (deterministic ordering) + # --------------------------------------------------------------------------- + + @flatten_tool_refs + Scenario: Flattening a skill with named tool refs produces deterministic order + Given a flatten registry skill "local/basic" with tool refs "local/alpha,local/beta,local/gamma" + When I flatten the skill "local/basic" + Then the flatten result should have 3 entries + And the flatten entry at index 0 should be "local/alpha" + And the flatten entry at index 1 should be "local/beta" + And the flatten entry at index 2 should be "local/gamma" + + # --------------------------------------------------------------------------- + # Flattening with includes (depth-first ordering) + # --------------------------------------------------------------------------- + + @flatten_includes + Scenario: Flattening a skill with includes uses depth-first ordering + Given a flatten registry skill "local/base" with tool refs "local/base-tool" + And a flatten registry skill "local/mid" including "local/base" with refs "local/mid-tool" + And a flatten registry skill "local/top" including "local/mid" with refs "local/top-tool" + When I flatten the skill "local/top" + Then the flatten result should have 3 entries + And the flatten entry at index 0 should be "local/base-tool" + And the flatten entry at index 1 should be "local/mid-tool" + And the flatten entry at index 2 should be "local/top-tool" + + # --------------------------------------------------------------------------- + # Flattening with inline tools + # --------------------------------------------------------------------------- + + @flatten_inline + Scenario: Flattening a skill with inline tools includes anonymous entries + Given a flatten registry skill "local/inline-skill" with 2 inline tools + When I flatten the skill "local/inline-skill" + Then the flatten result should have 2 entries + And the flatten entry at index 0 should be "local/inline-skill/_anon_0" + And the flatten entry at index 1 should be "local/inline-skill/_anon_1" + And the flatten entry at index 0 should be marked inline + + # --------------------------------------------------------------------------- + # Cycle detection with clear error path + # --------------------------------------------------------------------------- + + @flatten_cycle + Scenario: Flattening detects cycles and reports a clear path + Given a flatten registry skill "local/cycle-a" including "local/cycle-b" with no refs + And a flatten registry skill "local/cycle-b" including "local/cycle-a" with no refs + When I try to flatten the skill "local/cycle-a" + Then a flatten cycle error should be raised + And the flatten error should mention "local/cycle-a" + And the flatten error should mention "local/cycle-b" + + # --------------------------------------------------------------------------- + # Per-include override application + # --------------------------------------------------------------------------- + + @flatten_include_overrides + Scenario: Per-include overrides are applied to included tool entries + Given a flatten registry skill "local/child" with tool refs "local/tool-x" + And a flatten registry skill "local/parent" including "local/child" with overrides timeout 600 + When I flatten the skill "local/parent" + Then the flatten result should have 1 entries + And the flatten entry "local/tool-x" should have override "timeout" equal to 600 + + # --------------------------------------------------------------------------- + # Non-overridable field rejection + # --------------------------------------------------------------------------- + + @flatten_non_overridable + Scenario: Non-overridable fields are rejected with clear error + Given a flatten registry skill "local/child-nr" with tool refs "local/tool-y" + And a flatten registry skill "local/parent-nr" including "local/child-nr" with overrides on non-overridable field "name" + When I try to flatten the skill "local/parent-nr" + Then a flatten override error should be raised + And the flatten error should mention "name" + And the flatten error should mention "Non-overridable" + + # --------------------------------------------------------------------------- + # Capability summary computation + # --------------------------------------------------------------------------- + + @flatten_capability_summary + Scenario: Capability summary aggregates read/write/checkpoint/side-effect flags + Given a flatten registry skill "local/cap-skill" with mixed capability inline tools + When I flatten the skill "local/cap-skill" and compute summary + Then the flatten summary total_tools should be 3 + And the flatten summary read_only_tools should be 1 + And the flatten summary write_tools should be 1 + And the flatten summary has_side_effects should be true + And the flatten summary checkpointable_tools should be 1 + + # --------------------------------------------------------------------------- + # tools() method returns both entries and summary + # --------------------------------------------------------------------------- + + @flatten_tools_method + Scenario: The tools() method returns entries and capability summary + Given a flatten skill registry with skill "local/tools-test" having 3 refs + When I call the flatten tools method for "local/tools-test" + Then the flatten tools result should contain 3 entries + And the flatten tools result should include a capability summary + + # --------------------------------------------------------------------------- + # validate_plan() with valid plan + # --------------------------------------------------------------------------- + + @flatten_validate_plan + Scenario: validate_plan with a valid plan returns no errors + Given a flatten skill registry with skill "local/plan-skill" having 2 refs + When I call flatten validate_plan with skills "local/plan-skill" + Then the flatten validation result should have 0 errors + + # --------------------------------------------------------------------------- + # validate_plan() with missing skill + # --------------------------------------------------------------------------- + + @flatten_validate_plan_missing + Scenario: validate_plan with a missing skill reports error + Given an empty flatten skill registry + When I call flatten validate_plan with skills "local/nonexistent" + Then the flatten validation result should have 1 errors + And the flatten validation error should mention "local/nonexistent" + + # --------------------------------------------------------------------------- + # validate_plan() with cycle in includes + # --------------------------------------------------------------------------- + + @flatten_validate_plan_cycle + Scenario: validate_plan detects cycle in includes + Given a flatten registry skill "local/loop-a" including "local/loop-b" with no refs + And a flatten registry skill "local/loop-b" including "local/loop-a" with no refs + And the flatten registry skills are registered in the skill registry + When I call flatten validate_plan with skills "local/loop-a" + Then the flatten validation result should have 1 errors + And the flatten validation error should mention "ycle" + + # --------------------------------------------------------------------------- + # De-duplication semantics (last-wins) + # --------------------------------------------------------------------------- + + @flatten_dedup + Scenario: De-duplication uses last-wins for entry metadata + Given a flatten registry skill "local/dup-a" with tool refs "local/shared" + And a flatten registry skill "local/dup-b" with tool refs "local/shared" + And a flatten registry skill "local/dup-top" including "local/dup-a,local/dup-b" with no refs + When I flatten the skill "local/dup-top" + Then the flatten result should have 1 entries + And the flatten entry "local/shared" source_skill should be "local/dup-b" + + # --------------------------------------------------------------------------- + # Override merging (shallow merge) + # --------------------------------------------------------------------------- + + @flatten_override_merge + Scenario: Override merging uses shallow merge semantics + Given a flatten registry skill "local/merge-child" with tool refs "local/merge-tool" and overrides priority 1 + And a flatten registry skill "local/merge-parent" including "local/merge-child" with overrides priority 2 and extra key + When I flatten the skill "local/merge-parent" + Then the flatten entry "local/merge-tool" should have override "priority" equal to 2 + And the flatten entry "local/merge-tool" should have override "extra" equal to "yes" diff --git a/features/steps/actor_compiler_coverage_steps.py b/features/steps/actor_compiler_coverage_steps.py new file mode 100644 index 0000000000..0899cddfcd --- /dev/null +++ b/features/steps/actor_compiler_coverage_steps.py @@ -0,0 +1,746 @@ +"""Step definitions for actor compiler full coverage tests. + +Tests for features/actor_compiler_coverage.feature — targets every code +path in ``cleveragents.actor.compiler`` including edge-case branches in +node mapping, edge mapping, LSP binding extraction, subgraph cycle +detection, and defensive compile_actor error handling. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import patch + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.actor.compiler import ( + CompilationMetadata, + CompiledActor, + InvalidEntryExitError, + MissingNodeError, + SubgraphCycleError, + compile_actor, +) +from cleveragents.actor.schema import ( + ActorConfigSchema, + ActorType, + EdgeDefinition, + NodeDefinition, + NodeType, + RouteDefinition, +) +from cleveragents.langgraph import nodes as lg_nodes + +# ──────────────────────────────────────────────────────────── +# Helper builders +# ──────────────────────────────────────────────────────────── + + +def _build_graph_config( + name: str, + nodes: list[NodeDefinition], + edges: list[EdgeDefinition], + entry_node: str, + exit_nodes: list[str], +) -> ActorConfigSchema: + """Build a valid GRAPH ActorConfigSchema for testing.""" + route = RouteDefinition( + nodes=nodes, + edges=edges, + entry_node=entry_node, + exit_nodes=exit_nodes, + ) + return ActorConfigSchema( + name=name, + type=ActorType.GRAPH, + description="Coverage test graph actor", + model="gpt-4", + route=route, + ) + + +def _build_graph_config_raw( + name: str, + nodes: list[NodeDefinition], + edges: list[EdgeDefinition], + entry_node: str, + exit_nodes: list[str], +) -> ActorConfigSchema: + """Build a GRAPH ActorConfigSchema using model_construct to bypass validators. + + This allows constructing configs that would normally fail validation, + enabling tests for the compiler's own defensive checks. + """ + route = RouteDefinition.model_construct( + nodes=nodes, + edges=edges, + entry_node=entry_node, + exit_nodes=exit_nodes, + ) + return ActorConfigSchema.model_construct( + name=name, + type=ActorType.GRAPH, + description="Coverage test graph actor (raw)", + model="gpt-4", + version="1.0", + tools=[], + memory=None, + context=None, + context_view=None, + system_prompt=None, + route=route, + env_vars={}, + ) + + +def _agent_node(node_id: str, agent: str = "default") -> NodeDefinition: + """Shorthand for creating an AGENT NodeDefinition.""" + return NodeDefinition( + id=node_id, + type=NodeType.AGENT, + name=node_id.title(), + description=f"Agent {node_id}", + config={"agent": agent}, + ) + + +def _tool_node(node_id: str, tools: list[str] | None = None) -> NodeDefinition: + """Shorthand for creating a TOOL NodeDefinition.""" + return NodeDefinition( + id=node_id, + type=NodeType.TOOL, + name=node_id.title(), + description=f"Tool {node_id}", + config={"tool_name": "t/run", "tools": tools or []}, + ) + + +def _conditional_node(node_id: str, function: str = "check") -> NodeDefinition: + """Shorthand for creating a CONDITIONAL NodeDefinition.""" + return NodeDefinition( + id=node_id, + type=NodeType.CONDITIONAL, + name=node_id.title(), + description=f"Conditional {node_id}", + config={"function": function}, + ) + + +def _subgraph_node(node_id: str, actor_ref: str = "") -> NodeDefinition: + """Shorthand for creating a SUBGRAPH NodeDefinition.""" + return NodeDefinition( + id=node_id, + type=NodeType.SUBGRAPH, + name=node_id.title(), + description=f"Subgraph {node_id}", + config={"actor_ref": actor_ref}, + ) + + +# ──────────────────────────────────────────────────────────── +# Given steps — node type mapping +# ──────────────────────────────────────────────────────────── + + +@given("a GRAPH actor config containing AGENT, TOOL, CONDITIONAL, and SUBGRAPH nodes") +def step_given_all_four_node_types(context: Context) -> None: + nodes = [ + _agent_node("agent_node", agent="a1"), + _tool_node("tool_node"), + _conditional_node("cond_node", function="fn"), + _subgraph_node("sub_node", actor_ref="workflows/inner"), + ] + edges = [ + EdgeDefinition(from_node="agent_node", to_node="tool_node"), + EdgeDefinition(from_node="tool_node", to_node="cond_node"), + EdgeDefinition(from_node="cond_node", to_node="sub_node"), + ] + context.actor_config = _build_graph_config( + "workflows/all-types", nodes, edges, "agent_node", ["sub_node"] + ) + + +@given('a GRAPH actor config with an agent node configured as "{agent_name}"') +def step_given_agent_config(context: Context, agent_name: str) -> None: + nodes = [_agent_node("agent_node", agent=agent_name)] + context.actor_config = _build_graph_config( + "workflows/agent-cfg", + nodes, + [], + "agent_node", + ["agent_node"], + ) + + +@given('a GRAPH actor config with a conditional node configured with function "{fn}"') +def step_given_conditional_config(context: Context, fn: str) -> None: + nodes = [ + _agent_node("start"), + _conditional_node("cond_node", function=fn), + ] + edges = [EdgeDefinition(from_node="start", to_node="cond_node")] + context.actor_config = _build_graph_config( + "workflows/cond-cfg", nodes, edges, "start", ["cond_node"] + ) + + +@given('a GRAPH actor config with a tool node configured with tools "{tools_csv}"') +def step_given_tool_config(context: Context, tools_csv: str) -> None: + tools = [t.strip() for t in tools_csv.split(",")] + nodes = [ + _agent_node("start"), + NodeDefinition( + id="tool_node", + type=NodeType.TOOL, + name="Tools", + description="Tool node", + config={"tool_name": "t/run", "tools": tools}, + ), + ] + edges = [EdgeDefinition(from_node="start", to_node="tool_node")] + context.actor_config = _build_graph_config( + "workflows/tool-cfg", nodes, edges, "start", ["tool_node"] + ) + + +@given('a GRAPH actor config with a subgraph node referencing actor "{actor_ref}"') +def step_given_subgraph_config(context: Context, actor_ref: str) -> None: + nodes = [ + _agent_node("start"), + _subgraph_node("sub_node", actor_ref=actor_ref), + ] + edges = [EdgeDefinition(from_node="start", to_node="sub_node")] + context.actor_config = _build_graph_config( + "workflows/sub-cfg", nodes, edges, "start", ["sub_node"] + ) + + +# ──────────────────────────────────────────────────────────── +# Given steps — edge mapping +# ──────────────────────────────────────────────────────────── + + +@given('a GRAPH actor config with an unconditional edge from "{src}" to "{tgt}"') +def step_given_unconditional_edge(context: Context, src: str, tgt: str) -> None: + nodes = [_agent_node(src), _agent_node(tgt)] + edges = [EdgeDefinition(from_node=src, to_node=tgt)] + context.actor_config = _build_graph_config( + "workflows/uncond-edge", nodes, edges, src, [tgt] + ) + + +@given( + 'a GRAPH actor config with a conditional edge from "{src}" to "{tgt}" ' + 'with condition "{cond}"' +) +def step_given_conditional_edge( + context: Context, src: str, tgt: str, cond: str +) -> None: + nodes = [_agent_node(src), _agent_node(tgt)] + edges = [EdgeDefinition(from_node=src, to_node=tgt, condition=cond)] + context.actor_config = _build_graph_config( + "workflows/cond-edge", nodes, edges, src, [tgt] + ) + + +@given( + 'a GRAPH actor config with an edge from "{src}" to "{tgt}" with priority {pri:d}' +) +def step_given_priority_edge(context: Context, src: str, tgt: str, pri: int) -> None: + nodes = [_agent_node(src), _agent_node(tgt)] + edges = [EdgeDefinition(from_node=src, to_node=tgt, priority=pri)] + context.actor_config = _build_graph_config( + "workflows/pri-edge", nodes, edges, src, [tgt] + ) + + +# ──────────────────────────────────────────────────────────── +# Given steps — LSP binding edge cases +# ──────────────────────────────────────────────────────────── + + +def _single_node_with_lsp(lsp_bindings: Any) -> ActorConfigSchema: + """Build a config with one agent node carrying arbitrary lsp_bindings.""" + nodes = [ + NodeDefinition( + id="node_a", + type=NodeType.AGENT, + name="A", + description="Agent A", + config={"agent": "a", "lsp_bindings": lsp_bindings}, + ), + ] + return _build_graph_config("workflows/lsp-test", nodes, [], "node_a", ["node_a"]) + + +@given("a GRAPH actor config where lsp_bindings is a string instead of a list") +def step_given_lsp_not_list(context: Context) -> None: + context.actor_config = _single_node_with_lsp("not_a_list") + + +@given("a GRAPH actor config where lsp_bindings contains a non-dict entry") +def step_given_lsp_non_dict(context: Context) -> None: + context.actor_config = _single_node_with_lsp(["not_a_dict", 42]) + + +@given("a GRAPH actor config where an LSP binding has an empty server name") +def step_given_lsp_empty_server(context: Context) -> None: + context.actor_config = _single_node_with_lsp( + [{"lsp_server_name": "", "languages": ["python"]}] + ) + + +@given("a GRAPH actor config where an LSP binding has a numeric server name") +def step_given_lsp_numeric_server(context: Context) -> None: + context.actor_config = _single_node_with_lsp( + [{"lsp_server_name": 999, "languages": ["python"]}] + ) + + +@given("a GRAPH actor config where an LSP binding is missing the server name key") +def step_given_lsp_missing_key(context: Context) -> None: + context.actor_config = _single_node_with_lsp( + [{"languages": ["python"], "auto_detect": True}] + ) + + +@given("a GRAPH actor config with two valid LSP bindings on one node") +def step_given_two_lsp_bindings(context: Context) -> None: + context.actor_config = _single_node_with_lsp( + [ + {"lsp_server_name": "local/pyright", "languages": ["python"]}, + { + "lsp_server_name": "local/ts", + "languages": ["typescript"], + "auto_detect": False, + }, + ] + ) + + +# ──────────────────────────────────────────────────────────── +# Given steps — subgraph cycle detection edge cases +# ──────────────────────────────────────────────────────────── + + +@given("a GRAPH actor config with a subgraph node that has an empty actor_ref") +def step_given_empty_actor_ref(context: Context) -> None: + nodes = [ + _agent_node("start"), + _subgraph_node("sub_node", actor_ref=""), + ] + edges = [EdgeDefinition(from_node="start", to_node="sub_node")] + context.actor_config = _build_graph_config( + "workflows/empty-ref", nodes, edges, "start", ["sub_node"] + ) + context.actor_resolver = lambda name: None + + +@given('a GRAPH actor config with a subgraph referencing actor name "{ref}"') +def step_given_subgraph_ref_generic(context: Context, ref: str) -> None: + nodes = [ + _agent_node("start"), + _subgraph_node("sub_node", actor_ref=ref), + ] + edges = [EdgeDefinition(from_node="start", to_node="sub_node")] + context.actor_config = _build_graph_config( + "workflows/ref-test", nodes, edges, "start", ["sub_node"] + ) + + +@given("a cycle-detection resolver that returns None for all names") +def step_given_resolver_none(context: Context) -> None: + context.actor_resolver = lambda name: None + + +@given('a cycle-detection resolver where "{ref}" is an LLM type actor') +def step_given_resolver_llm_type(context: Context, ref: str) -> None: + llm_actor = ActorConfigSchema( + name=ref, + type=ActorType.LLM, + description="An LLM actor", + model="gpt-4", + ) + context.actor_resolver = lambda name, _r=ref, _a=llm_actor: ( + _a if name == _r else None + ) + + +@given('a cycle-detection resolver where "{ref}" is a GRAPH actor with no route') +def step_given_resolver_no_route(context: Context, ref: str) -> None: + graph_no_route = ActorConfigSchema.model_construct( + name=ref, + type=ActorType.GRAPH, + description="Graph with no route", + model="gpt-4", + version="1.0", + tools=[], + memory=None, + context=None, + context_view=None, + system_prompt=None, + route=None, + env_vars={}, + ) + context.actor_resolver = lambda name, _r=ref, _a=graph_no_route: ( + _a if name == _r else None + ) + + +@given('a GRAPH actor config named "{outer}" with subgraph ref to "{inner}"') +def step_given_outer_ref_inner(context: Context, outer: str, inner: str) -> None: + nodes = [ + _agent_node("main"), + _subgraph_node("sub", actor_ref=inner), + ] + edges = [EdgeDefinition(from_node="main", to_node="sub")] + context.actor_config = _build_graph_config(outer, nodes, edges, "main", ["sub"]) + context._outer_name = outer + context._inner_name = inner + + +@given( + 'a cycle-detection resolver where "{mid}" references "{leaf}" ' + 'and "{leaf}" references "{back}"' +) +def step_given_deep_cycle_resolver( + context: Context, mid: str, leaf: str, back: str +) -> None: + # mid actor: has subgraph referencing leaf + mid_nodes = [_subgraph_node("sub_mid", actor_ref=leaf)] + mid_actor = _build_graph_config(mid, mid_nodes, [], "sub_mid", ["sub_mid"]) + + # leaf actor: has subgraph referencing back (completes the cycle) + leaf_nodes = [_subgraph_node("sub_leaf", actor_ref=back)] + leaf_actor = _build_graph_config(leaf, leaf_nodes, [], "sub_leaf", ["sub_leaf"]) + + actors = {mid: mid_actor, leaf: leaf_actor} + context.actor_resolver = lambda name, _m=actors: _m.get(name) + + +# ──────────────────────────────────────────────────────────── +# Given steps — compile_actor error paths (defensive checks) +# ──────────────────────────────────────────────────────────── + + +@given("a GRAPH actor config with a cyclic route that bypasses schema validation") +def step_given_intra_cycle(context: Context) -> None: + """Create a graph with A→B→A cycle via model_construct to skip pydantic checks.""" + nodes = [_agent_node("nodeA"), _agent_node("nodeB")] + edges = [ + EdgeDefinition(from_node="nodeA", to_node="nodeB"), + EdgeDefinition(from_node="nodeB", to_node="nodeA"), + ] + context.actor_config = _build_graph_config_raw( + "workflows/cycle-raw", nodes, edges, "nodeA", ["nodeB"] + ) + + +@given( + "a GRAPH actor config with an edge whose from_node does not exist in the node list" +) +def step_given_missing_from_node(context: Context) -> None: + nodes = [_agent_node("real_node")] + edges = [EdgeDefinition(from_node="ghost", to_node="real_node")] + context.actor_config = _build_graph_config_raw( + "workflows/missing-from", nodes, edges, "real_node", ["real_node"] + ) + context._patch_validate_references = True + + +@given( + "a GRAPH actor config with an edge whose to_node does not exist in the node list" +) +def step_given_missing_to_node(context: Context) -> None: + nodes = [_agent_node("real_node")] + edges = [EdgeDefinition(from_node="real_node", to_node="ghost")] + context.actor_config = _build_graph_config_raw( + "workflows/missing-to", nodes, edges, "real_node", ["real_node"] + ) + context._patch_validate_references = True + + +@given("a GRAPH actor config whose entry node does not match any compiled node") +def step_given_invalid_entry(context: Context) -> None: + nodes = [_agent_node("actual")] + context.actor_config = _build_graph_config_raw( + "workflows/bad-entry", nodes, [], "nonexistent_entry", ["actual"] + ) + context._patch_validate_references = True + + +@given("a GRAPH actor config whose exit node does not match any compiled node") +def step_given_invalid_exit(context: Context) -> None: + nodes = [_agent_node("actual")] + context.actor_config = _build_graph_config_raw( + "workflows/bad-exit", nodes, [], "actual", ["nonexistent_exit"] + ) + context._patch_validate_references = True + + +# ──────────────────────────────────────────────────────────── +# Given steps — model defaults +# ──────────────────────────────────────────────────────────── + + +@given("a freshly constructed CompilationMetadata with no arguments") +def step_given_default_metadata(context: Context) -> None: + context.metadata = CompilationMetadata() + + +@given('a GRAPH actor config named "{name}" with entry "{entry}"') +def step_given_named_config(context: Context, name: str, entry: str) -> None: + nodes = [_agent_node(entry), _agent_node("end")] + edges = [EdgeDefinition(from_node=entry, to_node="end")] + context.actor_config = _build_graph_config(name, nodes, edges, entry, ["end"]) + + +# ──────────────────────────────────────────────────────────── +# When steps +# ──────────────────────────────────────────────────────────── + + +@when("I compile the full-coverage actor config") +def step_when_compile(context: Context) -> None: + context.compile_error = None + needs_patch = getattr(context, "_patch_validate_references", False) + try: + if needs_patch: + with ( + patch.object(RouteDefinition, "validate_references", return_value=None), + patch.object(RouteDefinition, "detect_cycles", return_value=[]), + ): + context.compiled = compile_actor(context.actor_config) + else: + context.compiled = compile_actor(context.actor_config) + except Exception as exc: + context.compile_error = exc + context.compiled = None + + +@when("I compile the full-coverage actor config with a resolver") +def step_when_compile_with_resolver(context: Context) -> None: + resolver = getattr(context, "actor_resolver", None) + context.compile_error = None + try: + context.compiled = compile_actor(context.actor_config, actor_resolver=resolver) + except Exception as exc: + context.compile_error = exc + context.compiled = None + + +# ──────────────────────────────────────────────────────────── +# Then steps — success +# ──────────────────────────────────────────────────────────── + + +@then("the compilation should succeed without errors") +def step_then_succeed(context: Context) -> None: + assert context.compile_error is None, ( + f"Expected compilation to succeed but got: {context.compile_error}" + ) + assert context.compiled is not None + + +# ──────────────────────────────────────────────────────────── +# Then steps — node type assertions +# ──────────────────────────────────────────────────────────── + + +@then('the compiled node "{node_id}" should have LangGraph type "{type_name}"') +def step_then_node_type(context: Context, node_id: str, type_name: str) -> None: + assert context.compiled is not None + node_cfg = context.compiled.nodes.get(node_id) + assert node_cfg is not None, f"Node '{node_id}' not in compiled nodes" + expected = lg_nodes.NodeType(type_name) + assert node_cfg.type == expected, f"Expected type {expected}, got {node_cfg.type}" + + +@then('the compiled node "{node_id}" should have agent set to "{agent}"') +def step_then_node_agent(context: Context, node_id: str, agent: str) -> None: + assert context.compiled is not None + node_cfg = context.compiled.nodes[node_id] + assert node_cfg.agent == agent, f"Expected agent={agent}, got {node_cfg.agent}" + + +@then('the compiled node "{node_id}" should have function set to "{fn}"') +def step_then_node_function(context: Context, node_id: str, fn: str) -> None: + assert context.compiled is not None + node_cfg = context.compiled.nodes[node_id] + assert node_cfg.function == fn, f"Expected function={fn}, got {node_cfg.function}" + + +@then('the compiled node "{node_id}" should have tools "{t1}" and "{t2}"') +def step_then_node_tools(context: Context, node_id: str, t1: str, t2: str) -> None: + assert context.compiled is not None + node_cfg = context.compiled.nodes[node_id] + assert t1 in node_cfg.tools, f"Expected {t1} in tools, got {node_cfg.tools}" + assert t2 in node_cfg.tools, f"Expected {t2} in tools, got {node_cfg.tools}" + + +@then('the compiled node "{node_id}" should have subgraph set to "{ref}"') +def step_then_node_subgraph(context: Context, node_id: str, ref: str) -> None: + assert context.compiled is not None + node_cfg = context.compiled.nodes[node_id] + assert node_cfg.subgraph == ref, f"Expected subgraph={ref}, got {node_cfg.subgraph}" + + +# ──────────────────────────────────────────────────────────── +# Then steps — edge assertions +# ──────────────────────────────────────────────────────────── + + +def _find_edge( + compiled: CompiledActor, source: str, target: str +) -> lg_nodes.Edge | None: + for e in compiled.edges: + if e.source == source and e.target == target: + return e + return None + + +@then('the compiled edge from "{src}" to "{tgt}" should have a null condition') +def step_then_edge_no_condition(context: Context, src: str, tgt: str) -> None: + assert context.compiled is not None + edge = _find_edge(context.compiled, src, tgt) + assert edge is not None, f"Edge {src}->{tgt} not found" + assert edge.condition is None, f"Expected None condition, got {edge.condition}" + + +@then( + 'the compiled edge from "{src}" to "{tgt}" should have condition expression "{expr}"' +) +def step_then_edge_condition(context: Context, src: str, tgt: str, expr: str) -> None: + assert context.compiled is not None + edge = _find_edge(context.compiled, src, tgt) + assert edge is not None, f"Edge {src}->{tgt} not found" + assert edge.condition is not None, "Expected condition dict, got None" + assert edge.condition.get("expression") == expr, ( + f"Expected expression='{expr}', got {edge.condition}" + ) + + +@then('the compiled edge from "{src}" to "{tgt}" should have metadata priority {pri:d}') +def step_then_edge_priority(context: Context, src: str, tgt: str, pri: int) -> None: + assert context.compiled is not None + edge = _find_edge(context.compiled, src, tgt) + assert edge is not None, f"Edge {src}->{tgt} not found" + assert edge.metadata.get("priority") == pri, ( + f"Expected priority={pri}, got {edge.metadata}" + ) + + +# ──────────────────────────────────────────────────────────── +# Then steps — LSP binding assertions +# ──────────────────────────────────────────────────────────── + + +@then("the compilation metadata should have {count:d} LSP bindings") +def step_then_lsp_count(context: Context, count: int) -> None: + assert context.compiled is not None + actual = len(context.compiled.metadata.lsp_bindings) + assert actual == count, f"Expected {count} LSP bindings, got {actual}" + + +@then('the LSP binding servers should include "{s1}" and "{s2}"') +def step_then_lsp_servers(context: Context, s1: str, s2: str) -> None: + assert context.compiled is not None + servers = [b.lsp_server_name for b in context.compiled.metadata.lsp_bindings] + assert s1 in servers, f"{s1} not in {servers}" + assert s2 in servers, f"{s2} not in {servers}" + + +# ──────────────────────────────────────────────────────────── +# Then steps — error assertions +# ──────────────────────────────────────────────────────────── + + +@then("the compilation should fail with a SubgraphCycleError") +def step_then_fail_cycle(context: Context) -> None: + assert context.compile_error is not None, "Expected SubgraphCycleError, got None" + assert isinstance(context.compile_error, SubgraphCycleError), ( + f"Expected SubgraphCycleError, got {type(context.compile_error).__name__}: " + f"{context.compile_error}" + ) + + +@then("the compilation should fail with a MissingNodeError") +def step_then_fail_missing(context: Context) -> None: + assert context.compile_error is not None, "Expected MissingNodeError, got None" + assert isinstance(context.compile_error, MissingNodeError), ( + f"Expected MissingNodeError, got {type(context.compile_error).__name__}: " + f"{context.compile_error}" + ) + + +@then("the compilation should fail with an InvalidEntryExitError") +def step_then_fail_entry_exit(context: Context) -> None: + assert context.compile_error is not None, "Expected InvalidEntryExitError, got None" + assert isinstance(context.compile_error, InvalidEntryExitError), ( + f"Expected InvalidEntryExitError, got " + f"{type(context.compile_error).__name__}: {context.compile_error}" + ) + + +@then('the compilation error message should mention "{text}"') +def step_then_error_contains(context: Context, text: str) -> None: + assert context.compile_error is not None + msg = str(context.compile_error) + assert text.lower() in msg.lower(), f"'{text}' not found in '{msg}'" + + +# ──────────────────────────────────────────────────────────── +# Then steps — model defaults +# ──────────────────────────────────────────────────────────── + + +@then("the metadata node_ids should be an empty list") +def step_then_empty_node_ids(context: Context) -> None: + assert context.metadata.node_ids == [] + + +@then("the metadata tool_nodes should be an empty list") +def step_then_empty_tool_nodes(context: Context) -> None: + assert context.metadata.tool_nodes == [] + + +@then("the metadata lsp_bindings should be an empty list") +def step_then_empty_lsp_bindings(context: Context) -> None: + assert context.metadata.lsp_bindings == [] + + +@then("the metadata subgraph_refs should be an empty dict") +def step_then_empty_subgraph_refs(context: Context) -> None: + assert context.metadata.subgraph_refs == {} + + +@then("the metadata entry_node should be an empty string") +def step_then_empty_entry(context: Context) -> None: + assert context.metadata.entry_node == "" + + +@then("the metadata exit_nodes should be an empty list") +def step_then_empty_exit_nodes(context: Context) -> None: + assert context.metadata.exit_nodes == [] + + +# ──────────────────────────────────────────────────────────── +# Then steps — result bundle +# ──────────────────────────────────────────────────────────── + + +@then('the compiled actor name should be "{name}"') +def step_then_actor_name(context: Context, name: str) -> None: + assert context.compiled is not None + assert context.compiled.name == name, ( + f"Expected name='{name}', got '{context.compiled.name}'" + ) + + +@then('the compiled actor entry_point field should be "{entry}"') +def step_then_actor_entry(context: Context, entry: str) -> None: + assert context.compiled is not None + assert context.compiled.entry_point == entry, ( + f"Expected entry_point='{entry}', got '{context.compiled.entry_point}'" + ) diff --git a/features/steps/actor_compiler_steps.py b/features/steps/actor_compiler_steps.py new file mode 100644 index 0000000000..bd30e44484 --- /dev/null +++ b/features/steps/actor_compiler_steps.py @@ -0,0 +1,536 @@ +"""Step definitions for actor compiler feature tests. + +Tests for features/actor_compiler.feature — validates that the actor +compiler correctly translates GRAPH-type ActorConfigSchema definitions +into LangGraph node/edge structures, detects cycles, and produces +compilation metadata. +""" + +from __future__ import annotations + +import json + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.actor.compiler import ( + ActorCompilationError, + SubgraphCycleError, + compile_actor, +) +from cleveragents.actor.schema import ( + ActorConfigSchema, + ActorType, + EdgeDefinition, + NodeDefinition, + NodeType, + RouteDefinition, +) + +# ──────────────────────────────────────────────────────────── +# Helper builders +# ──────────────────────────────────────────────────────────── + + +def _build_graph_config( + name: str, + nodes: list[NodeDefinition], + edges: list[EdgeDefinition], + entry_node: str, + exit_nodes: list[str], +) -> ActorConfigSchema: + route = RouteDefinition( + nodes=nodes, + edges=edges, + entry_node=entry_node, + exit_nodes=exit_nodes, + ) + return ActorConfigSchema( + name=name, + type=ActorType.GRAPH, + description="Test graph actor", + model="gpt-4", + route=route, + ) + + +# ──────────────────────────────────────────────────────────── +# Given steps +# ──────────────────────────────────────────────────────────── + + +@given("a GRAPH actor config with a linear two-node topology") +def step_given_linear_two_node(context: Context) -> None: + nodes = [ + NodeDefinition( + id="planner", + type=NodeType.AGENT, + name="Planner", + description="Plans tasks", + config={"agent": "planner_agent"}, + ), + NodeDefinition( + id="executor", + type=NodeType.TOOL, + name="Executor", + description="Runs tasks", + config={"tool_name": "exec/run"}, + ), + ] + edges = [EdgeDefinition(from_node="planner", to_node="executor")] + context.actor_config = _build_graph_config( + "workflows/simple", nodes, edges, "planner", ["executor"] + ) + + +@given("a GRAPH actor config with an agent and a tool node") +def step_given_agent_and_tool(context: Context) -> None: + nodes = [ + NodeDefinition( + id="writer", + type=NodeType.AGENT, + name="Writer", + description="Writes code", + config={"agent": "writer_agent"}, + ), + NodeDefinition( + id="linter", + type=NodeType.TOOL, + name="Linter", + description="Lints code", + config={"tool_name": "lint/run"}, + ), + ] + edges = [EdgeDefinition(from_node="writer", to_node="linter")] + context.actor_config = _build_graph_config( + "workflows/lint", nodes, edges, "writer", ["linter"] + ) + + +@given("a GRAPH actor config with LSP bindings on the agent node") +def step_given_lsp_bindings(context: Context) -> None: + nodes = [ + NodeDefinition( + id="coder", + type=NodeType.AGENT, + name="Coder", + description="Writes code", + config={ + "agent": "coder_agent", + "lsp_bindings": [ + { + "lsp_server_name": "local/pyright", + "languages": ["python"], + "auto_detect": True, + } + ], + }, + ), + ] + context.actor_config = _build_graph_config( + "workflows/lsp", nodes, [], "coder", ["coder"] + ) + + +@given("a GRAPH actor config with a conditional routing node") +def step_given_conditional(context: Context) -> None: + nodes = [ + NodeDefinition( + id="start_node", + type=NodeType.AGENT, + name="Agent", + description="Starts", + config={"agent": "a"}, + ), + NodeDefinition( + id="checker", + type=NodeType.CONDITIONAL, + name="Checker", + description="Checks result", + config={"function": "check_fn"}, + ), + NodeDefinition( + id="end_node", + type=NodeType.AGENT, + name="Finisher", + description="Ends", + config={"agent": "b"}, + ), + ] + edges = [ + EdgeDefinition(from_node="start_node", to_node="checker"), + EdgeDefinition( + from_node="checker", + to_node="end_node", + condition="state.get('ok')", + ), + ] + context.actor_config = _build_graph_config( + "workflows/cond", nodes, edges, "start_node", ["end_node"] + ) + + +@given('a GRAPH actor config with a subgraph node referencing "{ref_name}"') +def step_given_subgraph_ref(context: Context, ref_name: str) -> None: + nodes = [ + NodeDefinition( + id="main", + type=NodeType.AGENT, + name="Main", + description="Main agent", + config={"agent": "main_agent"}, + ), + NodeDefinition( + id="sub", + type=NodeType.SUBGRAPH, + name="Sub", + description="Subgraph", + config={"actor_ref": ref_name}, + ), + ] + edges = [EdgeDefinition(from_node="main", to_node="sub")] + context.actor_config = _build_graph_config( + "workflows/outer", nodes, edges, "main", ["sub"] + ) + inner_nodes = [ + NodeDefinition( + id="inner_agent", + type=NodeType.AGENT, + name="Inner", + description="Inner agent", + config={"agent": "inner"}, + ), + ] + inner = _build_graph_config( + ref_name, inner_nodes, [], "inner_agent", ["inner_agent"] + ) + context.actor_resolver = lambda name: inner if name == ref_name else None + + +@given("a GRAPH actor config with a single node") +def step_given_single_node(context: Context) -> None: + nodes = [ + NodeDefinition( + id="solo", + type=NodeType.AGENT, + name="Solo", + description="Solo agent", + config={"agent": "solo"}, + ), + ] + context.actor_config = _build_graph_config( + "workflows/solo", nodes, [], "solo", ["solo"] + ) + + +@given('a GRAPH actor config "{outer_name}" referencing "{inner_name}"') +def step_given_outer_referencing_inner( + context: Context, outer_name: str, inner_name: str +) -> None: + nodes = [ + NodeDefinition( + id="main", + type=NodeType.AGENT, + name="Main", + description="Main agent", + config={"agent": "main_agent"}, + ), + NodeDefinition( + id="sub", + type=NodeType.SUBGRAPH, + name="Sub", + description="Subgraph", + config={"actor_ref": inner_name}, + ), + ] + edges = [EdgeDefinition(from_node="main", to_node="sub")] + context.actor_config = _build_graph_config( + outer_name, nodes, edges, "main", ["sub"] + ) + context.outer_name = outer_name + context.inner_name = inner_name + + +@given('a resolver where "{inner_name}" references "{back_ref}"') +def step_given_resolver_cycle(context: Context, inner_name: str, back_ref: str) -> None: + inner_nodes = [ + NodeDefinition( + id="child", + type=NodeType.SUBGRAPH, + name="Child", + description="Back-ref", + config={"actor_ref": back_ref}, + ), + ] + inner = _build_graph_config(inner_name, inner_nodes, [], "child", ["child"]) + + def resolver(name: str) -> ActorConfigSchema | None: + if name == inner_name: + return inner + return None + + context.actor_resolver = resolver + + +@given('a resolver where "{inner_name}" has no subgraph nodes') +def step_given_resolver_no_subgraph(context: Context, inner_name: str) -> None: + inner_nodes = [ + NodeDefinition( + id="leaf", + type=NodeType.AGENT, + name="Leaf", + description="Leaf agent", + config={"agent": "leaf"}, + ), + ] + inner = _build_graph_config(inner_name, inner_nodes, [], "leaf", ["leaf"]) + + def resolver(name: str) -> ActorConfigSchema | None: + if name == inner_name: + return inner + return None + + context.actor_resolver = resolver + + +@given("an LLM actor config") +def step_given_llm_config(context: Context) -> None: + context.actor_config = ActorConfigSchema( + name="assistants/simple", + type=ActorType.LLM, + description="Simple LLM", + model="gpt-4", + ) + + +@given("a GRAPH actor config with route set to None") +def step_given_graph_no_route(context: Context) -> None: + context.actor_config = ActorConfigSchema.model_construct( + name="workflows/broken", + type=ActorType.GRAPH, + description="No route", + model="gpt-4", + version="1.0", + tools=[], + memory=None, + context=None, + context_view=None, + system_prompt=None, + route=None, + env_vars={}, + ) + + +@given('a GRAPH actor config with nodes "alpha", "beta", and "gamma"') +def step_given_three_nodes(context: Context) -> None: + nodes = [ + NodeDefinition( + id="gamma", + type=NodeType.AGENT, + name="Gamma", + description="G", + config={"agent": "g"}, + ), + NodeDefinition( + id="alpha", + type=NodeType.AGENT, + name="Alpha", + description="A", + config={"agent": "a"}, + ), + NodeDefinition( + id="beta", + type=NodeType.TOOL, + name="Beta", + description="B", + config={"tool_name": "b/run"}, + ), + ] + edges = [ + EdgeDefinition(from_node="alpha", to_node="beta"), + EdgeDefinition(from_node="beta", to_node="gamma"), + ] + context.actor_config = _build_graph_config( + "workflows/tri", nodes, edges, "alpha", ["gamma"] + ) + + +@given('a GRAPH actor config with entry "{entry}" and exit "{exit_node}"') +def step_given_entry_exit(context: Context, entry: str, exit_node: str) -> None: + nodes = [ + NodeDefinition( + id=entry, + type=NodeType.AGENT, + name="Start", + description="Start", + config={"agent": "s"}, + ), + NodeDefinition( + id=exit_node, + type=NodeType.AGENT, + name="End", + description="End", + config={"agent": "e"}, + ), + ] + edges = [EdgeDefinition(from_node=entry, to_node=exit_node)] + context.actor_config = _build_graph_config( + "workflows/ee", nodes, edges, entry, [exit_node] + ) + + +# ──────────────────────────────────────────────────────────── +# When steps +# ──────────────────────────────────────────────────────────── + + +@when("I compile the actor config") +def step_when_compile(context: Context) -> None: + context.compile_error = None + try: + context.compiled = compile_actor(context.actor_config) + except Exception as exc: + context.compile_error = exc + context.compiled = None + + +@when("I compile the actor config with a resolver") +def step_when_compile_with_resolver(context: Context) -> None: + resolver = getattr(context, "actor_resolver", None) + context.compile_error = None + try: + context.compiled = compile_actor(context.actor_config, actor_resolver=resolver) + except Exception as exc: + context.compile_error = exc + context.compiled = None + + +@when("I attempt to compile the non-graph actor") +def step_when_compile_non_graph(context: Context) -> None: + context.compile_error = None + try: + context.compiled = compile_actor(context.actor_config) + except Exception as exc: + context.compile_error = exc + context.compiled = None + + +@when("I attempt to compile the routeless actor") +def step_when_compile_routeless(context: Context) -> None: + context.compile_error = None + try: + context.compiled = compile_actor(context.actor_config) + except Exception as exc: + context.compile_error = exc + context.compiled = None + + +# ──────────────────────────────────────────────────────────── +# Then steps +# ──────────────────────────────────────────────────────────── + + +@then("the compilation should succeed") +def step_then_succeed(context: Context) -> None: + assert context.compile_error is None, ( + f"Expected compilation to succeed but got: {context.compile_error}" + ) + assert context.compiled is not None + + +@then("the compiled actor should have {count:d} nodes") +def step_then_node_count(context: Context, count: int) -> None: + assert context.compiled is not None + assert len(context.compiled.nodes) == count, ( + f"Expected {count} nodes, got {len(context.compiled.nodes)}" + ) + + +@then('the compiled actor entry point should be "{entry}"') +def step_then_entry_point(context: Context, entry: str) -> None: + assert context.compiled is not None + assert context.compiled.entry_point == entry + + +@then('the compilation metadata should list node IDs "{id1}" and "{id2}"') +def step_then_metadata_node_ids(context: Context, id1: str, id2: str) -> None: + assert context.compiled is not None + node_ids = context.compiled.metadata.node_ids + assert id1 in node_ids, f"{id1} not in {node_ids}" + assert id2 in node_ids, f"{id2} not in {node_ids}" + + +@then("the compilation metadata should have {count:d} tool node") +def step_then_tool_node_count(context: Context, count: int) -> None: + assert context.compiled is not None + assert len(context.compiled.metadata.tool_nodes) == count + + +@then("the compilation metadata should have {count:d} LSP binding") +def step_then_lsp_binding_count(context: Context, count: int) -> None: + assert context.compiled is not None + assert len(context.compiled.metadata.lsp_bindings) == count + + +@then('the LSP binding should reference server "{server}"') +def step_then_lsp_server(context: Context, server: str) -> None: + assert context.compiled is not None + servers = [b.lsp_server_name for b in context.compiled.metadata.lsp_bindings] + assert server in servers, f"{server} not in {servers}" + + +@then('the compilation metadata subgraph refs should map "{node}" to "{ref}"') +def step_then_subgraph_ref(context: Context, node: str, ref: str) -> None: + assert context.compiled is not None + refs = context.compiled.metadata.subgraph_refs + assert refs.get(node) == ref, f"Expected {node}->{ref}, got {refs}" + + +@then("the compilation should fail with SubgraphCycleError") +def step_then_fail_cycle(context: Context) -> None: + assert context.compile_error is not None + assert isinstance(context.compile_error, SubgraphCycleError), ( + f"Expected SubgraphCycleError, got {type(context.compile_error).__name__}" + ) + + +@then("the compilation should fail with ActorCompilationError") +def step_then_fail_compilation(context: Context) -> None: + assert context.compile_error is not None + assert isinstance(context.compile_error, ActorCompilationError), ( + f"Expected ActorCompilationError, got {type(context.compile_error).__name__}" + ) + + +@then('the compiler error message should contain "{text}"') +def step_then_compiler_error_contains(context: Context, text: str) -> None: + assert context.compile_error is not None + msg = str(context.compile_error) + assert text.lower() in msg.lower(), f"'{text}' not in '{msg}'" + + +@then("the metadata node IDs should be sorted alphabetically") +def step_then_sorted_ids(context: Context) -> None: + assert context.compiled is not None + ids = context.compiled.metadata.node_ids + assert ids == sorted(ids), f"Node IDs not sorted: {ids}" + + +@then('the metadata entry node should be "{entry}"') +def step_then_metadata_entry(context: Context, entry: str) -> None: + assert context.compiled is not None + assert context.compiled.metadata.entry_node == entry + + +@then('the metadata exit nodes should contain "{exit_node}"') +def step_then_metadata_exit(context: Context, exit_node: str) -> None: + assert context.compiled is not None + assert exit_node in context.compiled.metadata.exit_nodes + + +@then("the compilation metadata should be JSON-serializable") +def step_then_json_serializable(context: Context) -> None: + assert context.compiled is not None + data = context.compiled.metadata.model_dump(mode="json") + serialized = json.dumps(data) + assert isinstance(serialized, str) diff --git a/features/steps/skill_discovery_steps.py b/features/steps/skill_discovery_steps.py new file mode 100644 index 0000000000..d11a2dbaa0 --- /dev/null +++ b/features/steps/skill_discovery_steps.py @@ -0,0 +1,573 @@ +"""Step definitions for Agent Skills discovery tests.""" + +from __future__ import annotations + +import shutil +import tempfile +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.application.services.config_service import ConfigService +from cleveragents.skills.discovery import ( + DiscoveredAgentSkill, + build_tool_spec, + discover_agent_skills, + parse_agent_skills_paths, + register_discovered_skills, + scan_directory, +) +from cleveragents.tool.registry import ToolRegistry +from cleveragents.tool.runtime import ToolSpec + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _create_skill_md(folder: Path, name: str, description: str) -> None: + """Create a SKILL.md with YAML front-matter in *folder*.""" + folder.mkdir(parents=True, exist_ok=True) + content = ( + "---\n" + f"name: {name}\n" + f"description: {description}\n" + "---\n" + f"\n# {name}\n\n" + f"{description}\n" + ) + (folder / "SKILL.md").write_text(content, encoding="utf-8") + + +def _noop_handler(**_kwargs: Any) -> dict[str, Any]: + return {"status": "test"} + + +# --------------------------------------------------------------------------- +# Background +# --------------------------------------------------------------------------- + + +@given("the agent skills discovery module is available") +def step_discovery_module_available(context: Context) -> None: + """Verify imports are accessible.""" + assert parse_agent_skills_paths is not None + assert scan_directory is not None + assert discover_agent_skills is not None + + +# --------------------------------------------------------------------------- +# Config key +# --------------------------------------------------------------------------- + + +@when('I check the config registry for "{key}"') +def step_check_config_registry(context: Context, key: str) -> None: + context.config_entry = ConfigService.get_entry(key) + + +@then("the config entry should exist") +def step_config_entry_exists(context: Context) -> None: + assert context.config_entry is not None, "Config entry not found" + + +@then('the config entry type should be "{type_name}"') +def step_config_entry_type(context: Context, type_name: str) -> None: + entry = context.config_entry + assert entry is not None + assert entry.python_type.__name__ == type_name + + +@then('the config entry env var should be "{env_var}"') +def step_config_entry_env_var(context: Context, env_var: str) -> None: + entry = context.config_entry + assert entry is not None + assert entry.env_var == env_var + + +# --------------------------------------------------------------------------- +# Path parsing +# --------------------------------------------------------------------------- + + +@when('I parse agent skills paths "{raw_paths}"') +def step_parse_paths(context: Context, raw_paths: str) -> None: + context.parsed_paths = parse_agent_skills_paths(raw_paths) + + +@when("I parse an empty agent skills path string") +def step_parse_empty_paths(context: Context) -> None: + context.parsed_paths = parse_agent_skills_paths("") + + +@then("the result should contain {count:d} path") +def step_path_count_singular(context: Context, count: int) -> None: + assert len(context.parsed_paths) == count + + +@then("the result should contain {count:d} paths") +def step_path_count(context: Context, count: int) -> None: + assert len(context.parsed_paths) == count + + +# --------------------------------------------------------------------------- +# Discovery from directories +# --------------------------------------------------------------------------- + + +@given("a temporary directory with agent skill folders") +def step_temp_dir_with_skills(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + + assert context.table is not None, "Expected table data" + for row in context.table: + folder_name = row["folder_name"] + skill_name = row["skill_name"] + description = row["description"] + if skill_name == "_none_": + # Write SKILL.md without a name field + folder = tmpdir / folder_name + folder.mkdir(parents=True, exist_ok=True) + content = f"---\ndescription: {description}\n---\n\n# {folder_name}\n" + (folder / "SKILL.md").write_text(content, encoding="utf-8") + else: + _create_skill_md(tmpdir / folder_name, skill_name, description) + + +@given("a temporary directory with some folders missing SKILL.md") +def step_temp_dir_no_skill_md(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + # Create folder without SKILL.md + (tmpdir / "no-skill-folder").mkdir() + # Create a plain file (not a folder) + (tmpdir / "plain-file.txt").write_text("not a skill", encoding="utf-8") + + +@given("a temporary directory with a SKILL.md that has no front-matter") +def step_temp_dir_no_front_matter(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + folder = tmpdir / "bad-skill" + folder.mkdir() + (folder / "SKILL.md").write_text("# No front-matter here\n", encoding="utf-8") + + +@when("I scan the directory for agent skills") +def step_scan_directory(context: Context) -> None: + context.discovered = scan_directory(context.tmpdir) + + +@then("I should discover {count:d} agent skill") +def step_discover_count_singular(context: Context, count: int) -> None: + assert len(context.discovered) == count, ( + f"Expected {count}, got {len(context.discovered)}" + ) + + +@then("I should discover {count:d} agent skills") +def step_discover_count(context: Context, count: int) -> None: + assert len(context.discovered) == count, ( + f"Expected {count}, got {len(context.discovered)}" + ) + + +@then('the discovered skill "{name}" should have description "{description}"') +def step_discovered_skill_description( + context: Context, name: str, description: str +) -> None: + skill = next((s for s in context.discovered if s.name == name), None) + assert skill is not None, f"Skill '{name}' not found" + assert skill.description == description + + +@then("the discovered skill should use folder name as name") +def step_discovered_skill_folder_name(context: Context) -> None: + assert len(context.discovered) == 1 + skill = context.discovered[0] + # When no name in front-matter, it falls back to folder name + assert skill.name == "fallback-tool" + + +# --------------------------------------------------------------------------- +# Non-existent directory +# --------------------------------------------------------------------------- + + +@when("I run discovery on a non-existent directory") +def step_discover_nonexistent(context: Context) -> None: + context.discovery_result = discover_agent_skills( + [Path("/nonexistent/path/that/does/not/exist")] + ) + + +@then("the discovery result should have {count:d} discovered skills") +def step_discovery_result_count(context: Context, count: int) -> None: + assert len(context.discovery_result.discovered) == count + + +@then('the discovery result should have errors mentioning "{text}"') +def step_discovery_result_errors(context: Context, text: str) -> None: + errors_joined = " ".join(context.discovery_result.errors) + assert text in errors_joined, f"Expected '{text}' in errors, got: {errors_joined}" + + +# --------------------------------------------------------------------------- +# ToolSpec building +# --------------------------------------------------------------------------- + + +@given('a discovered agent skill named "{name}" at "{path}"') +def step_given_discovered_skill(context: Context, name: str, path: str) -> None: + context.discovered_skill = DiscoveredAgentSkill( + name=name, + description=f"Test skill {name}", + path=path, + ) + + +@when("I build a ToolSpec from the discovered skill") +def step_build_tool_spec(context: Context) -> None: + context.tool_spec = build_tool_spec(context.discovered_skill) + + +@then('the ToolSpec name should be "{expected}"') +def step_tool_spec_name(context: Context, expected: str) -> None: + assert context.tool_spec.name == expected + + +@then('the ToolSpec source should be "{expected}"') +def step_tool_spec_source(context: Context, expected: str) -> None: + assert context.tool_spec.source == expected + + +@then('the ToolSpec source_metadata should contain path "{expected}"') +def step_tool_spec_source_metadata_path(context: Context, expected: str) -> None: + assert context.tool_spec.source_metadata.get("path") == expected + + +# --------------------------------------------------------------------------- +# Registration +# --------------------------------------------------------------------------- + + +@given("a ToolRegistry with no existing tools") +def step_empty_tool_registry(context: Context) -> None: + context.tool_registry = ToolRegistry() + + +@given("a list of {count:d} discovered agent skills") +def step_discovered_skills_list(context: Context, count: int) -> None: + context.discovered_skills = [ + DiscoveredAgentSkill( + name=f"tool-{i}", + description=f"Test tool {i}", + path=f"/tmp/skills/tool-{i}", + ) + for i in range(count) + ] + + +@given('a ToolRegistry with an existing tool "{name}"') +def step_registry_with_tool(context: Context, name: str) -> None: + context.tool_registry = ToolRegistry() + existing_spec = ToolSpec( + name=name, + description="Existing tool", + handler=_noop_handler, + ) + context.tool_registry.register(existing_spec) + + +@given('a discovered agent skill named "{name}"') +def step_single_discovered_skill(context: Context, name: str) -> None: + context.discovered_skills = [ + DiscoveredAgentSkill( + name=name, + description=f"Discovered {name}", + path=f"/tmp/skills/{name}", + ) + ] + + +@when('I register discovered skills with "{strategy}" conflict strategy') +def step_register_discovered(context: Context, strategy: str) -> None: + try: + registered, conflicts = register_discovered_skills( + context.discovered_skills, + context.tool_registry, + on_conflict=strategy, + ) + context.registered_tools = registered + context.registration_conflicts = conflicts + context.registration_error = None + except ValueError as exc: + context.registration_error = exc + context.registered_tools = [] + context.registration_conflicts = [] + + +@then("{count:d} tools should be registered in the ToolRegistry") +def step_registered_tool_count(context: Context, count: int) -> None: + assert len(context.registered_tools) == count, ( + f"Expected {count}, got {len(context.registered_tools)}" + ) + + +@then("{count:d} tool should be registered in the ToolRegistry") +def step_registered_tool_count_singular(context: Context, count: int) -> None: + assert len(context.registered_tools) == count, ( + f"Expected {count}, got {len(context.registered_tools)}" + ) + + +@then("{count:d} conflicts should be reported") +def step_conflict_count(context: Context, count: int) -> None: + assert len(context.registration_conflicts) == count + + +@then('{count:d} conflict should be reported with tool name "{name}"') +def step_conflict_with_name(context: Context, count: int, name: str) -> None: + assert len(context.registration_conflicts) == count + if count > 0: + assert context.registration_conflicts[0].tool_name == name + + +@then('a discovery ValueError should be raised mentioning "{text}"') +def step_value_error_raised_discovery(context: Context, text: str) -> None: + assert context.registration_error is not None, "Expected ValueError not raised" + assert text in str(context.registration_error), ( + f"Expected '{text}' in error message, got: {context.registration_error}" + ) + + +@then('the tool "{name}" should have source "{expected_source}"') +def step_tool_has_source(context: Context, name: str, expected_source: str) -> None: + spec = context.tool_registry.get(name) + assert spec is not None, f"Tool '{name}' not found in registry" + assert spec.source == expected_source + + +# --------------------------------------------------------------------------- +# Refresh +# --------------------------------------------------------------------------- + + +@given("a SkillRegistryService with a ToolRegistry") +def step_service_with_registry(context: Context) -> None: + from cleveragents.application.services.skill_registry_service import ( + SkillRegistryService, + ) + + context.tool_registry = ToolRegistry() + # Create a mock skill_repo since we only need discovery features + mock_repo = MagicMock() + context.skill_registry_service = SkillRegistryService( + skill_repo=mock_repo, + tool_registry=context.tool_registry, + ) + + +@given("agent skills paths pointing to a directory with {count:d} skills") +def step_paths_with_skills(context: Context, count: int) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + for i in range(count): + _create_skill_md(tmpdir / f"skill-{i}", f"skill-{i}", f"Skill {i}") + context.agent_skills_raw_paths = str(tmpdir) + + +@when("I call discover_and_register") +def step_call_discover_and_register(context: Context) -> None: + context.discovery_result = context.skill_registry_service.discover_and_register( + context.agent_skills_raw_paths + ) + + +@then("{count:d} agent skills should be registered") +def step_agent_skills_registered(context: Context, count: int) -> None: + registered = context.skill_registry_service.registered_agent_tools + assert len(registered) == count, f"Expected {count}, got {len(registered)}" + + +@when("the directory now has {count:d} skills and I call refresh_agent_skills") +def step_add_more_and_refresh(context: Context, count: int) -> None: + # Add more skills to the directory + existing = list(context.tmpdir.iterdir()) + existing_count = len([d for d in existing if d.is_dir()]) + for i in range(existing_count, count): + _create_skill_md(context.tmpdir / f"skill-{i}", f"skill-{i}", f"Skill {i}") + context.discovery_result = context.skill_registry_service.refresh_agent_skills( + context.agent_skills_raw_paths + ) + + +# --------------------------------------------------------------------------- +# Edge cases +# --------------------------------------------------------------------------- + + +@when("I register an empty discovered skills list") +def step_register_empty_list(context: Context) -> None: + registered, conflicts = register_discovered_skills( + [], + context.tool_registry, + ) + context.registered_tools = registered + context.registration_conflicts = conflicts + + +@then( + "the registration should return {spec_count:d} specs and {conflict_count:d} conflicts" +) +def step_registration_returns( + context: Context, spec_count: int, conflict_count: int +) -> None: + assert len(context.registered_tools) == spec_count + assert len(context.registration_conflicts) == conflict_count + + +@when("I scan a non-directory path for agent skills") +def step_scan_non_dir(context: Context) -> None: + tmpfile = Path(tempfile.mktemp(suffix=".txt")) + tmpfile.write_text("not a directory", encoding="utf-8") + context.add_cleanup(lambda: tmpfile.unlink(missing_ok=True)) + context.discovered = scan_directory(tmpfile) + + +@when("I run discovery on a path that is a file") +def step_discover_file_path(context: Context) -> None: + tmpfile = Path(tempfile.mktemp(suffix=".txt")) + tmpfile.write_text("not a directory", encoding="utf-8") + context.add_cleanup(lambda: tmpfile.unlink(missing_ok=True)) + context.discovery_result = discover_agent_skills([tmpfile]) + + +@given("a temporary directory with a SKILL.md that has invalid YAML") +def step_temp_dir_invalid_yaml(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + folder = tmpdir / "bad-yaml-skill" + folder.mkdir() + (folder / "SKILL.md").write_text( + "---\ninvalid: [yaml: {broken\n---\n", + encoding="utf-8", + ) + + +@given("a temporary directory with a SKILL.md that has empty front-matter") +def step_temp_dir_empty_fm(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + folder = tmpdir / "empty-fm-skill" + folder.mkdir() + (folder / "SKILL.md").write_text( + "---\n\n---\n\n# Empty FM\n", + encoding="utf-8", + ) + + +@given("a temporary directory with a SKILL.md that has non-dict YAML") +def step_temp_dir_non_dict_yaml(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + folder = tmpdir / "list-yaml-skill" + folder.mkdir() + (folder / "SKILL.md").write_text( + "---\n- item1\n- item2\n---\n\n# List YAML\n", + encoding="utf-8", + ) + + +@given("a temporary directory with a SKILL.md that has numeric description") +def step_temp_dir_numeric_desc(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + folder = tmpdir / "numeric-desc-skill" + folder.mkdir() + (folder / "SKILL.md").write_text( + "---\nname: numeric-skill\ndescription: 42\n---\n\n# Numeric\n", + encoding="utf-8", + ) + + +@given("a temporary directory with a SKILL.md that has empty description") +def step_temp_dir_empty_desc(context: Context) -> None: + tmpdir = Path(tempfile.mkdtemp()) + context.tmpdir = tmpdir + context.add_cleanup(lambda: shutil.rmtree(str(tmpdir), ignore_errors=True)) + folder = tmpdir / "empty-desc-skill" + folder.mkdir() + (folder / "SKILL.md").write_text( + '---\nname: empty-desc\ndescription: ""\n---\n\n# Empty Desc\n', + encoding="utf-8", + ) + + +@then('the first discovered skill description should contain "{text}"') +def step_first_skill_desc_contains(context: Context, text: str) -> None: + assert len(context.discovered) > 0, "No discovered skills" + desc = context.discovered[0].description + assert text in desc, f"Expected '{text}' in description, got: {desc}" + + +@when("I call the noop handler") +def step_call_noop_handler(context: Context) -> None: + from cleveragents.skills.discovery import _noop_handler + + context.noop_result = _noop_handler() + + +@then('the noop handler should return status "{expected}"') +def step_noop_result(context: Context, expected: str) -> None: + assert context.noop_result["status"] == expected + + +# --------------------------------------------------------------------------- +# Source metadata +# --------------------------------------------------------------------------- + + +@given('a registered skill "{name}" with an agent_skill source') +def step_registered_skill_with_agent_source(context: Context, name: str) -> None: + from cleveragents.application.services.skill_service import SkillService + from cleveragents.skills.schema import ( + SkillAgentFolderSchema, + SkillConfigSchema, + ) + + context.skill_service = SkillService() + config = SkillConfigSchema( + name=name, + description="Test skill with agent_skill source", + agent_skill_folders=[ + SkillAgentFolderSchema(path="/tmp/agent-skill-folder", name="test-agent") + ], + ) + context.skill_service.add_skill(config, config_path="/tmp/test.yaml") + context.skill_name = name + + +@when('I resolve tools for "{name}"') +def step_resolve_tools(context: Context, name: str) -> None: + skill, entries = context.skill_service.resolve_tools(name) + context.resolved_skill = skill + context.resolved_entries = entries + + +@then("the resolved tools should include agent_skill source entries") +def step_resolved_tools_agent_source(context: Context) -> None: + agent_entries = [e for e in context.resolved_entries if "agent_skill:" in e.name] + assert len(agent_entries) > 0, "Expected at least one agent_skill source entry" diff --git a/features/steps/skill_flatten_steps.py b/features/steps/skill_flatten_steps.py new file mode 100644 index 0000000000..a75694066f --- /dev/null +++ b/features/steps/skill_flatten_steps.py @@ -0,0 +1,506 @@ +"""Step definitions for skill flattening and capability summary tests. + +All step text patterns are prefixed with ``flatten`` to avoid collisions +with existing step definitions in the Behave step registry. +""" + +from __future__ import annotations + +from typing import Any + +from behave import given, then, when +from behave.runner import Context + +from cleveragents.domain.models.core.skill import ( + ResolvedToolEntry, + Skill, + SkillCapabilitySummary, + SkillInclude, + SkillInlineTool, + SkillResolver, +) +from cleveragents.domain.models.core.tool import ToolCapability, ToolSource +from cleveragents.skills.protocol import SkillDefinition, SkillMetadata +from cleveragents.skills.registry import SkillRegistry + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _ensure_flatten_registry(context: Context) -> dict[str, Skill]: + """Return (and lazily create) the flatten skill lookup dict.""" + if not hasattr(context, "flatten_registry"): + context.flatten_registry = {} + return context.flatten_registry # type: ignore[no-any-return] + + +def _make_skill_definition(skill: Skill) -> SkillDefinition: + """Build a ``SkillDefinition`` wrapping *skill* with default metadata.""" + resolver = SkillResolver() + resolved = resolver.resolve_tools(skill, {}) + meta = SkillMetadata.from_skill(skill, resolved=resolved) + return SkillDefinition( + skill=skill, + resolved_tools=resolved, + metadata=meta, + ) + + +# --------------------------------------------------------------------------- +# Given steps (all prefixed with "flatten") +# --------------------------------------------------------------------------- + + +@given('a flatten registry skill "{name}" with tool refs "{refs}"') +def flatten_given_skill_refs(context: Context, name: str, refs: str) -> None: + """Register a skill with tool refs in the flatten lookup.""" + registry = _ensure_flatten_registry(context) + ref_list = [r.strip() for r in refs.split(",") if r.strip()] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + tool_refs=ref_list, + ) + + +@given('a flatten registry skill "{name}" including "{inc}" with refs "{refs}"') +def flatten_given_skill_include_refs( + context: Context, name: str, inc: str, refs: str +) -> None: + """Register a skill with one include and own tool refs.""" + registry = _ensure_flatten_registry(context) + ref_list = [r.strip() for r in refs.split(",") if r.strip()] + includes = [SkillInclude(name=i.strip()) for i in inc.split(",") if i.strip()] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + tool_refs=ref_list, + includes=includes, + ) + + +@given('a flatten registry skill "{name}" with {count:d} inline tools') +def flatten_given_skill_inline(context: Context, name: str, count: int) -> None: + """Register a skill with *count* inline tools.""" + registry = _ensure_flatten_registry(context) + inlines = [ + SkillInlineTool( + description=f"Inline tool {i}", + source=ToolSource.CUSTOM, + code=f"return {i}", + ) + for i in range(count) + ] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + anonymous_tools=inlines, + ) + + +@given('a flatten registry skill "{name}" including "{inc}" with no refs') +def flatten_given_skill_include_no_refs(context: Context, name: str, inc: str) -> None: + """Register a skill that includes another with no own refs.""" + registry = _ensure_flatten_registry(context) + includes = [SkillInclude(name=i.strip()) for i in inc.split(",") if i.strip()] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + includes=includes, + ) + + +@given( + 'a flatten registry skill "{name}" including "{inc}"' + " with overrides timeout {timeout:d}" +) +def flatten_given_skill_include_overrides( + context: Context, name: str, inc: str, timeout: int +) -> None: + """Register a skill that includes another with per-include overrides.""" + registry = _ensure_flatten_registry(context) + includes = [ + SkillInclude( + name=inc.strip(), + overrides={"timeout": timeout}, + ) + ] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + includes=includes, + ) + + +@given( + 'a flatten registry skill "{name}" including "{inc}"' + ' with overrides on non-overridable field "{field}"' +) +def flatten_given_skill_include_bad_override( + context: Context, name: str, inc: str, field: str +) -> None: + """Register a skill whose include overrides a non-overridable field.""" + registry = _ensure_flatten_registry(context) + includes = [ + SkillInclude( + name=inc.strip(), + overrides={field: "forbidden-value"}, + ) + ] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + includes=includes, + ) + + +@given('a flatten registry skill "{name}" with mixed capability inline tools') +def flatten_given_skill_capability_mix(context: Context, name: str) -> None: + """Register a skill with inline tools having diverse capabilities.""" + registry = _ensure_flatten_registry(context) + inlines = [ + SkillInlineTool( + description="Read-only tool", + source=ToolSource.CUSTOM, + code="pass", + capability=ToolCapability(read_only=True), + ), + SkillInlineTool( + description="Write tool with side effects", + source=ToolSource.CUSTOM, + code="pass", + capability=ToolCapability( + writes=True, + side_effects=["filesystem"], + ), + ), + SkillInlineTool( + description="Checkpointable tool", + source=ToolSource.CUSTOM, + code="pass", + capability=ToolCapability(checkpointable=True), + ), + ] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + anonymous_tools=inlines, + ) + + +@given('a flatten skill registry with skill "{name}" having {count:d} refs') +def flatten_given_full_registry(context: Context, name: str, count: int) -> None: + """Create a full SkillRegistry and register a skill with *count* refs.""" + refs = [f"local/tool-{i}" for i in range(count)] + skill = Skill(name=name, description=f"Skill {name}", tool_refs=refs) + defn = _make_skill_definition(skill) + reg = SkillRegistry() + reg.register(defn) + context.flatten_skill_registry = reg + + +@given("an empty flatten skill registry") +def flatten_given_empty_registry(context: Context) -> None: + """Create an empty SkillRegistry.""" + context.flatten_skill_registry = SkillRegistry() + + +@given("the flatten registry skills are registered in the skill registry") +def flatten_register_all_in_skill_registry(context: Context) -> None: + """Register all skills from the flatten lookup into a SkillRegistry. + + Uses minimal metadata (no resolution) because the skill graph may + contain cycles that would fail resolution. + """ + registry = _ensure_flatten_registry(context) + sr = SkillRegistry() + for skill in registry.values(): + meta = SkillMetadata( + name=skill.name, + description=skill.description, + ) + defn = SkillDefinition( + skill=skill, + resolved_tools=[], + metadata=meta, + ) + sr.register(defn) + context.flatten_skill_registry = sr + + +@given( + 'a flatten registry skill "{name}" including "{inc}"' + " with overrides priority {prio:d} and extra key" +) +def flatten_given_skill_include_merge_overrides( + context: Context, name: str, inc: str, prio: int +) -> None: + """Register a skill whose include overrides have multiple keys.""" + registry = _ensure_flatten_registry(context) + includes = [ + SkillInclude( + name=inc.strip(), + overrides={"priority": prio, "extra": "yes"}, + ) + ] + registry[name] = Skill( + name=name, + description=f"Skill {name}", + includes=includes, + ) + + +@given( + 'a flatten registry skill "{name}" with tool refs "{refs}"' + " and overrides priority {prio:d}" +) +def flatten_given_skill_refs_with_overrides( + context: Context, name: str, refs: str, prio: int +) -> None: + """Register a skill with tool refs that have skill-level overrides.""" + registry = _ensure_flatten_registry(context) + ref_list = [r.strip() for r in refs.split(",") if r.strip()] + overrides: dict[str, dict[str, Any]] = {} + for ref in ref_list: + overrides[ref] = {"priority": prio} + registry[name] = Skill( + name=name, + description=f"Skill {name}", + tool_refs=ref_list, + overrides=overrides, + ) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when('I flatten the skill "{name}"') +def flatten_when_resolve(context: Context, name: str) -> None: + """Resolve/flatten a skill from the flatten registry.""" + registry = _ensure_flatten_registry(context) + skill = registry[name] + resolver = SkillResolver() + context.flatten_result = resolver.resolve_tools(skill, registry) + context.flatten_error = None + + +@when('I try to flatten the skill "{name}"') +def flatten_when_try_resolve(context: Context, name: str) -> None: + """Try to flatten; capture errors.""" + registry = _ensure_flatten_registry(context) + skill = registry[name] + resolver = SkillResolver() + context.flatten_result = None + context.flatten_error = None + try: + context.flatten_result = resolver.resolve_tools(skill, registry) + except ValueError as exc: + context.flatten_error = exc + + +@when('I flatten the skill "{name}" and compute summary') +def flatten_when_resolve_summary(context: Context, name: str) -> None: + """Flatten a skill and compute the capability summary.""" + registry = _ensure_flatten_registry(context) + skill = registry[name] + resolver = SkillResolver() + entries = resolver.resolve_tools(skill, registry) + context.flatten_result = entries + context.flatten_summary = resolver.compute_capability_summary(skill, entries) + + +@when('I call the flatten tools method for "{name}"') +def flatten_when_tools_method(context: Context, name: str) -> None: + """Call SkillRegistry.tools() on a registered skill.""" + sr: SkillRegistry = context.flatten_skill_registry + entries, summary = sr.tools(name) + context.flatten_tools_entries = entries + context.flatten_tools_summary = summary + + +@when('I call flatten validate_plan with skills "{skills_csv}"') +def flatten_when_validate_plan(context: Context, skills_csv: str) -> None: + """Call SkillRegistry.validate_plan().""" + sr: SkillRegistry = context.flatten_skill_registry + skill_list = [s.strip() for s in skills_csv.split(",") if s.strip()] + context.flatten_validation_errors = sr.validate_plan({"skills": skill_list}) + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then("the flatten result should have {count:d} entries") +def flatten_then_count(context: Context, count: int) -> None: + """Check flattened entry count.""" + actual = len(context.flatten_result) + assert actual == count, ( + f"Expected {count} entries, got {actual}: " + f"{[e.name for e in context.flatten_result]}" + ) + + +@then('the flatten entry at index {idx:d} should be "{expected}"') +def flatten_then_entry_at(context: Context, idx: int, expected: str) -> None: + """Check entry name at a given index.""" + entries: list[ResolvedToolEntry] = context.flatten_result + assert idx < len(entries), f"Index {idx} out of range ({len(entries)} entries)" + actual = entries[idx].name + assert actual == expected, ( + f"Expected entry at {idx} to be '{expected}', got '{actual}'" + ) + + +@then("the flatten entry at index {idx:d} should be marked inline") +def flatten_then_entry_inline(context: Context, idx: int) -> None: + """Check that entry at index is inline.""" + entries: list[ResolvedToolEntry] = context.flatten_result + assert entries[idx].is_inline is True, ( + f"Expected inline at index {idx}, got is_inline=False" + ) + + +@then("a flatten cycle error should be raised") +def flatten_then_cycle_error(context: Context) -> None: + """Verify a cycle error was raised.""" + assert context.flatten_error is not None, "Expected cycle error" + assert "ycle" in str(context.flatten_error), ( + f"Expected cycle error, got: {context.flatten_error}" + ) + + +@then('the flatten error should mention "{text}"') +def flatten_then_error_mention(context: Context, text: str) -> None: + """Check the error message contains text.""" + msg = str(context.flatten_error) + assert text in msg, f"Expected '{text}' in error: {msg}" + + +@then('the flatten entry "{name}" should have override "{key}" equal to {value:d}') +def flatten_then_override_int( + context: Context, name: str, key: str, value: int +) -> None: + """Check that a resolved entry has a specific int override.""" + entries: list[ResolvedToolEntry] = context.flatten_result + for entry in entries: + if entry.name == name: + assert key in entry.overrides, ( + f"Override '{key}' not found on '{name}': {entry.overrides}" + ) + assert entry.overrides[key] == value, ( + f"Expected override '{key}'={value}, got {entry.overrides[key]}" + ) + return + raise AssertionError(f"Entry '{name}' not found: {[e.name for e in entries]}") + + +@then('the flatten entry "{name}" should have override "{key}" equal to "{value}"') +def flatten_then_override_str( + context: Context, name: str, key: str, value: str +) -> None: + """Check that a resolved entry has a specific string override.""" + entries: list[ResolvedToolEntry] = context.flatten_result + for entry in entries: + if entry.name == name: + assert key in entry.overrides, ( + f"Override '{key}' not found on '{name}': {entry.overrides}" + ) + assert entry.overrides[key] == value, ( + f"Expected override '{key}'='{value}', got {entry.overrides[key]}" + ) + return + raise AssertionError(f"Entry '{name}' not found: {[e.name for e in entries]}") + + +@then("a flatten override error should be raised") +def flatten_then_override_error(context: Context) -> None: + """Verify a non-overridable field error was raised.""" + assert context.flatten_error is not None, ( + "Expected override error but no error was raised" + ) + + +@then("the flatten summary total_tools should be {expected:d}") +def flatten_then_summary_total(context: Context, expected: int) -> None: + """Check summary total_tools.""" + summary: SkillCapabilitySummary = context.flatten_summary + assert summary.total_tools == expected, ( + f"Expected total_tools={expected}, got {summary.total_tools}" + ) + + +@then("the flatten summary read_only_tools should be {expected:d}") +def flatten_then_summary_readonly(context: Context, expected: int) -> None: + """Check summary read_only_tools.""" + summary: SkillCapabilitySummary = context.flatten_summary + assert summary.read_only_tools == expected + + +@then("the flatten summary write_tools should be {expected:d}") +def flatten_then_summary_writes(context: Context, expected: int) -> None: + """Check summary write_tools.""" + summary: SkillCapabilitySummary = context.flatten_summary + assert summary.write_tools == expected + + +@then("the flatten summary has_side_effects should be true") +def flatten_then_summary_side_effects(context: Context) -> None: + """Check summary has_side_effects.""" + summary: SkillCapabilitySummary = context.flatten_summary + assert summary.has_side_effects is True + + +@then("the flatten summary checkpointable_tools should be {expected:d}") +def flatten_then_summary_checkpoint(context: Context, expected: int) -> None: + """Check summary checkpointable_tools.""" + summary: SkillCapabilitySummary = context.flatten_summary + assert summary.checkpointable_tools == expected + + +@then("the flatten tools result should contain {count:d} entries") +def flatten_then_tools_count(context: Context, count: int) -> None: + """Check tools() returned correct entry count.""" + actual = len(context.flatten_tools_entries) + assert actual == count, f"Expected {count} entries, got {actual}" + + +@then("the flatten tools result should include a capability summary") +def flatten_then_tools_summary(context: Context) -> None: + """Check tools() returned a SkillCapabilitySummary.""" + summary = context.flatten_tools_summary + assert isinstance(summary, SkillCapabilitySummary), ( + f"Expected SkillCapabilitySummary, got {type(summary)}" + ) + + +@then("the flatten validation result should have {count:d} errors") +def flatten_then_validation_count(context: Context, count: int) -> None: + """Check validate_plan error count.""" + actual = len(context.flatten_validation_errors) + assert actual == count, ( + f"Expected {count} errors, got {actual}: {context.flatten_validation_errors}" + ) + + +@then('the flatten validation error should mention "{text}"') +def flatten_then_validation_mention(context: Context, text: str) -> None: + """Check that at least one validation error mentions text.""" + errors: list[str] = context.flatten_validation_errors + assert any(text in e for e in errors), f"Expected '{text}' in errors: {errors}" + + +@then('the flatten entry "{name}" source_skill should be "{expected}"') +def flatten_then_entry_source(context: Context, name: str, expected: str) -> None: + """Check which skill contributed a tool entry.""" + entries: list[ResolvedToolEntry] = context.flatten_result + for entry in entries: + if entry.name == name: + assert entry.source_skill == expected, ( + f"Expected source_skill='{expected}', got '{entry.source_skill}'" + ) + return + raise AssertionError(f"Entry '{name}' not found: {[e.name for e in entries]}") diff --git a/robot/actor_compiler.robot b/robot/actor_compiler.robot new file mode 100644 index 0000000000..2052e00d78 --- /dev/null +++ b/robot/actor_compiler.robot @@ -0,0 +1,44 @@ +*** Settings *** +Documentation Smoke tests for the Actor Compiler (GRAPH → LangGraph) +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_actor_compiler.py + +*** Test Cases *** +Compile Simple Graph Actor YAML + [Documentation] Compile the simple_graph example and verify node/edge counts + [Tags] slow + ${result}= Run Process ${PYTHON} ${HELPER} compile ${CURDIR}/../examples/actors/simple_graph.yaml cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-compiler-ok + +Compile Graph Workflow Actor YAML + [Documentation] Compile the graph_workflow example and verify success + [Tags] slow + ${result}= Run Process ${PYTHON} ${HELPER} compile ${CURDIR}/../examples/actors/graph_workflow.yaml cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-compiler-ok + +Retrieve Compilation Metadata + [Documentation] Verify metadata output from the compiler contains node IDs + [Tags] slow + ${result}= Run Process ${PYTHON} ${HELPER} metadata ${CURDIR}/../examples/actors/simple_graph.yaml cwd=${WORKSPACE} + Log ${result.stdout} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-compiler-metadata + +Reject LLM Actor Compilation + [Documentation] Attempt to compile an LLM-type actor and expect failure + [Tags] slow + ${result}= Run Process ${PYTHON} ${HELPER} compile-fail ${CURDIR}/../examples/actors/simple_llm.yaml cwd=${WORKSPACE} + Log ${result.stdout} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} actor-compiler-expected-fail + Should Contain ${result.stdout} GRAPH diff --git a/robot/helper_actor_compiler.py b/robot/helper_actor_compiler.py new file mode 100644 index 0000000000..21a767224d --- /dev/null +++ b/robot/helper_actor_compiler.py @@ -0,0 +1,74 @@ +"""Robot Framework helper for actor compiler smoke tests. + +Provides a CLI interface for Robot to invoke the actor compiler on +YAML-defined GRAPH actors and inspect compilation metadata. + +Usage: + python robot/helper_actor_compiler.py compile + python robot/helper_actor_compiler.py compile-fail + python robot/helper_actor_compiler.py metadata +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from cleveragents.actor.compiler import compile_actor # noqa: E402 +from cleveragents.actor.schema import ActorConfigSchema # noqa: E402 + + +def main() -> int: + """Entry point called by Robot Framework ``Run Process``.""" + if len(sys.argv) < 3: + print("Usage: helper_actor_compiler.py ") + return 1 + + command = sys.argv[1] + yaml_path = sys.argv[2] + + if command == "compile": + try: + config = ActorConfigSchema.from_yaml_file(yaml_path) + compiled = compile_actor(config) + print(f"actor-compiler-ok: {compiled.name}") + print(f"nodes: {len(compiled.nodes)}") + print(f"edges: {len(compiled.edges)}") + print(f"entry: {compiled.entry_point}") + return 0 + except Exception as exc: + print(f"actor-compiler-fail: {exc}") + return 1 + + if command == "compile-fail": + try: + config = ActorConfigSchema.from_yaml_file(yaml_path) + compile_actor(config) + print("actor-compiler-unexpected-success") + return 1 + except Exception as exc: + print(f"actor-compiler-expected-fail: {exc}") + return 0 + + if command == "metadata": + try: + config = ActorConfigSchema.from_yaml_file(yaml_path) + compiled = compile_actor(config) + meta = compiled.metadata.model_dump(mode="json") + print(f"actor-compiler-metadata: {json.dumps(meta)}") + return 0 + except Exception as exc: + print(f"actor-compiler-fail: {exc}") + return 1 + + print(f"Unknown command: {command}") + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/robot/helper_skill_discovery.py b/robot/helper_skill_discovery.py new file mode 100644 index 0000000000..548e727345 --- /dev/null +++ b/robot/helper_skill_discovery.py @@ -0,0 +1,147 @@ +"""Robot Framework helper for Agent Skills discovery tests.""" + +from __future__ import annotations + +import shutil +import tempfile +from pathlib import Path +from typing import Any + +from cleveragents.application.services.config_service import ConfigService +from cleveragents.skills.discovery import ( + DiscoveredAgentSkill, + build_tool_spec, + register_discovered_skills, + scan_directory, +) +from cleveragents.skills.discovery import ( + parse_agent_skills_paths as _parse_paths, +) +from cleveragents.tool.registry import ToolRegistry +from cleveragents.tool.runtime import ToolSpec + + +def _noop_handler(**_kwargs: Any) -> dict[str, Any]: + return {"status": "placeholder"} + + +def _create_skill_md(folder: Path, name: str, description: str) -> None: + folder.mkdir(parents=True, exist_ok=True) + content = ( + "---\n" + f"name: {name}\n" + f"description: {description}\n" + "---\n" + f"\n# {name}\n\n{description}\n" + ) + (folder / "SKILL.md").write_text(content, encoding="utf-8") + + +# --------------------------------------------------------------------------- +# Robot keywords +# --------------------------------------------------------------------------- + + +def parse_agent_skills_paths(raw_value: str) -> list[str]: + """Parse comma-separated paths and return as string list.""" + paths = _parse_paths(raw_value) + return [str(p) for p in paths] + + +def create_agent_skills_directory(count: int) -> str: + """Create a temp directory with *count* agent skill folders.""" + tmpdir = Path(tempfile.mkdtemp()) + for i in range(int(count)): + _create_skill_md(tmpdir / f"skill-{i}", f"skill-{i}", f"Skill {i}") + return str(tmpdir) + + +def create_mixed_directory() -> str: + """Create a directory with one valid skill and one without SKILL.md.""" + tmpdir = Path(tempfile.mkdtemp()) + _create_skill_md(tmpdir / "valid-skill", "valid-skill", "A valid skill") + (tmpdir / "no-skill").mkdir() + return str(tmpdir) + + +def scan_agent_skills_directory(directory: str) -> list[dict[str, Any]]: + """Scan a directory and return discovered skills as dicts.""" + results = scan_directory(Path(directory)) + return [ + {"name": s.name, "description": s.description, "path": s.path} for s in results + ] + + +def build_agent_skill_toolspec(name: str, path: str) -> ToolSpec: + """Build a ToolSpec from a discovered agent skill.""" + skill = DiscoveredAgentSkill( + name=name, + description=f"Agent skill {name}", + path=path, + ) + return build_tool_spec(skill) + + +def register_agent_skills_in_fresh_registry( + count: int, +) -> dict[str, int]: + """Register discovered skills in a fresh ToolRegistry.""" + registry = ToolRegistry() + discovered = [ + DiscoveredAgentSkill( + name=f"tool-{i}", + description=f"Tool {i}", + path=f"/tmp/skills/tool-{i}", + ) + for i in range(int(count)) + ] + registered, conflicts = register_discovered_skills( + discovered, registry, on_conflict="skip" + ) + return { + "registered_count": len(registered), + "conflict_count": len(conflicts), + } + + +def register_agent_skills_with_collision( + strategy: str, +) -> dict[str, int]: + """Register a skill that collides with an existing tool.""" + registry = ToolRegistry() + existing = ToolSpec( + name="agent_skills/collider", + description="Existing tool", + handler=_noop_handler, + ) + registry.register(existing) + + discovered = [ + DiscoveredAgentSkill( + name="collider", + description="Conflicting skill", + path="/tmp/skills/collider", + ) + ] + + try: + registered, conflicts = register_discovered_skills( + discovered, registry, on_conflict=strategy + ) + except ValueError: + return {"registered_count": 0, "conflict_count": 1} + + return { + "registered_count": len(registered), + "conflict_count": len(conflicts), + } + + +def get_config_entry(key: str) -> Any: + """Get a config entry from the registry.""" + return ConfigService.get_entry(key) + + +def cleanup_temp_directory(path: str) -> None: + """Remove a temporary directory.""" + shutil.rmtree(path, ignore_errors=True) diff --git a/robot/helper_skill_flatten.py b/robot/helper_skill_flatten.py new file mode 100644 index 0000000000..67e053d0db --- /dev/null +++ b/robot/helper_skill_flatten.py @@ -0,0 +1,365 @@ +"""Robot Framework helper for skill flattening smoke tests. + +Provides a CLI-style interface for Robot to invoke skill flattening, +capability summary computation, validate_plan(), and the tools() method. + +Usage: + python robot/helper_skill_flatten.py flatten-simple + python robot/helper_skill_flatten.py flatten-deep + python robot/helper_skill_flatten.py flatten-wide + python robot/helper_skill_flatten.py flatten-inline + python robot/helper_skill_flatten.py flatten-cycle + python robot/helper_skill_flatten.py flatten-include-overrides + python robot/helper_skill_flatten.py flatten-non-overridable + python robot/helper_skill_flatten.py capability-summary + python robot/helper_skill_flatten.py tools-method + python robot/helper_skill_flatten.py validate-plan-ok + python robot/helper_skill_flatten.py validate-plan-missing +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +# Ensure the src directory is on the import path. +_SRC = str(Path(__file__).resolve().parents[1] / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from cleveragents.domain.models.core.skill import ( # noqa: E402 + Skill, + SkillInclude, + SkillInlineTool, + SkillResolver, +) +from cleveragents.domain.models.core.tool import ( # noqa: E402 + ToolCapability, + ToolSource, +) +from cleveragents.skills.protocol import ( # noqa: E402 + SkillDefinition, + SkillMetadata, +) +from cleveragents.skills.registry import SkillRegistry # noqa: E402 + + +def _make_defn(skill: Skill) -> SkillDefinition: + """Build a SkillDefinition from a Skill with default metadata.""" + resolver = SkillResolver() + resolved = resolver.resolve_tools(skill, {}) + meta = SkillMetadata.from_skill(skill, resolved=resolved) + return SkillDefinition(skill=skill, resolved_tools=resolved, metadata=meta) + + +def _flatten_simple() -> int: + """Flatten a skill with 5 tool refs.""" + skill = Skill( + name="local/simple", + description="Simple skill", + tool_refs=[f"local/tool-{i}" for i in range(5)], + ) + resolver = SkillResolver() + result = resolver.resolve_tools(skill, {}) + names = [e.name for e in result] + expected = [f"local/tool-{i}" for i in range(5)] + if names != expected: + print(f"flatten-fail: expected {expected}, got {names}") + return 1 + print(f"flatten-simple-ok: {json.dumps(names)}") + return 0 + + +def _flatten_deep() -> int: + """Flatten a skill with 3 levels of includes.""" + base = Skill(name="local/base", description="Base", tool_refs=["local/base-t"]) + mid = Skill( + name="local/mid", + description="Mid", + tool_refs=["local/mid-t"], + includes=[SkillInclude(name="local/base")], + ) + top = Skill( + name="local/top", + description="Top", + tool_refs=["local/top-t"], + includes=[SkillInclude(name="local/mid")], + ) + registry = { + "local/base": base, + "local/mid": mid, + "local/top": top, + } + resolver = SkillResolver() + result = resolver.resolve_tools(top, registry) + names = [e.name for e in result] + expected = ["local/base-t", "local/mid-t", "local/top-t"] + if names != expected: + print(f"flatten-fail: expected {expected}, got {names}") + return 1 + print(f"flatten-deep-ok: {json.dumps(names)}") + return 0 + + +def _flatten_wide() -> int: + """Flatten a skill that includes 10 other skills.""" + children: dict[str, Skill] = {} + includes: list[SkillInclude] = [] + for i in range(10): + name = f"local/child-{i}" + children[name] = Skill( + name=name, + description=f"Child {i}", + tool_refs=[f"local/child-{i}-tool"], + ) + includes.append(SkillInclude(name=name)) + top = Skill(name="local/wide", description="Wide", includes=includes) + registry = {**children, "local/wide": top} + resolver = SkillResolver() + result = resolver.resolve_tools(top, registry) + if len(result) != 10: + print(f"flatten-fail: expected 10 tools, got {len(result)}") + return 1 + print(f"flatten-wide-ok: {len(result)} tools") + return 0 + + +def _flatten_inline() -> int: + """Flatten a skill with inline tools.""" + skill = Skill( + name="local/inline", + description="Inline", + anonymous_tools=[ + SkillInlineTool( + description=f"Inline {i}", + source=ToolSource.CUSTOM, + code=f"return {i}", + ) + for i in range(3) + ], + ) + resolver = SkillResolver() + result = resolver.resolve_tools(skill, {}) + if len(result) != 3: + print(f"flatten-fail: expected 3, got {len(result)}") + return 1 + for i, entry in enumerate(result): + if not entry.is_inline: + print(f"flatten-fail: entry {i} not inline") + return 1 + print(f"flatten-inline-ok: {len(result)} inline tools") + return 0 + + +def _flatten_cycle() -> int: + """Detect a cycle and verify error.""" + a = Skill( + name="local/cyc-a", + description="A", + includes=[SkillInclude(name="local/cyc-b")], + ) + b = Skill( + name="local/cyc-b", + description="B", + includes=[SkillInclude(name="local/cyc-a")], + ) + registry = {"local/cyc-a": a, "local/cyc-b": b} + resolver = SkillResolver() + try: + resolver.resolve_tools(a, registry) + print("flatten-fail: expected cycle error") + return 1 + except ValueError as exc: + if "ycle" not in str(exc): + print(f"flatten-fail: wrong error: {exc}") + return 1 + print(f"flatten-cycle-ok: {exc}") + return 0 + + +def _flatten_include_overrides() -> int: + """Apply per-include overrides.""" + child = Skill( + name="local/child", + description="Child", + tool_refs=["local/tool-x"], + ) + parent = Skill( + name="local/parent", + description="Parent", + includes=[ + SkillInclude( + name="local/child", + overrides={"timeout": 600}, + ) + ], + ) + registry = {"local/child": child, "local/parent": parent} + resolver = SkillResolver() + result = resolver.resolve_tools(parent, registry) + entry = result[0] + if entry.overrides.get("timeout") != 600: + print(f"flatten-fail: override not applied: {entry.overrides}") + return 1 + print(f"flatten-include-overrides-ok: {entry.overrides}") + return 0 + + +def _flatten_non_overridable() -> int: + """Reject non-overridable fields.""" + child = Skill( + name="local/child-nr", + description="Child", + tool_refs=["local/tool-y"], + ) + parent = Skill( + name="local/parent-nr", + description="Parent", + includes=[ + SkillInclude( + name="local/child-nr", + overrides={"name": "bad"}, + ) + ], + ) + registry = {"local/child-nr": child, "local/parent-nr": parent} + resolver = SkillResolver() + try: + resolver.resolve_tools(parent, registry) + print("flatten-fail: expected override rejection error") + return 1 + except ValueError as exc: + if "Non-overridable" not in str(exc): + print(f"flatten-fail: wrong error: {exc}") + return 1 + print(f"flatten-non-overridable-ok: {exc}") + return 0 + + +def _capability_summary() -> int: + """Compute capability summary with diverse inline tools.""" + skill = Skill( + name="local/cap", + description="Cap test", + anonymous_tools=[ + SkillInlineTool( + description="RO", + source=ToolSource.CUSTOM, + code="pass", + capability=ToolCapability(read_only=True), + ), + SkillInlineTool( + description="Writer", + source=ToolSource.CUSTOM, + code="pass", + capability=ToolCapability(writes=True, side_effects=["fs"]), + ), + SkillInlineTool( + description="Checkpoint", + source=ToolSource.CUSTOM, + code="pass", + capability=ToolCapability(checkpointable=True), + ), + ], + ) + resolver = SkillResolver() + entries = resolver.resolve_tools(skill, {}) + summary = resolver.compute_capability_summary(skill, entries) + checks = [ + summary.total_tools == 3, + summary.read_only_tools == 1, + summary.write_tools == 1, + summary.checkpointable_tools == 1, + summary.has_side_effects is True, + ] + if not all(checks): + print(f"flatten-fail: summary checks failed: {summary}") + return 1 + print("flatten-capability-summary-ok") + return 0 + + +def _tools_method() -> int: + """Call SkillRegistry.tools().""" + skill = Skill( + name="local/tm", + description="Tools method test", + tool_refs=["local/t1", "local/t2"], + ) + defn = _make_defn(skill) + sr = SkillRegistry() + sr.register(defn) + entries, summary = sr.tools("local/tm") + if len(entries) != 2: + print(f"flatten-fail: expected 2 entries, got {len(entries)}") + return 1 + if summary.total_tools != 2: + print(f"flatten-fail: summary total={summary.total_tools}") + return 1 + print("flatten-tools-method-ok") + return 0 + + +def _validate_plan_ok() -> int: + """validate_plan with valid plan.""" + skill = Skill( + name="local/vp", + description="VP test", + tool_refs=["local/t1"], + ) + defn = _make_defn(skill) + sr = SkillRegistry() + sr.register(defn) + errors = sr.validate_plan({"skills": ["local/vp"]}) + if errors: + print(f"flatten-fail: unexpected errors: {errors}") + return 1 + print("flatten-validate-plan-ok") + return 0 + + +def _validate_plan_missing() -> int: + """validate_plan with missing skill.""" + sr = SkillRegistry() + errors = sr.validate_plan({"skills": ["local/ghost"]}) + if not errors: + print("flatten-fail: expected error for missing skill") + return 1 + if "local/ghost" not in errors[0]: + print(f"flatten-fail: wrong error: {errors[0]}") + return 1 + print(f"flatten-validate-plan-missing-ok: {errors[0]}") + return 0 + + +def main() -> int: + """Entry point called by Robot Framework ``Run Process``.""" + if len(sys.argv) < 2: + print("Usage: helper_skill_flatten.py ") + return 1 + + commands = { + "flatten-simple": _flatten_simple, + "flatten-deep": _flatten_deep, + "flatten-wide": _flatten_wide, + "flatten-inline": _flatten_inline, + "flatten-cycle": _flatten_cycle, + "flatten-include-overrides": _flatten_include_overrides, + "flatten-non-overridable": _flatten_non_overridable, + "capability-summary": _capability_summary, + "tools-method": _tools_method, + "validate-plan-ok": _validate_plan_ok, + "validate-plan-missing": _validate_plan_missing, + } + + handler = commands.get(sys.argv[1]) + if handler is None: + print(f"Unknown command: {sys.argv[1]}") + return 1 + + return handler() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/robot/skill_discovery.robot b/robot/skill_discovery.robot new file mode 100644 index 0000000000..752b2ccb69 --- /dev/null +++ b/robot/skill_discovery.robot @@ -0,0 +1,68 @@ +*** Settings *** +Documentation Agent Skills discovery integration tests +Library OperatingSystem +Library Collections +Library helper_skill_discovery.py +Variables ${CURDIR}/common_vars.py + +*** Test Cases *** + +Parse Single Agent Skills Path + [Documentation] Parse a single comma-separated path string + ${paths}= Parse Agent Skills Paths /opt/skills + Length Should Be ${paths} 1 + +Parse Multiple Agent Skills Paths + [Documentation] Parse multiple comma-separated paths + ${paths}= Parse Agent Skills Paths /opt/skills,/home/user/skills + Length Should Be ${paths} 2 + +Parse Empty Agent Skills Paths + [Documentation] Empty string returns no paths + ${paths}= Parse Agent Skills Paths ${EMPTY} + Length Should Be ${paths} 0 + +Discover Agent Skills From Directory + [Documentation] Scan a temp directory with SKILL.md folders + ${tmpdir}= Create Agent Skills Directory 2 + ${discovered}= Scan Agent Skills Directory ${tmpdir} + Length Should Be ${discovered} 2 + Cleanup Temp Directory ${tmpdir} + +Discover Agent Skills Skips Folders Without SKILL MD + [Documentation] Directories without SKILL.md are skipped + ${tmpdir}= Create Mixed Directory + ${discovered}= Scan Agent Skills Directory ${tmpdir} + Length Should Be ${discovered} 1 + Cleanup Temp Directory ${tmpdir} + +Build ToolSpec From Discovered Skill + [Documentation] Build a ToolSpec with agent_skills source metadata + ${spec}= Build Agent Skill ToolSpec my-tool /opt/skills/my-tool + Should Be Equal ${spec.name} agent_skills/my-tool + Should Be Equal ${spec.source} agent_skills + Dictionary Should Contain Key ${spec.source_metadata} path + +Register Discovered Skills Without Conflicts + [Documentation] Register skills in an empty ToolRegistry + ${result}= Register Agent Skills In Fresh Registry 3 + Should Be Equal As Integers ${result}[registered_count] 3 + Should Be Equal As Integers ${result}[conflict_count] 0 + +Register Discovered Skills With Conflict Skip + [Documentation] Name collisions are skipped with skip strategy + ${result}= Register Agent Skills With Collision skip + Should Be Equal As Integers ${result}[registered_count] 0 + Should Be Equal As Integers ${result}[conflict_count] 1 + +Register Discovered Skills With Conflict Replace + [Documentation] Name collisions are replaced with replace strategy + ${result}= Register Agent Skills With Collision replace + Should Be Equal As Integers ${result}[registered_count] 1 + Should Be Equal As Integers ${result}[conflict_count] 1 + +Config Key Registered + [Documentation] skills.agent_skills_paths config key exists + ${entry}= Get Config Entry skills.agent_skills_paths + Should Not Be Equal ${entry} ${None} + Should Be Equal ${entry.env_var} CLEVERAGENTS_SKILLS_AGENT_SKILLS_PATHS diff --git a/robot/skill_flatten.robot b/robot/skill_flatten.robot new file mode 100644 index 0000000000..6e99784b87 --- /dev/null +++ b/robot/skill_flatten.robot @@ -0,0 +1,97 @@ +*** Settings *** +Documentation Smoke tests for Skill flattening, capability summaries, and plan validation +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_skill_flatten.py + +*** Test Cases *** +Flatten Simple Skill With Tool Refs + [Documentation] Flatten a skill with 5 tool refs and verify deterministic order + ${result}= Run Process ${PYTHON} ${HELPER} flatten-simple cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-simple-ok + +Flatten Skill With Deep Includes + [Documentation] Flatten skill with 3 levels of includes in depth-first order + ${result}= Run Process ${PYTHON} ${HELPER} flatten-deep cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-deep-ok + +Flatten Skill With Wide Includes + [Documentation] Flatten skill including 10 other skills + ${result}= Run Process ${PYTHON} ${HELPER} flatten-wide cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-wide-ok + +Flatten Skill With Inline Tools + [Documentation] Flatten a skill with inline tools and verify keying + ${result}= Run Process ${PYTHON} ${HELPER} flatten-inline cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-inline-ok + +Flatten Skill With Cycle Raises Error + [Documentation] Detect cycles in include chains and report clear error path + ${result}= Run Process ${PYTHON} ${HELPER} flatten-cycle cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-cycle-ok + +Flatten Skill With Per-Include Overrides + [Documentation] Apply per-include overrides to included tool entries + ${result}= Run Process ${PYTHON} ${HELPER} flatten-include-overrides cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-include-overrides-ok + +Flatten Skill Rejects Non-Overridable Fields + [Documentation] Reject overrides on non-overridable fields (name, source_skill, is_inline) + ${result}= Run Process ${PYTHON} ${HELPER} flatten-non-overridable cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-non-overridable-ok + +Compute Capability Summary + [Documentation] Compute capability summary with diverse inline tools + ${result}= Run Process ${PYTHON} ${HELPER} capability-summary cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-capability-summary-ok + +Tools Method Returns Entries And Summary + [Documentation] SkillRegistry.tools() returns both entries and summary + ${result}= Run Process ${PYTHON} ${HELPER} tools-method cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-tools-method-ok + +Validate Plan With Valid Skills + [Documentation] validate_plan with a valid plan returns empty error list + ${result}= Run Process ${PYTHON} ${HELPER} validate-plan-ok cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-validate-plan-ok + +Validate Plan With Missing Skill + [Documentation] validate_plan reports error for missing skill + ${result}= Run Process ${PYTHON} ${HELPER} validate-plan-missing cwd=${WORKSPACE} + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} flatten-validate-plan-missing-ok diff --git a/src/cleveragents/actor/__init__.py b/src/cleveragents/actor/__init__.py index 9b150c05a4..b85e0e6d09 100644 --- a/src/cleveragents/actor/__init__.py +++ b/src/cleveragents/actor/__init__.py @@ -1,7 +1,27 @@ -"""Actor package for actor registry, configuration, and loading helpers.""" +"""Actor package for actor registry, configuration, loading, and compilation.""" +from .compiler import ( + ActorCompilationError, + CompilationMetadata, + CompiledActor, + InvalidEntryExitError, + MissingNodeError, + SubgraphCycleError, + compile_actor, +) from .config import ActorConfiguration from .loader import ActorLoader from .registry import ActorRegistry -__all__ = ["ActorConfiguration", "ActorLoader", "ActorRegistry"] +__all__ = [ + "ActorCompilationError", + "ActorConfiguration", + "ActorLoader", + "ActorRegistry", + "CompilationMetadata", + "CompiledActor", + "InvalidEntryExitError", + "MissingNodeError", + "SubgraphCycleError", + "compile_actor", +] diff --git a/src/cleveragents/actor/compiler.py b/src/cleveragents/actor/compiler.py new file mode 100644 index 0000000000..5abc0439b7 --- /dev/null +++ b/src/cleveragents/actor/compiler.py @@ -0,0 +1,345 @@ +"""Actor compiler: translate hierarchical YAML actor configs to LangGraph. + +Compiles ``ActorConfigSchema`` (type=GRAPH) definitions into LangGraph +``GraphConfig`` objects with validated node/edge wiring, subgraph +resolution, cycle detection, and LSP binding metadata. + +Public API: + - :func:`compile_actor` — compile a single actor config. + - :class:`CompilationMetadata` — diagnostic output. + - :class:`CompiledActor` — compiled result bundle. + - :class:`ActorCompilationError` — raised on compile failures. + - :class:`SubgraphCycleError` — raised when subgraph refs form a cycle. + - :class:`MissingNodeError` — raised when a referenced node is absent. + +Usage:: + + from cleveragents.actor.compiler import compile_actor + compiled = compile_actor(config) + print(compiled.metadata.node_ids) +""" + +from __future__ import annotations + +from typing import Any + +from pydantic import BaseModel, ConfigDict, Field + +from cleveragents.actor.schema import ( + ActorConfigSchema, + ActorType, + EdgeDefinition, + NodeDefinition, + NodeType, +) +from cleveragents.core.exceptions import ValidationError +from cleveragents.langgraph import nodes as lg_nodes +from cleveragents.lsp.models import LspBinding + +# --------------------------------------------------------------------------- +# Errors +# --------------------------------------------------------------------------- + + +class ActorCompilationError(ValidationError): + """Raised when actor compilation fails.""" + + +class SubgraphCycleError(ActorCompilationError): + """Raised when subgraph references form a cycle.""" + + +class MissingNodeError(ActorCompilationError): + """Raised when a referenced node does not exist.""" + + +class InvalidEntryExitError(ActorCompilationError): + """Raised when entry/exit node mapping is invalid.""" + + +# --------------------------------------------------------------------------- +# Compilation metadata & result models +# --------------------------------------------------------------------------- + +_NODE_TYPE_MAP: dict[NodeType, lg_nodes.NodeType] = { + NodeType.AGENT: lg_nodes.NodeType.AGENT, + NodeType.TOOL: lg_nodes.NodeType.TOOL, + NodeType.CONDITIONAL: lg_nodes.NodeType.CONDITIONAL, + NodeType.SUBGRAPH: lg_nodes.NodeType.SUBGRAPH, +} + + +class CompilationMetadata(BaseModel): + """Diagnostic metadata produced by the actor compiler. + + Attributes: + node_ids: All compiled node IDs. + tool_nodes: Node IDs that are tool-type nodes. + lsp_bindings: Per-node LSP binding records. + subgraph_refs: Mapping of subgraph node ID → referenced actor name. + entry_node: The validated entry-point node ID. + exit_nodes: The validated exit-point node IDs. + """ + + node_ids: list[str] = Field(default_factory=list) + tool_nodes: list[str] = Field(default_factory=list) + lsp_bindings: list[LspBinding] = Field(default_factory=list) + subgraph_refs: dict[str, str] = Field(default_factory=dict) + entry_node: str = Field(default="") + exit_nodes: list[str] = Field(default_factory=list) + + model_config = ConfigDict( + str_strip_whitespace=True, + validate_assignment=True, + ) + + +class CompiledActor(BaseModel): + """Result bundle returned by :func:`compile_actor`. + + Attributes: + name: Actor name from the source config. + nodes: Compiled LangGraph ``NodeConfig`` mapping. + edges: Compiled LangGraph ``Edge`` list. + entry_point: Entry node ID. + metadata: Diagnostic compilation metadata. + """ + + name: str = Field(..., description="Actor name") + nodes: dict[str, lg_nodes.NodeConfig] = Field(default_factory=dict) + edges: list[lg_nodes.Edge] = Field(default_factory=list) + entry_point: str = Field(default="start") + metadata: CompilationMetadata = Field(default_factory=CompilationMetadata) + + model_config = ConfigDict( + str_strip_whitespace=True, + validate_assignment=True, + arbitrary_types_allowed=True, + ) + + +# --------------------------------------------------------------------------- +# Internal helpers +# --------------------------------------------------------------------------- + +ActorResolver = Any # callable(name: str) -> ActorConfigSchema | None + + +def _map_node(node: NodeDefinition) -> lg_nodes.NodeConfig: + """Map an actor schema node to a LangGraph ``NodeConfig``.""" + lg_type = _NODE_TYPE_MAP.get(node.type, lg_nodes.NodeType.FUNCTION) + config = node.config + return lg_nodes.NodeConfig( + name=node.id, + type=lg_type, + agent=config.get("agent") if node.type == NodeType.AGENT else None, + function=( + config.get("function") if node.type == NodeType.CONDITIONAL else None + ), + tools=config.get("tools", []) if node.type == NodeType.TOOL else [], + subgraph=(config.get("actor_ref") if node.type == NodeType.SUBGRAPH else None), + metadata=dict(config), + ) + + +def _map_edge(edge: EdgeDefinition) -> lg_nodes.Edge: + """Map an actor schema edge to a LangGraph ``Edge``.""" + condition: dict[str, Any] | None = None + if edge.condition is not None: + condition = {"expression": edge.condition} + return lg_nodes.Edge( + source=edge.from_node, + target=edge.to_node, + condition=condition, + metadata={"priority": edge.priority}, + ) + + +def _extract_lsp_bindings(node: NodeDefinition) -> list[LspBinding]: + """Extract LSP bindings from a node config block.""" + bindings: list[LspBinding] = [] + raw_bindings = node.config.get("lsp_bindings", []) + if not isinstance(raw_bindings, list): + return bindings + for entry in raw_bindings: + if not isinstance(entry, dict): + continue + server = entry.get("lsp_server_name", "") + if not server or not isinstance(server, str): + continue + bindings.append( + LspBinding( + node_name=node.id, + lsp_server_name=server, + languages=entry.get("languages", []), + auto_detect=entry.get("auto_detect", True), + ) + ) + return bindings + + +def _detect_subgraph_cycles( + actor_name: str, + route_nodes: list[NodeDefinition], + resolver: ActorResolver | None, + visited: frozenset[str], +) -> list[str]: + """Detect cycles in subgraph references across actors. + + Returns list of actor names forming a cycle, or empty list. + """ + if resolver is None: + return [] + + for node in route_nodes: + if node.type != NodeType.SUBGRAPH: + continue + ref_name = node.config.get("actor_ref", "") + if not ref_name: + continue + if ref_name in visited: + return [*list(visited), ref_name] + + referenced: ActorConfigSchema | None = resolver(ref_name) + if referenced is None: + continue + if referenced.type != ActorType.GRAPH or referenced.route is None: + continue + + deeper = _detect_subgraph_cycles( + ref_name, + referenced.route.nodes, + resolver, + visited | {ref_name}, + ) + if deeper: + return deeper + return [] + + +# --------------------------------------------------------------------------- +# Public compile function +# --------------------------------------------------------------------------- + + +def compile_actor( + config: ActorConfigSchema, + *, + actor_resolver: ActorResolver | None = None, +) -> CompiledActor: + """Compile an ``ActorConfigSchema`` into a LangGraph-ready bundle. + + Args: + config: The actor configuration to compile. Must have + ``type == ActorType.GRAPH`` and a valid ``route``. + actor_resolver: Optional callable ``(name: str) -> ActorConfigSchema | None`` + used to resolve subgraph actor references. + + Returns: + A :class:`CompiledActor` with nodes, edges, and metadata. + + Raises: + ActorCompilationError: If the config type is not GRAPH. + MissingNodeError: If a referenced node is not found. + InvalidEntryExitError: If entry/exit mapping is invalid. + SubgraphCycleError: If subgraph references form a cycle. + """ + if config.type != ActorType.GRAPH: + raise ActorCompilationError( + f"Only GRAPH actors can be compiled; got type={config.type.value}" + ) + + route = config.route + if route is None: + raise ActorCompilationError( + f"GRAPH actor '{config.name}' has no route definition" + ) + + # Validate references first + route.validate_references() + + # Detect intra-graph cycles (already done in schema validation, + # but repeated here for compile-time safety) + intra_cycles = route.detect_cycles() + if intra_cycles: + raise SubgraphCycleError(f"Graph contains intra-graph cycles: {intra_cycles}") + + # Detect cross-actor subgraph cycles + cross_cycles = _detect_subgraph_cycles( + config.name, route.nodes, actor_resolver, frozenset({config.name}) + ) + if cross_cycles: + raise SubgraphCycleError( + f"Subgraph references form a cycle: {' -> '.join(cross_cycles)}" + ) + + # Build node map + node_ids = {n.id for n in route.nodes} + nodes: dict[str, lg_nodes.NodeConfig] = {} + tool_node_ids: list[str] = [] + all_lsp_bindings: list[LspBinding] = [] + subgraph_refs: dict[str, str] = {} + + for node_def in route.nodes: + nodes[node_def.id] = _map_node(node_def) + + if node_def.type == NodeType.TOOL: + tool_node_ids.append(node_def.id) + + bindings = _extract_lsp_bindings(node_def) + all_lsp_bindings.extend(bindings) + + if node_def.type == NodeType.SUBGRAPH: + ref = node_def.config.get("actor_ref", "") + if ref: + subgraph_refs[node_def.id] = ref + + # Build edges + edges: list[lg_nodes.Edge] = [] + for edge_def in route.edges: + if edge_def.from_node not in node_ids: + raise MissingNodeError( + f"Edge from_node '{edge_def.from_node}' not in compiled nodes" + ) + if edge_def.to_node not in node_ids: + raise MissingNodeError( + f"Edge to_node '{edge_def.to_node}' not in compiled nodes" + ) + edges.append(_map_edge(edge_def)) + + # Validate entry/exit + if route.entry_node not in node_ids: + raise InvalidEntryExitError( + f"Entry node '{route.entry_node}' not found in nodes" + ) + for exit_id in route.exit_nodes: + if exit_id not in node_ids: + raise InvalidEntryExitError(f"Exit node '{exit_id}' not found in nodes") + + metadata = CompilationMetadata( + node_ids=sorted(node_ids), + tool_nodes=tool_node_ids, + lsp_bindings=all_lsp_bindings, + subgraph_refs=subgraph_refs, + entry_node=route.entry_node, + exit_nodes=list(route.exit_nodes), + ) + + return CompiledActor( + name=config.name, + nodes=nodes, + edges=edges, + entry_point=route.entry_node, + metadata=metadata, + ) + + +__all__ = [ + "ActorCompilationError", + "CompilationMetadata", + "CompiledActor", + "InvalidEntryExitError", + "MissingNodeError", + "SubgraphCycleError", + "compile_actor", +] diff --git a/src/cleveragents/application/services/config_service.py b/src/cleveragents/application/services/config_service.py index 5b8526c7c3..efc9bd8022 100644 --- a/src/cleveragents/application/services/config_service.py +++ b/src/cleveragents/application/services/config_service.py @@ -244,6 +244,18 @@ def _build_catalog() -> None: description="Auto-include gitignore patterns in context filtering.", ) + # -- skills.* ------------------------------------------------------------- + _register( + "skills", + "agent_skills_paths", + str, + str(Path.home() / ".cleveragents" / "agent_skills"), + description=( + "Comma-separated list of directories to scan for Agent Skills " + "Standard folders (each containing a SKILL.md)." + ), + ) + # -- index.* -------------------------------------------------------------- _register( "index", diff --git a/src/cleveragents/application/services/skill_registry_service.py b/src/cleveragents/application/services/skill_registry_service.py index dffdae5185..367bb41f4a 100644 --- a/src/cleveragents/application/services/skill_registry_service.py +++ b/src/cleveragents/application/services/skill_registry_service.py @@ -2,14 +2,30 @@ Orchestrates persistence through ``SkillRepository``, providing a clean API for CLI and higher-level orchestrators. + +Includes Agent Skills discovery via configurable paths +(``skills.agent_skills_paths``) with refresh-on-demand semantics. """ from __future__ import annotations +import logging +from typing import Any + from cleveragents.domain.models.core.skill import Skill from cleveragents.infrastructure.database.repositories import ( SkillRepository, ) +from cleveragents.skills.discovery import ( + DiscoveryConflict, + DiscoveryResult, + discover_agent_skills, + parse_agent_skills_paths, + register_discovered_skills, +) +from cleveragents.tool.runtime import ToolSpec + +logger = logging.getLogger(__name__) class SkillRegistryService: @@ -17,10 +33,22 @@ class SkillRegistryService: Delegates all persistence to the repository layer. Designed to be injected with a ``SkillRepository`` (using session-factory pattern). + + Provides Agent Skills discovery via ``discover_and_register`` and a + refresh hook (``refresh_agent_skills``) that re-scans configured + paths and updates the ``ToolRegistry``. """ - def __init__(self, skill_repo: SkillRepository) -> None: + def __init__( + self, + skill_repo: SkillRepository, + tool_registry: Any | None = None, + ) -> None: self._skill_repo = skill_repo + self._tool_registry = tool_registry + self._last_discovery: DiscoveryResult | None = None + self._registered_agent_tools: list[ToolSpec] = [] + self._discovery_conflicts: list[DiscoveryConflict] = [] # -- Skill CRUD -------------------------------------------------------- @@ -89,3 +117,86 @@ class SkillRegistryService: ``True`` if the skill was removed, ``False`` if not found. """ return self._skill_repo.delete(name) + + # -- Agent Skills discovery -------------------------------------------- + + def discover_and_register( + self, + raw_paths: str, + *, + on_conflict: str = "skip", + ) -> DiscoveryResult: + """Run agent skills discovery and register tools. + + Parses the comma-separated *raw_paths* string, scans each + directory for Agent Skills Standard folders, and registers the + discovered tools in the ``ToolRegistry``. + + Args: + raw_paths: Comma-separated directory paths to scan. + on_conflict: Conflict strategy — ``"skip"`` (default), + ``"error"``, or ``"replace"``. + + Returns: + ``DiscoveryResult`` with discovered skills and any errors. + """ + if not raw_paths: + return DiscoveryResult() + + paths = parse_agent_skills_paths(raw_paths) + result = discover_agent_skills(paths) + self._last_discovery = result + + if result.discovered and self._tool_registry is not None: + registered, conflicts = register_discovered_skills( + result.discovered, + self._tool_registry, + on_conflict=on_conflict, + ) + self._registered_agent_tools = registered + self._discovery_conflicts = conflicts + result.conflicts = conflicts + + return result + + def refresh_agent_skills( + self, + raw_paths: str, + *, + on_conflict: str = "skip", + ) -> DiscoveryResult: + """Re-scan Agent Skills paths and update registrations. + + Removes previously discovered agent skill tools from the + ``ToolRegistry`` before re-scanning and re-registering. + + Args: + raw_paths: Comma-separated directory paths to scan. + on_conflict: Conflict strategy. + + Returns: + Fresh ``DiscoveryResult``. + """ + # Remove previously registered agent skill tools + if self._tool_registry is not None: + for spec in self._registered_agent_tools: + self._tool_registry.remove(spec.name) + self._registered_agent_tools = [] + self._discovery_conflicts = [] + + return self.discover_and_register(raw_paths, on_conflict=on_conflict) + + @property + def last_discovery(self) -> DiscoveryResult | None: + """Return the result of the most recent discovery scan.""" + return self._last_discovery + + @property + def registered_agent_tools(self) -> list[ToolSpec]: + """Return the list of currently registered agent skill tools.""" + return list(self._registered_agent_tools) + + @property + def discovery_conflicts(self) -> list[DiscoveryConflict]: + """Return conflicts from the most recent discovery scan.""" + return list(self._discovery_conflicts) diff --git a/src/cleveragents/cli/commands/skill.py b/src/cleveragents/cli/commands/skill.py index 0e226c1591..18c288ed82 100644 --- a/src/cleveragents/cli/commands/skill.py +++ b/src/cleveragents/cli/commands/skill.py @@ -739,6 +739,13 @@ def tools( str, typer.Argument(help="Namespaced skill name to resolve tools for"), ], + refresh: Annotated[ + bool, + typer.Option( + "--refresh", + help="Re-scan Agent Skills paths before resolving tools", + ), + ] = False, fmt: Annotated[ str, typer.Option( @@ -753,21 +760,61 @@ def tools( Includes tools inherited from included skills. Displays tool source, origin skill, read-only/write/checkpoint flags, and a summary. + Use ``--refresh`` to re-scan Agent Skills discovery paths before + resolving tools (picks up new or changed Agent Skills folders). + Examples: agents skill tools local/devops-toolkit + agents skill tools local/devops-toolkit --refresh agents skill tools local/devops-toolkit --format json """ try: service = _get_skill_service() + + if refresh: + from cleveragents.application.services.config_service import ( + ConfigService, + ) + + config_svc = ConfigService() + resolved = config_svc.resolve("skills.agent_skills_paths") + raw_paths: str = str(resolved.value) if resolved.value else "" + if raw_paths: + from cleveragents.skills.discovery import ( + discover_agent_skills, + parse_agent_skills_paths, + ) + + paths = parse_agent_skills_paths(raw_paths) + result = discover_agent_skills(paths) + if result.discovered: + console.print( + f"[blue]Refreshed:[/blue] Found " + f"{len(result.discovered)} agent skill(s)" + ) + if result.errors: + for err in result.errors: + console.print(f"[yellow]Warning:[/yellow] {err}") + skill, entries = service.resolve_tools(name) if fmt != OutputFormat.RICH.value: data: list[dict[str, Any]] = [] for entry in entries: + # Determine source type for metadata + source_type = "builtin" + if entry.name.startswith("mcp:"): + source_type = "mcp" + elif entry.name.startswith("agent_skill:"): + source_type = "agent_skills" + elif entry.is_inline: + source_type = "custom" + data.append( { "name": entry.name, "source_skill": entry.source_skill, + "source": source_type, "is_direct": entry.source_skill == skill.name, "is_inline": entry.is_inline, } diff --git a/src/cleveragents/domain/models/core/skill.py b/src/cleveragents/domain/models/core/skill.py index 8a369610de..8680076dd4 100644 --- a/src/cleveragents/domain/models/core/skill.py +++ b/src/cleveragents/domain/models/core/skill.py @@ -431,6 +431,46 @@ class ResolvedToolEntry(BaseModel): ) +# --------------------------------------------------------------------------- +# Non-overridable fields (used by SkillResolver) +# --------------------------------------------------------------------------- + +_NON_OVERRIDABLE_FIELDS: frozenset[str] = frozenset( + {"name", "source_skill", "is_inline"} +) +"""Fields that may never be overridden via ``SkillInclude.overrides``. + +Attempting to override any of these raises ``ValueError`` with the +originating skill name and tool name for traceability. +""" + + +def _validate_override_keys( + overrides: dict[str, Any], + *, + include_path: str, + skill_name: str, +) -> None: + """Raise ``ValueError`` if *overrides* contains non-overridable keys. + + Args: + overrides: The override dict to validate. + include_path: Human-readable include chain for error context. + skill_name: The skill that declared the include. + + Raises: + ValueError: If any key in *overrides* is in + ``_NON_OVERRIDABLE_FIELDS``. + """ + bad_keys = _NON_OVERRIDABLE_FIELDS & overrides.keys() + if bad_keys: + sorted_bad = ", ".join(sorted(bad_keys)) + raise ValueError( + f"Non-overridable field(s) {sorted_bad} in overrides " + f"from skill '{skill_name}' (include path: {include_path})" + ) + + # --------------------------------------------------------------------------- # SkillResolver # --------------------------------------------------------------------------- @@ -443,6 +483,24 @@ class SkillResolver: tool de-duplication (last-wins for overrides), and deterministic ordering of the output. + **Deterministic ordering guarantee**: The output list is ordered by + *include-order-first, then definition-order*: + + 1. Tools from depth-first traversal of includes (leftmost include + recursed first, its includes before its own tool_refs). + 2. Tool refs from the current skill, in definition order. + 3. Inline tools from the current skill, in definition order. + 4. MCP tools, then agent skill tools, in definition order. + + When a tool name appears multiple times (from overlapping includes), + the first occurrence determines position in the list, but the **last + occurrence's entry metadata wins** (last-wins de-duplication). + + **Per-include overrides**: Each ``SkillInclude`` may carry an + ``overrides`` dict that is applied (shallow-merged) to tool entries + resolved from the included skill. Fields listed in + ``_NON_OVERRIDABLE_FIELDS`` are rejected with a ``ValueError``. + Example: ```python resolver = SkillResolver() @@ -500,7 +558,14 @@ class SkillResolver: visited: set[str], path: list[str], ) -> None: - """Recursively resolve includes, collecting tools along the way.""" + """Recursively resolve includes, collecting tools along the way. + + Per-include overrides from ``SkillInclude.overrides`` are applied + to the tool entries contributed by the included skill after + recursion completes. Non-overridable fields (see + ``_NON_OVERRIDABLE_FIELDS``) are rejected with a ``ValueError`` + that includes the include path for traceability. + """ if skill.name in visited: cycle_path = " -> ".join([*path, skill.name]) raise ValueError(f"Cycle detected in skill includes: {cycle_path}") @@ -516,6 +581,9 @@ class SkillResolver: f"Included skill '{include.name}' not found " f"(referenced from '{skill.name}')" ) + # Snapshot keys before recursing so we can identify new entries + keys_before = set(seen_tools.keys()) + self._resolve_recursive( skill=included_skill, skill_lookup=skill_lookup, @@ -525,6 +593,20 @@ class SkillResolver: path=list(path), ) + # Apply per-include overrides to entries added by this include + if include.overrides: + include_path = " -> ".join([*path, include.name]) + _validate_override_keys( + include.overrides, + include_path=include_path, + skill_name=skill.name, + ) + new_keys = set(seen_tools.keys()) - keys_before + for key in new_keys: + entry = seen_tools[key] + merged = {**entry.overrides, **include.overrides} + seen_tools[key] = entry.model_copy(update={"overrides": merged}) + # 2. Add tool_refs from this skill for ref in skill.tool_refs: entry = ResolvedToolEntry( diff --git a/src/cleveragents/skills/__init__.py b/src/cleveragents/skills/__init__.py index c579d50e2f..d9939aede7 100644 --- a/src/cleveragents/skills/__init__.py +++ b/src/cleveragents/skills/__init__.py @@ -19,6 +19,16 @@ from cleveragents.skills.context import ( SkillExecutionError, ToolInvocationRecord, ) +from cleveragents.skills.discovery import ( + DiscoveredAgentSkill, + DiscoveryConflict, + DiscoveryResult, + build_tool_spec, + discover_agent_skills, + parse_agent_skills_paths, + register_discovered_skills, + scan_directory, +) from cleveragents.skills.inline_executor import ( InlineToolExecutor, InlineToolResult, @@ -35,6 +45,9 @@ from cleveragents.skills.registry import SkillRegistry from cleveragents.skills.schema import SkillConfigSchema __all__ = [ + "DiscoveredAgentSkill", + "DiscoveryConflict", + "DiscoveryResult", "InlineToolExecutor", "InlineToolResult", "SkillConfigSchema", @@ -47,5 +60,10 @@ __all__ = [ "SkillRegistry", "SkillResult", "ToolInvocationRecord", + "build_tool_spec", + "discover_agent_skills", "map_tool_error", + "parse_agent_skills_paths", + "register_discovered_skills", + "scan_directory", ] diff --git a/src/cleveragents/skills/discovery.py b/src/cleveragents/skills/discovery.py new file mode 100644 index 0000000000..28cb077871 --- /dev/null +++ b/src/cleveragents/skills/discovery.py @@ -0,0 +1,340 @@ +"""Agent Skills discovery for CleverAgents v3. + +Scans configured directories for Agent Skills Standard folders (each +containing a ``SKILL.md`` file) and produces +:class:`~cleveragents.tool.runtime.ToolSpec` instances ready for +registration in the :class:`~cleveragents.tool.registry.ToolRegistry`. + +## Discovery Algorithm + +1. Read the comma-separated ``skills.agent_skills_paths`` config value. +2. For each directory, find immediate subdirectories containing a + ``SKILL.md`` file. +3. Parse the ``SKILL.md`` front-matter (YAML between ``---`` fences) to + extract tool name, description, and optional input/output schemas. +4. Construct a ``ToolSpec`` with ``source="agent_skills"`` and + ``source_metadata`` containing the filesystem path. + +## Conflict Handling + +When a discovered tool name already exists in the ``ToolRegistry`` the +discovery process records a ``DiscoveryConflict`` rather than silently +overwriting. Callers decide whether to skip, warn, or error. + +Based on ``docs/specification.md`` Agent Skills discovery section and +issue #161. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +import yaml + +from cleveragents.tool.runtime import ToolSpec + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Data types +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True, slots=True) +class DiscoveredAgentSkill: + """Metadata extracted from a single Agent Skills Standard folder.""" + + name: str + description: str + path: str + input_schema: dict[str, Any] = field(default_factory=dict) + output_schema: dict[str, Any] = field(default_factory=dict) + + +@dataclass(frozen=True, slots=True) +class DiscoveryConflict: + """Records a name collision during agent skills discovery.""" + + tool_name: str + existing_source: str + discovered_path: str + + +@dataclass(slots=True) +class DiscoveryResult: + """Aggregate result of an agent skills discovery scan.""" + + discovered: list[DiscoveredAgentSkill] = field(default_factory=list) + conflicts: list[DiscoveryConflict] = field(default_factory=list) + errors: list[str] = field(default_factory=list) + + +# --------------------------------------------------------------------------- +# Parsing helpers +# --------------------------------------------------------------------------- + +_FRONT_MATTER_SEP = "---" + + +def _parse_skill_md(skill_md_path: Path) -> dict[str, Any] | None: + """Parse the YAML front-matter from a ``SKILL.md`` file. + + Returns the parsed dict or ``None`` if the file has no valid + front-matter block. + """ + if not skill_md_path.is_file(): + return None + + text = skill_md_path.read_text(encoding="utf-8") + lines = text.split("\n") + + # Find opening and closing ``---`` fences + fence_indices: list[int] = [] + for idx, line in enumerate(lines): + if line.strip() == _FRONT_MATTER_SEP: + fence_indices.append(idx) + if len(fence_indices) == 2: + break + + if len(fence_indices) < 2: + return None + + yaml_block = "\n".join(lines[fence_indices[0] + 1 : fence_indices[1]]) + if not yaml_block.strip(): + return None + + try: + data = yaml.safe_load(yaml_block) + except yaml.YAMLError as exc: + logger.warning("Failed to parse SKILL.md at %s: %s", skill_md_path, exc) + return None + + if not isinstance(data, dict): + return None + + return data + + +def _skill_from_front_matter( + data: dict[str, Any], + folder_path: Path, +) -> DiscoveredAgentSkill | None: + """Build a ``DiscoveredAgentSkill`` from parsed front-matter. + + Returns ``None`` when required fields are missing. + """ + name = data.get("name") + if not name or not isinstance(name, str): + # Fall back to folder name + name = folder_path.name + + description = data.get("description", "") + if not isinstance(description, str): + description = str(description) + if not description: + description = f"Agent skill from {folder_path.name}" + + return DiscoveredAgentSkill( + name=name, + description=description, + path=str(folder_path), + input_schema=data.get("input_schema", {}), + output_schema=data.get("output_schema", {}), + ) + + +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- + +_AGENT_SKILLS_NAMESPACE = "agent_skills" + + +def _noop_handler(**_kwargs: Any) -> dict[str, Any]: + """Placeholder handler for discovered agent skill tools. + + Agent skill tools are executed via the Agent Skills runtime; this + placeholder satisfies the ``ToolSpec.handler`` requirement. + """ + return {"status": "agent_skill_placeholder"} + + +def parse_agent_skills_paths(raw_value: str) -> list[Path]: + """Split a comma-separated path string into resolved ``Path`` objects. + + Empty segments and whitespace are stripped. Tilde (``~``) is expanded. + + Args: + raw_value: The raw config value (comma-separated directory paths). + + Returns: + List of resolved ``Path`` objects. + """ + if not raw_value: + return [] + paths: list[Path] = [] + for segment in raw_value.split(","): + segment = segment.strip() + if segment: + paths.append(Path(segment).expanduser().resolve()) + return paths + + +def scan_directory(directory: Path) -> list[DiscoveredAgentSkill]: + """Scan a single directory for Agent Skills Standard folders. + + Each immediate subdirectory containing a ``SKILL.md`` file is treated + as an agent skill. + + Args: + directory: Root directory to scan. + + Returns: + List of discovered agent skills. + """ + if not directory.is_dir(): + logger.debug("Skipping non-directory path: %s", directory) + return [] + + results: list[DiscoveredAgentSkill] = [] + for child in sorted(directory.iterdir()): + if not child.is_dir(): + continue + skill_md = child / "SKILL.md" + if not skill_md.is_file(): + continue + + data = _parse_skill_md(skill_md) + if data is None: + logger.debug("No valid front-matter in %s", skill_md) + continue + + skill = _skill_from_front_matter(data, child) + if skill is not None: + results.append(skill) + + return results + + +def discover_agent_skills( + paths: list[Path], +) -> DiscoveryResult: + """Run a full agent skills discovery scan across all configured paths. + + Args: + paths: Directories to scan for Agent Skills folders. + + Returns: + ``DiscoveryResult`` containing discovered skills and any errors. + """ + result = DiscoveryResult() + + for directory in paths: + if not directory.exists(): + result.errors.append(f"Directory does not exist: {directory}") + continue + if not directory.is_dir(): + result.errors.append(f"Path is not a directory: {directory}") + continue + + try: + skills = scan_directory(directory) + result.discovered.extend(skills) + except OSError as exc: + result.errors.append(f"Error scanning {directory}: {exc}") + + return result + + +def build_tool_spec(skill: DiscoveredAgentSkill) -> ToolSpec: + """Convert a ``DiscoveredAgentSkill`` into a ``ToolSpec``. + + The tool name uses the ``agent_skills/`` namespace pattern. + + Args: + skill: Discovered agent skill metadata. + + Returns: + A ``ToolSpec`` ready for registration. + """ + namespaced_name = f"{_AGENT_SKILLS_NAMESPACE}/{skill.name}" + return ToolSpec( + name=namespaced_name, + description=skill.description, + input_schema=skill.input_schema, + output_schema=skill.output_schema, + handler=_noop_handler, + source="agent_skills", + source_metadata={ + "path": skill.path, + "skill_name": skill.name, + }, + ) + + +def register_discovered_skills( + discovered: list[DiscoveredAgentSkill], + registry: Any, + *, + on_conflict: str = "skip", +) -> tuple[list[ToolSpec], list[DiscoveryConflict]]: + """Register discovered agent skills into a ToolRegistry. + + Args: + discovered: List of discovered agent skills. + registry: A ``ToolRegistry`` instance. + on_conflict: Conflict strategy — ``"skip"`` (default), ``"error"``, + or ``"replace"``. + + Returns: + Tuple of (registered_specs, conflicts). + + Raises: + ValueError: When *on_conflict* is ``"error"`` and a collision occurs. + """ + if not discovered: + return [], [] + + registered: list[ToolSpec] = [] + conflicts: list[DiscoveryConflict] = [] + + for skill in discovered: + spec = build_tool_spec(skill) + existing = registry.get(spec.name) + + if existing is not None: + conflict = DiscoveryConflict( + tool_name=spec.name, + existing_source=getattr(existing, "source", "unknown"), + discovered_path=skill.path, + ) + conflicts.append(conflict) + + if on_conflict == "error": + raise ValueError( + f"Agent skill name collision: '{spec.name}' already " + f"registered (source: {conflict.existing_source}). " + f"Discovered at: {skill.path}" + ) + if on_conflict == "replace": + registry.remove(spec.name) + registry.register(spec) + registered.append(spec) + else: + # skip — do not register, record conflict + logger.warning( + "Skipping agent skill '%s' — name collision with " + "existing tool (source: %s)", + spec.name, + conflict.existing_source, + ) + continue + + registry.register(spec) + registered.append(spec) + + return registered, conflicts diff --git a/src/cleveragents/skills/registry.py b/src/cleveragents/skills/registry.py index 2fc29deef9..dffa0df76a 100644 --- a/src/cleveragents/skills/registry.py +++ b/src/cleveragents/skills/registry.py @@ -32,6 +32,7 @@ from typing import Any from cleveragents.domain.models.core.skill import ( ResolvedToolEntry, + SkillCapabilitySummary, SkillResolver, ) from cleveragents.skills.context import SkillExecutionError @@ -175,6 +176,86 @@ class SkillRegistry: return resolver.resolve_tools(defn.skill, skill_lookup) + # -- Flattened tools with summary ------------------------------------------ + + def tools( + self, skill_name: str + ) -> tuple[list[ResolvedToolEntry], SkillCapabilitySummary]: + """Return flattened tool list with capability summary for a skill. + + Combines ``resolve_tools()`` and ``compute_capability_summary()`` + into a single call. + + Args: + skill_name: The namespaced skill name. + + Returns: + A 2-tuple of (resolved entries, capability summary). + + Raises: + SkillExecutionError: With ``SKILL_NOT_FOUND`` if the skill + is not registered. + """ + defn = self.get(skill_name) + resolver = SkillResolver() + skill_lookup = {name: d.skill for name, d in self._skills.items()} + entries = resolver.resolve_tools(defn.skill, skill_lookup) + summary = resolver.compute_capability_summary(defn.skill, entries) + return entries, summary + + # -- Plan validation ------------------------------------------------------- + + def validate_plan(self, plan: dict[str, Any]) -> list[str]: + """Validate that a plan's skill references are satisfiable. + + Checks: + - All referenced skills exist in the registry. + - All includes are resolvable (no missing skills). + - No cycles in include chains. + - Tool refs are valid (if ``_tool_registry`` is configured). + + Args: + plan: A plan dict with at minimum a ``'skills'`` key listing + skill names. + + Returns: + List of validation error messages (empty means valid). + """ + errors: list[str] = [] + skill_names: list[str] = plan.get("skills", []) + + for skill_name in skill_names: + if skill_name not in self._skills: + errors.append( + f"Skill '{skill_name}' referenced in plan is not registered" + ) + continue + + # Attempt full resolution to catch missing includes and cycles + defn = self._skills[skill_name] + resolver = SkillResolver() + skill_lookup = {n: d.skill for n, d in self._skills.items()} + try: + resolved = resolver.resolve_tools(defn.skill, skill_lookup) + except ValueError as exc: + errors.append(f"Skill '{skill_name}': {exc}") + continue + + # Validate tool refs against tool registry (if configured) + if self._tool_registry is not None: + for entry in resolved: + if not entry.is_inline and not entry.name.startswith( + ("mcp:", "agent_skill:") + ): + tool = self._tool_registry.get_tool(entry.name) + if tool is None: + errors.append( + f"Tool '{entry.name}' in skill " + f"'{skill_name}' not found in tool registry" + ) + + return errors + # -- Validation ------------------------------------------------------------ def validate_skill(self, skill: SkillDefinition) -> list[str]: diff --git a/src/cleveragents/tool/registry.py b/src/cleveragents/tool/registry.py index b21bb5b5d0..288aca6f13 100644 --- a/src/cleveragents/tool/registry.py +++ b/src/cleveragents/tool/registry.py @@ -59,6 +59,7 @@ class ToolRegistry: self, namespace: str | None = None, tool_type: str | None = None, + source: str | None = None, ) -> list[ToolSpec]: """List registered tools with optional filters. @@ -70,6 +71,8 @@ class ToolRegistry: tool_type: Reserved for future use (e.g. filtering by ``tool`` vs ``validation``). Currently unused. + source: + If provided, only return tools whose ``source`` field matches. """ with self._lock: specs = list(self._tools.values()) @@ -78,6 +81,9 @@ class ToolRegistry: prefix = f"{namespace}/" specs = [s for s in specs if s.name.startswith(prefix)] + if source is not None: + specs = [s for s in specs if s.source == source] + # tool_type filtering is a no-op for now but the parameter is # part of the public API to avoid a breaking change later. _ = tool_type diff --git a/src/cleveragents/tool/runtime.py b/src/cleveragents/tool/runtime.py index 93adee47f1..eb04287b8c 100644 --- a/src/cleveragents/tool/runtime.py +++ b/src/cleveragents/tool/runtime.py @@ -41,6 +41,16 @@ class ToolSpec(BaseModel): Combines the domain-level metadata (name, schemas, capabilities) with an actual callable *handler* that the ``ToolRunner`` invokes. + + The optional ``source`` field tracks where the tool originated: + + - ``"builtin"`` — shipped with the CleverAgents runtime. + - ``"agent_skills"`` — discovered from an Agent Skills Standard folder. + - ``"mcp"`` — imported from an MCP server. + - ``"custom"`` — user-defined inline tool. + + When ``source_metadata`` is populated it carries provenance details + such as the filesystem path or MCP server URI that the tool came from. """ name: str = Field( @@ -69,6 +79,14 @@ class ToolSpec(BaseModel): ..., description="Callable that executes the tool logic", ) + source: str = Field( + default="builtin", + description=("Origin of the tool: builtin, agent_skills, mcp, or custom"), + ) + source_metadata: dict[str, Any] = Field( + default_factory=dict, + description=("Provenance details for the tool (e.g. path, server URI)"), + ) model_config = ConfigDict( str_strip_whitespace=True, diff --git a/vulture_whitelist.py b/vulture_whitelist.py index 9f8240c8c5..4210c867ac 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -267,3 +267,13 @@ normalize_tool_schema_for_provider # noqa: B018, F821 route_batch # noqa: B018, F821 route_streaming # noqa: B018, F821 export_schemas # noqa: B018, F821 + +# Actor compiler — public API surface used by CLI, tests, and benchmarks +compile_actor # noqa: B018, F821 +CompilationMetadata # noqa: B018, F821 +CompiledActor # noqa: B018, F821 +ActorCompilationError # noqa: B018, F821 +SubgraphCycleError # noqa: B018, F821 +MissingNodeError # noqa: B018, F821 +InvalidEntryExitError # noqa: B018, F821 +ActorResolver # noqa: B018, F821