feat(beads): add cycle detection for molecule dependencies
Implement DFS-based cycle detection in ValidateMolecule to catch circular dependencies in molecule step graphs. The algorithm uses three-color marking (unvisited/visiting/visited) to detect back edges that indicate cycles. When a cycle is detected, the error message shows the cycle path (e.g., "a -> b -> c -> a") for easy debugging. Add 4 new tests: - SimpleCycle: A -> B -> A - LongerCycle: A -> B -> C -> A - DiamondNoCycle: ensures valid diamond patterns pass - CycleInSubgraph: cycle not involving root node Closes gt-ai1z. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -299,7 +299,74 @@ func ValidateMolecule(mol *Issue) error {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: Detect cycles in dependency graph
|
||||
// Detect cycles in dependency graph
|
||||
if err := detectCycles(steps); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// detectCycles checks for circular dependencies in the step graph using DFS.
|
||||
// Returns an error describing the cycle if one is found.
|
||||
func detectCycles(steps []MoleculeStep) error {
|
||||
// Build adjacency list: step -> steps it depends on
|
||||
deps := make(map[string][]string)
|
||||
for _, step := range steps {
|
||||
deps[step.Ref] = step.Needs
|
||||
}
|
||||
|
||||
// Track visit state: 0 = unvisited, 1 = visiting (in stack), 2 = visited
|
||||
state := make(map[string]int)
|
||||
|
||||
// DFS from each node to find cycles
|
||||
var path []string
|
||||
var dfs func(node string) error
|
||||
|
||||
dfs = func(node string) error {
|
||||
if state[node] == 2 {
|
||||
return nil // Already fully processed
|
||||
}
|
||||
if state[node] == 1 {
|
||||
// Found a back edge - there's a cycle
|
||||
// Build cycle path for error message
|
||||
cycleStart := -1
|
||||
for i, n := range path {
|
||||
if n == node {
|
||||
cycleStart = i
|
||||
break
|
||||
}
|
||||
}
|
||||
cycle := append(path[cycleStart:], node)
|
||||
return fmt.Errorf("cycle detected in step dependencies: %s", formatCycle(cycle))
|
||||
}
|
||||
|
||||
state[node] = 1 // Mark as visiting
|
||||
path = append(path, node)
|
||||
|
||||
for _, dep := range deps[node] {
|
||||
if err := dfs(dep); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
path = path[:len(path)-1] // Pop from path
|
||||
state[node] = 2 // Mark as visited
|
||||
return nil
|
||||
}
|
||||
|
||||
for _, step := range steps {
|
||||
if state[step.Ref] == 0 {
|
||||
if err := dfs(step.Ref); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// formatCycle formats a cycle path as "a -> b -> c -> a".
|
||||
func formatCycle(cycle []string) string {
|
||||
return strings.Join(cycle, " -> ")
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ package beads
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -489,3 +490,108 @@ Has content.`
|
||||
t.Errorf("step[1].Instructions = %q", steps[1].Instructions)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateMolecule_SimpleCycle(t *testing.T) {
|
||||
// A -> B -> A (simple 2-node cycle)
|
||||
mol := &Issue{
|
||||
ID: "mol-xyz",
|
||||
Type: "molecule",
|
||||
Description: `## Step: a
|
||||
First step.
|
||||
Needs: b
|
||||
|
||||
## Step: b
|
||||
Second step.
|
||||
Needs: a`,
|
||||
}
|
||||
|
||||
err := ValidateMolecule(mol)
|
||||
if err == nil {
|
||||
t.Error("ValidateMolecule() = nil, want error for cycle")
|
||||
}
|
||||
if err != nil && !strings.Contains(err.Error(), "cycle") {
|
||||
t.Errorf("error %q should mention 'cycle'", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateMolecule_LongerCycle(t *testing.T) {
|
||||
// A -> B -> C -> A (3-node cycle)
|
||||
mol := &Issue{
|
||||
ID: "mol-xyz",
|
||||
Type: "molecule",
|
||||
Description: `## Step: a
|
||||
First step.
|
||||
Needs: c
|
||||
|
||||
## Step: b
|
||||
Second step.
|
||||
Needs: a
|
||||
|
||||
## Step: c
|
||||
Third step.
|
||||
Needs: b`,
|
||||
}
|
||||
|
||||
err := ValidateMolecule(mol)
|
||||
if err == nil {
|
||||
t.Error("ValidateMolecule() = nil, want error for cycle")
|
||||
}
|
||||
if err != nil && !strings.Contains(err.Error(), "cycle") {
|
||||
t.Errorf("error %q should mention 'cycle'", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateMolecule_DiamondNoCycle(t *testing.T) {
|
||||
// Diamond pattern: A -> B, A -> C, B -> D, C -> D
|
||||
// This has no cycle, should pass
|
||||
mol := &Issue{
|
||||
ID: "mol-xyz",
|
||||
Type: "molecule",
|
||||
Description: `## Step: a
|
||||
Root step.
|
||||
|
||||
## Step: b
|
||||
Branch 1.
|
||||
Needs: a
|
||||
|
||||
## Step: c
|
||||
Branch 2.
|
||||
Needs: a
|
||||
|
||||
## Step: d
|
||||
Merge point.
|
||||
Needs: b, c`,
|
||||
}
|
||||
|
||||
err := ValidateMolecule(mol)
|
||||
if err != nil {
|
||||
t.Errorf("ValidateMolecule() = %v, want nil (diamond has no cycle)", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateMolecule_CycleInSubgraph(t *testing.T) {
|
||||
// Root -> A, A -> B -> C -> A (cycle not involving root)
|
||||
mol := &Issue{
|
||||
ID: "mol-xyz",
|
||||
Type: "molecule",
|
||||
Description: `## Step: root
|
||||
Starting point.
|
||||
|
||||
## Step: a
|
||||
First in cycle.
|
||||
Needs: root, c
|
||||
|
||||
## Step: b
|
||||
Second in cycle.
|
||||
Needs: a
|
||||
|
||||
## Step: c
|
||||
Third in cycle.
|
||||
Needs: b`,
|
||||
}
|
||||
|
||||
err := ValidateMolecule(mol)
|
||||
if err == nil {
|
||||
t.Error("ValidateMolecule() = nil, want error for cycle in subgraph")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user