Skip to content

Commit 14809f5

Browse files
Copilotekzhu
andauthored
Fix GraphFlow cycle detection to properly clean up recursion state (#7026)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ekzhu <320302+ekzhu@users.noreply.github.com> Co-authored-by: Eric Zhu <ekzhu@users.noreply.github.com>
1 parent 79d5d6a commit 14809f5

File tree

2 files changed

+49
-3
lines changed

2 files changed

+49
-3
lines changed

python/packages/autogen-agentchat/src/autogen_agentchat/teams/_group_chat/_graph/_digraph_group_chat.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,13 @@ def dfs(node_name: str) -> bool:
165165
visited.add(node_name)
166166
rec_stack.add(node_name)
167167
path.append(node_name)
168+
cycle = False
168169

169170
for edge in self.nodes[node_name].edges:
170171
target = edge.target
171172
if target not in visited:
172173
if dfs(target):
173-
return True
174+
cycle = True
174175
elif target in rec_stack:
175176
# Found a cycle → extract the cycle
176177
cycle_start_index = path.index(target)
@@ -182,11 +183,11 @@ def dfs(node_name: str) -> bool:
182183
raise ValueError(
183184
f"Cycle detected without exit condition: {' -> '.join(cycle_nodes + cycle_nodes[:1])}"
184185
)
185-
return True # Found cycle, but it has an exit condition
186+
cycle = True # Found cycle, but it has an exit condition
186187

187188
rec_stack.remove(node_name)
188189
path.pop()
189-
return False
190+
return cycle
190191

191192
has_cycle = False
192193
for node in self.nodes:

python/packages/autogen-agentchat/tests/test_group_chat_graph.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,51 @@ def test_cycle_detection_without_exit_condition() -> None:
247247
graph.has_cycles_with_exit()
248248

249249

250+
def test_cycle_detection_cleanup_bug() -> None:
251+
"""Test that cycle detection properly cleans up recursion state.
252+
253+
This test reproduces the bug where the DFS algorithm in has_cycles_with_exit
254+
didn't properly clean up rec_stack and path when returning early upon finding
255+
a cycle with valid exit conditions. The bug could cause incorrect behavior
256+
when processing subsequent unvisited nodes in graphs with multiple components.
257+
"""
258+
259+
# Create a graph that exposes the cleanup bug:
260+
# A -> B -> C -> A (cycle with condition)
261+
# A -> D (separate branch that could be affected by stale rec_stack/path)
262+
# E (disconnected component that could be affected by cleanup issues)
263+
graph = DiGraph(
264+
nodes={
265+
"A": DiGraphNode(name="A", edges=[DiGraphEdge(target="B"), DiGraphEdge(target="D")]),
266+
"B": DiGraphNode(name="B", edges=[DiGraphEdge(target="C")]),
267+
"C": DiGraphNode(name="C", edges=[DiGraphEdge(target="A", condition="loop")]),
268+
"D": DiGraphNode(name="D", edges=[]),
269+
"E": DiGraphNode(name="E", edges=[]), # Disconnected component
270+
}
271+
)
272+
273+
# This should work correctly with the fix - proper cleanup ensures
274+
# that processing node E is not affected by stale recursion state
275+
# from processing the A->B->C->A cycle
276+
result = graph.has_cycles_with_exit()
277+
assert result is True # Has valid cycles
278+
279+
# Test with multiple cycles to ensure thorough cleanup
280+
multi_cycle_graph = DiGraph(
281+
nodes={
282+
"A": DiGraphNode(name="A", edges=[DiGraphEdge(target="B")]),
283+
"B": DiGraphNode(name="B", edges=[DiGraphEdge(target="A", condition="cycle1")]),
284+
"C": DiGraphNode(name="C", edges=[DiGraphEdge(target="D")]),
285+
"D": DiGraphNode(name="D", edges=[DiGraphEdge(target="C", condition="cycle2")]),
286+
"E": DiGraphNode(name="E", edges=[DiGraphEdge(target="F")]),
287+
"F": DiGraphNode(name="F", edges=[]),
288+
}
289+
)
290+
291+
result = multi_cycle_graph.has_cycles_with_exit()
292+
assert result is True # Has valid cycles
293+
294+
250295
def test_different_activation_groups_detection() -> None:
251296
"""Test different activation groups."""
252297
graph = DiGraph(

0 commit comments

Comments
 (0)