From ee66c931b9389a94107f83b5e81b9355c332e7cd Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 21 Dec 2025 21:49:07 -0800 Subject: [PATCH] feat(beads): add cycle detection for molecule dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/beads/molecule.go | 69 ++++++++++++++++++++- internal/beads/molecule_test.go | 106 ++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 1 deletion(-) diff --git a/internal/beads/molecule.go b/internal/beads/molecule.go index 79b6250b..4b4f8d92 100644 --- a/internal/beads/molecule.go +++ b/internal/beads/molecule.go @@ -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, " -> ") +} diff --git a/internal/beads/molecule_test.go b/internal/beads/molecule_test.go index 09b5fe14..004307c2 100644 --- a/internal/beads/molecule_test.go +++ b/internal/beads/molecule_test.go @@ -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") + } +}