diff --git a/AGENTS.md b/AGENTS.md index d022c77f..f25b7cbb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -336,7 +336,7 @@ A resolved agent graph returned by `resolve_graph()`. A class with attribute acc | `run_node(node, input?, opts?)` | Executes a single node through the tracked `config().invoke()` path. | | `route(node, input?, opts?)` | Executes a node, presenting outgoing edges as handoff choices; returns the response plus the chosen `next` node. | | `traverse(fn, ctx?)` | Awaits each visitor in BFS order (root → leaves). Visitor may be sync or async. | -| `reverse_traverse(fn, ctx?)` | Awaits each visitor in reverse BFS order (leaves → root). Visitor may be sync or async. | +| `reverse_traverse(fn, ctx?)` | Awaits each visitor, every child before its parents (post-order from the root, each node once, root last). Visitor may be sync or async. | #### `GraphOptions` diff --git a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py index ac6694e4..aea24b7d 100644 --- a/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py +++ b/packages/claude-agents/src/launchdarkly_ai_claude_agents/native_graph.py @@ -5,6 +5,7 @@ from __future__ import annotations +import logging import re import time import types @@ -48,6 +49,8 @@ MCP_TOOL_PREFIX = f"mcp__{TOOL_MCP_NAME}__" SUBAGENT_TOOL_PREFIX = f"mcp__{SUBAGENT_MCP_NAME}__" +logger = logging.getLogger(__name__) + def _sanitize_name(key: str) -> str: return re.sub(r"[^a-z0-9_-]", "_", key, flags=re.IGNORECASE) @@ -276,11 +279,15 @@ async def _build_node(node: GraphNode) -> None: return node_key = _sanitize_name(node.key) - child_tools = [ - subagent_tool_ctx[e.target_key] - for e in node.edges - if e.target_key in subagent_tool_ctx - ] + child_tools = [] + for e in node.edges: + if e.target_key not in subagent_tool_ctx: + # The root is never a tool, so only an edge back to the root or into a cycle lands here. + logger.warning( + 'Dropping cyclic edge "%s" -> "%s"', node.key, e.target_key + ) + continue + child_tools.append(subagent_tool_ctx[e.target_key]) async def _subagent_execute( args: Any, _node: GraphNode = node diff --git a/packages/claude-agents/tests/test_native_graph.py b/packages/claude-agents/tests/test_native_graph.py index 04117b91..81b14831 100644 --- a/packages/claude-agents/tests/test_native_graph.py +++ b/packages/claude-agents/tests/test_native_graph.py @@ -549,6 +549,42 @@ def register(fn: Any) -> Any: assert len(created_tools) == 1 assert "child" in created_tools[0] + @pytest.mark.asyncio + async def test_cyclic_edge_is_dropped_with_a_warning( + self, caplog: pytest.LogCaptureFixture + ) -> None: + mock_sdk = _make_sdk_mock("done") + + # root -> a -> b -> a: post-order builds b while a is still unbuilt. + def _node(key: str, target: str) -> dict[str, Any]: + return { + "key": key, + "config": {"model": {"name": "claude-3"}, "instructions": key}, + "meta": {}, + "edges": [{"source_key": key, "target_key": target}], + "is_terminal": False, + } + + edges = [("root", "a"), ("a", "b"), ("b", "a")] + graph_def = _make_graph_def( + nodes={ + "root": _node("root", "a"), + "a": _node("a", "b"), + "b": _node("b", "a"), + }, + edges=[{"source_key": s, "target_key": t} for s, t in edges], + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: ( + mock_sdk if n == "claude_agent_sdk" else __import__(n) + ), + ): + await to_claude_agents(_make_def_promise(graph_def)).invoke("hi") + + assert 'Dropping cyclic edge "b" -> "a"' in caplog.text + @pytest.mark.asyncio async def test_emits_handoff_success_when_sub_agent_tool_invoked(self) -> None: track_calls: list[tuple[str, Any]] = [] diff --git a/packages/client/src/launchdarkly_ai_server/graph.py b/packages/client/src/launchdarkly_ai_server/graph.py index 46be8969..c75adce0 100644 --- a/packages/client/src/launchdarkly_ai_server/graph.py +++ b/packages/client/src/launchdarkly_ai_server/graph.py @@ -751,7 +751,7 @@ async def stream_route( raise # ── traverse / reverse_traverse ─────────────────────────────────────────── - # Async BFS visitors that mirror the TS GraphDefinition.traverse / + # Async visitors that mirror the TS GraphDefinition.traverse / # reverseTraverse. The visitor fn receives (node, ctx) and its return value # is stored in ctx[node_key]. Both handle sync and async visitors. @@ -791,33 +791,23 @@ async def traverse(fn: Any, ctx: dict[str, Any] | None = None) -> Any: return ctx.get(root_node.key) async def reverse_traverse(fn: Any, ctx: dict[str, Any] | None = None) -> Any: - """Visit nodes leaf-first (terminal nodes first, root last).""" + """Visit every child before its parents (post-order DFS, each node once, root last).""" if root_node is None: return None ctx = ctx if ctx is not None else {} - terminals = terminal_nodes() - if not terminals: - return None visited: set[str] = set() - frontier: list[str] = [n.key for n in terminals] - iterations = 0 - while frontier and iterations < MAX_TRAVERSAL_DEPTH: - iterations += 1 - next_frontier: list[str] = [] - for node_key in frontier: - if node_key in visited: - continue - visited.add(node_key) - if node_key == root_node.key: - continue - node = nodes.get(node_key) - if node: - ctx[node_key] = await _call_visitor(fn, node, ctx) - for parent in get_parent_nodes(node_key): - if parent.key not in visited: - next_frontier.append(parent.key) - frontier = next_frontier - ctx[root_node.key] = await _call_visitor(fn, root_node, ctx) + + async def visit(node_key: str) -> None: + if node_key in visited: + return + visited.add(node_key) + for child in get_child_nodes(node_key): + await visit(child.key) + node = nodes.get(node_key) + if node: + ctx[node_key] = await _call_visitor(fn, node, ctx) + + await visit(root_node.key) return ctx.get(root_node.key) graph_def = GraphDefinition( diff --git a/packages/client/tests/test_graph.py b/packages/client/tests/test_graph.py index c3379682..71586df1 100644 --- a/packages/client/tests/test_graph.py +++ b/packages/client/tests/test_graph.py @@ -439,6 +439,38 @@ async def visitor(node: Any, ctx: Any) -> None: await gd.reverse_traverse(visitor) assert visited.index("leaf-node") < visited.index("root-node") + @pytest.mark.parametrize( + ("edges", "expected"), + [ + # Uneven depths: P has a terminal child and a deeper one. + ( + {"root": ["P"], "P": ["T1", "Q"], "Q": ["T2"]}, + ["T1", "T2", "Q", "P", "root"], + ), + # Cycle: each node is visited exactly once. + ({"root": ["A"], "A": ["B"], "B": ["A", "T"]}, ["T", "B", "A", "root"]), + ], + ) + async def test_reverse_traverse_visits_every_child_before_its_parent( + self, edges: dict[str, list[str]], expected: list[str] + ) -> None: + topology = { + "root": "root", + "edges": {k: [{"key": t} for t in ts] for k, ts in edges.items()}, + } + lifecycle_module._set_client_for_testing(_make_client(topology)) + try: + gd = await resolve_graph("graph-key", context=CONTEXT) + visited: list[str] = [] + + async def visitor(node: Any, ctx: Any) -> None: + visited.append(node.key) + + await gd.reverse_traverse(visitor) + assert visited == expected + finally: + lifecycle_module._reset_for_testing() + async def test_reverse_traverse_key_is_snake_case( self, mock_ld_client: MagicMock ) -> None: diff --git a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py index e7225936..a1520b64 100644 --- a/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py +++ b/packages/openai-agents/src/launchdarkly_ai_openai_agents/native_graph.py @@ -6,6 +6,7 @@ from __future__ import annotations import inspect +import logging import re import time import types @@ -37,6 +38,8 @@ except ImportError: _HAS_OTEL = False +logger = logging.getLogger(__name__) + def _sanitize_name(key: str) -> str: return re.sub(r"[^a-z0-9_-]", "_", key, flags=re.IGNORECASE)[:64] @@ -182,9 +185,13 @@ async def _visit(node_key: str) -> None: for edge in edges_from(node_key): child_agent = agent_ctx.get(edge.target_key) if child_agent is None: - raise ValueError( - f'Child agent "{edge.target_key}" not built before parent "{node_key}"' + # Unbuilt only when the edge closes a cycle; drop it so the graph still builds. + logger.warning( + 'Dropping cyclic edge "%s" -> "%s"', + node_key, + edge.target_key, ) + continue child_handoffs.append(handoff_fn(child_agent)) instructions = _build_instructions(node, vs) diff --git a/packages/openai-agents/tests/test_native_graph.py b/packages/openai-agents/tests/test_native_graph.py index c1413b24..e0dac6e8 100644 --- a/packages/openai-agents/tests/test_native_graph.py +++ b/packages/openai-agents/tests/test_native_graph.py @@ -339,6 +339,42 @@ async def test_handoff_tool_injected_for_edges(self) -> None: # handoff() should be called for the child agent agents_mock.handoff.assert_called() + @pytest.mark.asyncio + async def test_cyclic_edge_is_dropped_with_a_warning( + self, caplog: pytest.LogCaptureFixture + ) -> None: + agents_mock = _make_agents_mock(_make_run_result("out")) + + # root -> a -> b -> a: post-order builds b while a is still unbuilt. + def _node(key: str, target: str) -> dict[str, Any]: + return { + "key": key, + "config": {"model": {"name": "gpt-4o"}, "instructions": key}, + "meta": {}, + "edges": [{"source_key": key, "target_key": target}], + "is_terminal": False, + } + + edges = [("root", "a"), ("a", "b"), ("b", "a")] + graph_def = _make_graph_def( + nodes={ + "root": _node("root", "a"), + "a": _node("a", "b"), + "b": _node("b", "a"), + }, + edges=[{"source_key": s, "target_key": t} for s, t in edges], + ) + + with patch( + "importlib.import_module", + side_effect=lambda n: agents_mock if n == "agents" else __import__(n), + ): + await to_openai_agents(_make_def_promise(graph_def)).invoke("hi") + + assert [a.name for a in agents_mock._created_agents] == ["b", "a", "root"] + assert "handoffs" not in agents_mock._created_agents[0]._kw + assert 'Dropping cyclic edge "b" -> "a"' in caplog.text + @pytest.mark.asyncio async def test_runner_starts_at_root_and_returns_output(self) -> None: run_result = _make_run_result("final-output")