Compare commits

...

2 Commits

Author SHA1 Message Date
HAL9000 a7fb57ea9c fix(actor): remove type: ignore from subgraph cycle detection steps
CI / push-validation (pull_request) Successful in 35s
CI / helm (pull_request) Successful in 46s
CI / build (pull_request) Successful in 59s
CI / lint (pull_request) Successful in 1m8s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 1m25s
CI / typecheck (pull_request) Successful in 1m42s
CI / security (pull_request) Successful in 1m43s
CI / integration_tests (pull_request) Successful in 3m54s
CI / e2e_tests (pull_request) Successful in 4m20s
CI / unit_tests (pull_request) Successful in 7m18s
CI / docker (pull_request) Successful in 1m28s
CI / coverage (pull_request) Successful in 11m24s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Failing after 38m41s
Remove # type: ignore[import-untyped] suppression comments from
features/steps/actor_subgraph_cycle_detection_steps.py per
CONTRIBUTING.md zero-tolerance policy on inline type suppressions.
Pyright does not check the features/ directory so these suppressions
were unnecessary and violated project policy.
2026-05-05 11:21:17 +00:00
HAL9000 a1093430bd fix(actor): read actor_ref from NodeDefinition field in _detect_subgraph_cycles
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 45s
CI / typecheck (pull_request) Successful in 1m7s
CI / build (pull_request) Successful in 46s
CI / security (pull_request) Successful in 1m18s
CI / push-validation (pull_request) Successful in 21s
CI / quality (pull_request) Successful in 1m22s
CI / helm (pull_request) Successful in 42s
CI / integration_tests (pull_request) Successful in 4m16s
CI / e2e_tests (pull_request) Successful in 5m19s
CI / unit_tests (pull_request) Successful in 10m35s
CI / coverage (pull_request) Successful in 13m47s
CI / docker (pull_request) Successful in 2m3s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h4m16s
Fix cross-actor subgraph cycle detection in the actor compiler by reading
actor_ref from the top-level NodeDefinition field instead of the config dict.

The _detect_subgraph_cycles(), _map_node(), and compile_actor() functions
were all reading node.config.get("actor_ref", "") instead of node.actor_ref.
Because actor_ref is a typed, validated Pydantic field on NodeDefinition (not
a key inside the untyped config dict), the old code always returned an empty
string, causing cross-actor cycle detection to silently fail and leaving the
system vulnerable to infinite recursion at runtime.

Changes:
- Fix all three call sites in src/cleveragents/actor/compiler.py
- Add Behave regression tests (features/actor_subgraph_cycle_detection.feature)
  with @tdd_issue and @tdd_issue_1431 tags on all scenarios
- Add Robot Framework integration test (robot/actor_compiler.robot)
- Fix existing test helpers to use actor_ref as top-level field
- Add CHANGELOG entry

ISSUES CLOSED: #1431
2026-05-05 01:36:48 +00:00
8 changed files with 356 additions and 7 deletions
+13
View File
@@ -5,6 +5,19 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
## [Unreleased]
### Fixed
- **Cross-actor subgraph cycle detection reads actor_ref field** (#1431): Fixed
`_detect_subgraph_cycles()`, `_map_node()`, and the `compile_actor()` main loop
in `src/cleveragents/actor/compiler.py` to read `actor_ref` from the top-level
`NodeDefinition.actor_ref` field instead of `node.config.get("actor_ref", "")`.
Because `actor_ref` is a typed, validated Pydantic field (not a key inside the
untyped `config` dict), the old code always returned an empty string, causing
cross-actor cycle detection to silently fail and leaving the system vulnerable to
infinite recursion at runtime. Added Behave regression tests
(`features/actor_subgraph_cycle_detection.feature`) and a Robot Framework
integration test (`robot/actor_compiler.robot`) to prevent regressions.
### Changed
- **Diagnostics spec examples expanded to all 9 providers** (#5320): Updated the
@@ -0,0 +1,38 @@
Feature: Cross-actor subgraph cycle detection reads actor_ref field
As a CleverAgents developer
I want the actor compiler to correctly detect cross-actor subgraph cycles
So that mutually-referencing actors raise SubgraphCycleError at compile time
Background:
Given the actor compiler is available
# ────────────────────────────────────────────────────────────
# Bug fix: actor_ref is a top-level NodeDefinition field
# ────────────────────────────────────────────────────────────
@tdd_issue @tdd_issue_1431
Scenario: Mutually-referencing actors raise SubgraphCycleError
Given actor "test/actor-a" has a subgraph node with actor_ref "test/actor-b"
And actor "test/actor-b" has a subgraph node with actor_ref "test/actor-a"
And a registry containing both actors
When I compile actor "test/actor-a" with the registry resolver
Then the compilation should raise SubgraphCycleError
And the cycle error message should mention "cycle"
@tdd_issue @tdd_issue_1431
Scenario: Non-cyclic subgraph reference compiles successfully
Given actor "test/actor-x" has a subgraph node with actor_ref "test/actor-y"
And actor "test/actor-y" has no subgraph nodes
And a registry containing both actors
When I compile actor "test/actor-x" with the registry resolver
Then the actor compilation should succeed
And the actor subgraph_refs should map "sub" to "test/actor-y"
@tdd_issue @tdd_issue_1431
Scenario: Subgraph node actor_ref is reflected in compiled metadata
Given actor "test/actor-p" has a subgraph node with actor_ref "test/actor-q"
And actor "test/actor-q" has no subgraph nodes
And a registry containing both actors
When I compile actor "test/actor-p" with the registry resolver
Then the actor compilation should succeed
And the compiled node "sub" should have subgraph set to "test/actor-q"
@@ -135,7 +135,8 @@ def _subgraph_node(node_id: str, actor_ref: str = "") -> NodeDefinition:
type=NodeType.SUBGRAPH,
name=node_id.title(),
description=f"Subgraph {node_id}",
config={"actor_ref": actor_ref},
config={},
actor_ref=actor_ref if actor_ref else None,
)
+6 -3
View File
@@ -186,7 +186,8 @@ def step_given_subgraph_ref(context: Context, ref_name: str) -> None:
type=NodeType.SUBGRAPH,
name="Sub",
description="Subgraph",
config={"actor_ref": ref_name},
config={},
actor_ref=ref_name,
),
]
edges = [EdgeDefinition(from_node="main", to_node="sub")]
@@ -241,7 +242,8 @@ def step_given_outer_referencing_inner(
type=NodeType.SUBGRAPH,
name="Sub",
description="Subgraph",
config={"actor_ref": inner_name},
config={},
actor_ref=inner_name,
),
]
edges = [EdgeDefinition(from_node="main", to_node="sub")]
@@ -260,7 +262,8 @@ def step_given_resolver_cycle(context: Context, inner_name: str, back_ref: str)
type=NodeType.SUBGRAPH,
name="Child",
description="Back-ref",
config={"actor_ref": back_ref},
config={},
actor_ref=back_ref,
),
]
inner = _build_graph_config(inner_name, inner_nodes, [], "child", ["child"])
@@ -0,0 +1,201 @@
"""Step definitions for cross-actor subgraph cycle detection tests.
Tests for features/actor_subgraph_cycle_detection.feature — validates that
the actor compiler correctly reads actor_ref from the top-level
NodeDefinition field (not from node.config) when detecting cross-actor
subgraph cycles.
This is the regression test for issue #1431: _detect_subgraph_cycles()
was reading node.config.get("actor_ref", "") instead of node.actor_ref,
causing cycle detection to always return an empty string and never detect
cross-actor cycles.
"""
from __future__ import annotations
from behave import given, then, when
from behave.runner import Context
from cleveragents.actor.compiler import (
SubgraphCycleError,
compile_actor,
)
from cleveragents.actor.schema import (
ActorConfigSchema,
ActorType,
EdgeDefinition,
NodeDefinition,
NodeType,
RouteDefinition,
)
# ────────────────────────────────────────────────────────────
# Helpers
# ────────────────────────────────────────────────────────────
def _make_graph_actor(
name: str,
nodes: list[NodeDefinition],
edges: list[EdgeDefinition],
entry_node: str,
exit_nodes: list[str],
) -> ActorConfigSchema:
"""Build a minimal valid GRAPH actor for testing."""
route = RouteDefinition(
nodes=nodes,
edges=edges,
entry_node=entry_node,
exit_nodes=exit_nodes,
)
return ActorConfigSchema(
name=name,
type=ActorType.GRAPH,
description=f"Test actor {name}",
provider="openai",
model="gpt-4",
route=route,
)
def _make_agent_node(node_id: str) -> NodeDefinition:
"""Build a minimal AGENT node."""
return NodeDefinition(
id=node_id,
type=NodeType.AGENT,
name=node_id.title(),
description=f"Agent {node_id}",
config={"agent": "default"},
)
def _make_subgraph_node(node_id: str, actor_ref: str) -> NodeDefinition:
"""Build a SUBGRAPH node using the top-level actor_ref field.
The actor_ref is set as a first-class field on NodeDefinition, NOT
inside config. This is the correct way to reference a subgraph actor.
"""
return NodeDefinition(
id=node_id,
type=NodeType.SUBGRAPH,
name=node_id.title(),
description=f"Subgraph {node_id}",
config={},
actor_ref=actor_ref,
)
# ────────────────────────────────────────────────────────────
# Given steps
# ────────────────────────────────────────────────────────────
@given("the actor compiler is available")
def step_compiler_available(context: Context) -> None:
"""Ensure the actor compiler module is importable."""
context.actor_registry: dict[str, ActorConfigSchema] = {}
@given('actor "{name}" has a subgraph node with actor_ref "{ref}"')
def step_actor_has_subgraph_ref(context: Context, name: str, ref: str) -> None:
"""Create a GRAPH actor with one agent node and one subgraph node."""
agent = _make_agent_node("start")
sub = _make_subgraph_node("sub", actor_ref=ref)
actor = _make_graph_actor(
name,
nodes=[agent, sub],
edges=[EdgeDefinition(from_node="start", to_node="sub")],
entry_node="start",
exit_nodes=["sub"],
)
context.actor_registry[name] = actor
context.actor_to_compile = name
@given('actor "{name}" has no subgraph nodes')
def step_actor_has_no_subgraph(context: Context, name: str) -> None:
"""Create a GRAPH actor with only an agent node (no subgraph references)."""
agent = _make_agent_node("leaf")
actor = _make_graph_actor(
name,
nodes=[agent],
edges=[],
entry_node="leaf",
exit_nodes=["leaf"],
)
context.actor_registry[name] = actor
@given("a registry containing both actors")
def step_registry_contains_both(context: Context) -> None:
"""Set up the resolver from the accumulated registry."""
registry = context.actor_registry
def resolver(actor_name: str) -> ActorConfigSchema | None:
return registry.get(actor_name)
context.resolver = resolver
# ────────────────────────────────────────────────────────────
# When steps
# ────────────────────────────────────────────────────────────
@when('I compile actor "{name}" with the registry resolver')
def step_compile_actor(context: Context, name: str) -> None:
"""Compile the named actor using the registry resolver."""
context.compile_error = None
context.compiled = None
actor = context.actor_registry[name]
try:
context.compiled = compile_actor(actor, actor_resolver=context.resolver)
except Exception as exc:
context.compile_error = exc
# ────────────────────────────────────────────────────────────
# Then steps
# ────────────────────────────────────────────────────────────
@then("the compilation should raise SubgraphCycleError")
def step_raises_cycle_error(context: Context) -> None:
"""Assert that compilation raised SubgraphCycleError."""
assert context.compile_error is not None, (
"Expected SubgraphCycleError but compilation succeeded"
)
assert isinstance(context.compile_error, SubgraphCycleError), (
f"Expected SubgraphCycleError, got {type(context.compile_error).__name__}: "
f"{context.compile_error}"
)
@then("the actor compilation should succeed")
def step_actor_compilation_succeeds(context: Context) -> None:
"""Assert that compilation succeeded without errors."""
assert context.compile_error is None, (
f"Expected compilation to succeed but got: {context.compile_error}"
)
assert context.compiled is not None
@then('the cycle error message should mention "{text}"')
def step_cycle_error_mentions(context: Context, text: str) -> None:
"""Assert that the cycle error message contains the given text."""
assert context.compile_error is not None
msg = str(context.compile_error)
assert text.lower() in msg.lower(), (
f"Expected '{text}' in error message, got: {msg}"
)
@then('the actor subgraph_refs should map "{node}" to "{ref}"')
def step_actor_subgraph_refs_map(context: Context, node: str, ref: str) -> None:
"""Assert that the compiled metadata maps the node to the expected actor ref."""
assert context.compiled is not None
refs = context.compiled.metadata.subgraph_refs
assert refs.get(node) == ref, (
f"Expected subgraph_refs[{node!r}] == {ref!r}, got {refs}"
)
+12
View File
@@ -42,3 +42,15 @@ Reject LLM Actor Compilation
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-compiler-expected-fail
Should Contain ${result.stdout} GRAPH
Detect Cross-Actor Subgraph Cycle Via actor_ref Field
[Documentation] Verify that mutually-referencing actors raise SubgraphCycleError.
... This is the regression test for issue #1431: _detect_subgraph_cycles()
... was reading node.config.get("actor_ref") instead of node.actor_ref,
... causing cycle detection to silently fail.
[Tags] slow
${result}= Run Process ${PYTHON} ${HELPER} cycle-detect dummy cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-compiler-cycle-detected
+76
View File
@@ -66,6 +66,82 @@ def main() -> int:
print(f"actor-compiler-fail: {exc}")
return 1
if command == "cycle-detect":
# Test cross-actor cycle detection using actor_ref top-level field.
# Creates two mutually-referencing actors and verifies SubgraphCycleError
# is raised, confirming the fix for issue #1431.
from cleveragents.actor.compiler import SubgraphCycleError
from cleveragents.actor.schema import (
ActorType,
EdgeDefinition,
NodeDefinition,
NodeType,
RouteDefinition,
)
def _make_actor(
name: str, subgraph_ref: str | None = None
) -> ActorConfigSchema:
if subgraph_ref is not None:
nodes = [
NodeDefinition(
id="start",
type=NodeType.AGENT,
name="Start",
description="Start",
config={"agent": "default"},
),
NodeDefinition(
id="sub",
type=NodeType.SUBGRAPH,
name="Sub",
description="Subgraph",
config={},
actor_ref=subgraph_ref,
),
]
edges = [EdgeDefinition(from_node="start", to_node="sub")]
entry, exits = "start", ["sub"]
else:
nodes = [
NodeDefinition(
id="leaf",
type=NodeType.AGENT,
name="Leaf",
description="Leaf",
config={"agent": "default"},
)
]
edges = []
entry, exits = "leaf", ["leaf"]
route = RouteDefinition(
nodes=nodes, edges=edges, entry_node=entry, exit_nodes=exits
)
return ActorConfigSchema(
name=name,
type=ActorType.GRAPH,
description=f"Test actor {name}",
model="gpt-4",
route=route,
)
actor_a = _make_actor("test/actor-a", subgraph_ref="test/actor-b")
actor_b = _make_actor("test/actor-b", subgraph_ref="test/actor-a")
registry = {"test/actor-a": actor_a, "test/actor-b": actor_b}
try:
compile_actor(actor_a, actor_resolver=registry.get)
print(
"actor-compiler-cycle-not-detected: FAIL — expected SubgraphCycleError"
)
return 1
except SubgraphCycleError as exc:
print(f"actor-compiler-cycle-detected: {exc}")
return 0
except Exception as exc:
print(f"actor-compiler-unexpected-error: {exc}")
return 1
print(f"Unknown command: {command}")
return 1
+8 -3
View File
@@ -137,7 +137,7 @@ def _map_node(node: NodeDefinition) -> lg_nodes.NodeConfig:
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),
subgraph=(node.actor_ref if node.type == NodeType.SUBGRAPH else None),
metadata=dict(config),
)
@@ -187,6 +187,11 @@ def _detect_subgraph_cycles(
"""Detect cycles in subgraph references across actors.
Returns list of actor names forming a cycle, or empty list.
The actor reference is read from the top-level actor_ref field on
NodeDefinition, not from node.config. Reading from config
would always return an empty string because actor_ref is stored as a
first-class field on the schema model.
"""
if resolver is None:
return []
@@ -194,7 +199,7 @@ def _detect_subgraph_cycles(
for node in route_nodes:
if node.type != NodeType.SUBGRAPH:
continue
ref_name = node.config.get("actor_ref", "")
ref_name = node.actor_ref or ""
if not ref_name:
continue
if ref_name in visited:
@@ -290,7 +295,7 @@ def compile_actor(
all_lsp_bindings.extend(bindings)
if node_def.type == NodeType.SUBGRAPH:
ref = node_def.config.get("actor_ref", "")
ref = node_def.actor_ref or ""
if ref:
subgraph_refs[node_def.id] = ref