From ce2b05356a0a05943a790f9b5dceaf72764b5a5f Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Wed, 24 Dec 2025 13:03:13 -0800 Subject: [PATCH] =?UTF-8?q?feat(deps):=20detect/prevent=20child=E2=86=92pa?= =?UTF-8?q?rent=20dependency=20anti-pattern=20(bd-nim5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prevents a common mistake where users add dependencies from child issues to their parent epics. This creates a deadlock: - Child can't start (blocked by open parent) - Parent can't close (children not done) Changes: - dep.go: Reject child→parent deps at creation time with clear error - server_labels_deps_comments.go: Same check for daemon RPC - doctor/validation.go: New check detects existing bad deps - doctor/fix/validation.go: Auto-fix removes bad deps - doctor.go: Wire up check and fix handler 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/dep.go | 26 +++++ cmd/bd/dep_test.go | 83 ++++++++++++++++ cmd/bd/doctor.go | 7 ++ cmd/bd/doctor/fix/validation.go | 64 ++++++++++++ cmd/bd/doctor/fix/validation_test.go | 103 ++++++++++++++++++++ cmd/bd/doctor/validation.go | 73 ++++++++++++++ internal/rpc/server_labels_deps_comments.go | 23 +++++ 7 files changed, 379 insertions(+) diff --git a/cmd/bd/dep.go b/cmd/bd/dep.go index 90ddd8ad..50cef5f2 100644 --- a/cmd/bd/dep.go +++ b/cmd/bd/dep.go @@ -15,6 +15,23 @@ import ( "github.com/steveyegge/beads/internal/utils" ) +// isChildOf returns true if childID is a hierarchical child of parentID. +// For example, "bd-abc.1" is a child of "bd-abc", and "bd-abc.1.2" is a child of "bd-abc.1". +func isChildOf(childID, parentID string) bool { + // A child ID has the format "parentID.N" or "parentID.N.M" etc. + // Use ParseHierarchicalID to get the actual parent + _, actualParentID, depth := types.ParseHierarchicalID(childID) + if depth == 0 { + return false // Not a hierarchical ID + } + // Check if the immediate parent matches + if actualParentID == parentID { + return true + } + // Also check if parentID is an ancestor (e.g., "bd-abc" is parent of "bd-abc.1.2") + return strings.HasPrefix(childID, parentID+".") +} + var depCmd = &cobra.Command{ Use: "dep", GroupID: "deps", @@ -107,6 +124,15 @@ Examples: } } + // Check for child→parent dependency anti-pattern (bd-nim5) + // This creates a deadlock: child can't start (parent open), parent can't close (children not done) + if isChildOf(fromID, toID) { + fmt.Fprintf(os.Stderr, "Error: Cannot add dependency: %s is already a child of %s.\n", fromID, toID) + fmt.Fprintf(os.Stderr, "Children inherit dependency on parent completion via hierarchy.\n") + fmt.Fprintf(os.Stderr, "Adding an explicit dependency would create a deadlock.\n") + os.Exit(1) + } + // If daemon is running, use RPC if daemonClient != nil { depArgs := &rpc.DepAddArgs{ diff --git a/cmd/bd/dep_test.go b/cmd/bd/dep_test.go index 003735df..e944f62a 100644 --- a/cmd/bd/dep_test.go +++ b/cmd/bd/dep_test.go @@ -956,3 +956,86 @@ func TestMergeBidirectionalTrees_PreservesDepth(t *testing.T) { } } } + +// Tests for child→parent dependency detection (bd-nim5) +func TestIsChildOf(t *testing.T) { + tests := []struct { + name string + childID string + parentID string + want bool + }{ + // Positive cases: should be detected as child + { + name: "direct child", + childID: "bd-abc.1", + parentID: "bd-abc", + want: true, + }, + { + name: "grandchild", + childID: "bd-abc.1.2", + parentID: "bd-abc", + want: true, + }, + { + name: "nested grandchild direct parent", + childID: "bd-abc.1.2", + parentID: "bd-abc.1", + want: true, + }, + { + name: "deeply nested child", + childID: "bd-abc.1.2.3", + parentID: "bd-abc", + want: true, + }, + + // Negative cases: should NOT be detected as child + { + name: "same ID", + childID: "bd-abc", + parentID: "bd-abc", + want: false, + }, + { + name: "not a child - unrelated IDs", + childID: "bd-xyz", + parentID: "bd-abc", + want: false, + }, + { + name: "not a child - sibling", + childID: "bd-abc.2", + parentID: "bd-abc.1", + want: false, + }, + { + name: "reversed - parent is not child of child", + childID: "bd-abc", + parentID: "bd-abc.1", + want: false, + }, + { + name: "prefix but not hierarchical", + childID: "bd-abcd", + parentID: "bd-abc", + want: false, + }, + { + name: "not hierarchical ID", + childID: "bd-abc", + parentID: "bd-xyz", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isChildOf(tt.childID, tt.parentID) + if got != tt.want { + t.Errorf("isChildOf(%q, %q) = %v, want %v", tt.childID, tt.parentID, got, tt.want) + } + }) + } +} diff --git a/cmd/bd/doctor.go b/cmd/bd/doctor.go index 5e020566..aff7160c 100644 --- a/cmd/bd/doctor.go +++ b/cmd/bd/doctor.go @@ -400,6 +400,8 @@ func applyFixList(path string, fixes []doctorCheck) { err = fix.MergeArtifacts(path) case "Orphaned Dependencies": err = fix.OrphanedDependencies(path) + case "Child-Parent Dependencies": + err = fix.ChildParentDependencies(path) case "Duplicate Issues": // No auto-fix: duplicates require user review fmt.Printf(" ⚠ Run 'bd duplicates' to review and merge duplicates\n") @@ -785,6 +787,11 @@ func runDiagnostics(path string) doctorResult { result.Checks = append(result.Checks, orphanedDepsCheck) // Don't fail overall check for orphaned deps, just warn + // Check 22a: Child→parent dependencies (anti-pattern, bd-nim5) + childParentDepsCheck := convertDoctorCheck(doctor.CheckChildParentDependencies(path)) + result.Checks = append(result.Checks, childParentDepsCheck) + // Don't fail overall check for child→parent deps, just warn + // Check 23: Duplicate issues (from bd validate) duplicatesCheck := convertDoctorCheck(doctor.CheckDuplicateIssues(path)) result.Checks = append(result.Checks, duplicatesCheck) diff --git a/cmd/bd/doctor/fix/validation.go b/cmd/bd/doctor/fix/validation.go index 6d00df97..e48aec03 100644 --- a/cmd/bd/doctor/fix/validation.go +++ b/cmd/bd/doctor/fix/validation.go @@ -162,6 +162,70 @@ func OrphanedDependencies(path string) error { return nil } +// ChildParentDependencies removes child→parent dependencies (anti-pattern). +// This fixes the deadlock where children depend on their parent epic. +func ChildParentDependencies(path string) error { + if err := validateBeadsWorkspace(path); err != nil { + return err + } + + beadsDir := filepath.Join(path, ".beads") + dbPath := filepath.Join(beadsDir, "beads.db") + + // Open database + db, err := openDB(dbPath) + if err != nil { + return fmt.Errorf("failed to open database: %w", err) + } + defer db.Close() + + // Find child→parent dependencies where issue_id starts with depends_on_id + "." + query := ` + SELECT d.issue_id, d.depends_on_id + FROM dependencies d + WHERE d.issue_id LIKE d.depends_on_id || '.%' + ` + rows, err := db.Query(query) + if err != nil { + return fmt.Errorf("failed to query child-parent dependencies: %w", err) + } + defer rows.Close() + + type badDep struct { + issueID string + dependsOnID string + } + var badDeps []badDep + + for rows.Next() { + var d badDep + if err := rows.Scan(&d.issueID, &d.dependsOnID); err == nil { + badDeps = append(badDeps, d) + } + } + + if len(badDeps) == 0 { + fmt.Println(" No child→parent dependencies to fix") + return nil + } + + // Delete child→parent dependencies + for _, d := range badDeps { + _, err := db.Exec("DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ?", + d.issueID, d.dependsOnID) + if err != nil { + fmt.Printf(" Warning: failed to remove %s→%s: %v\n", d.issueID, d.dependsOnID, err) + } else { + // Mark issue as dirty for export + _, _ = db.Exec("INSERT OR IGNORE INTO dirty_issues (issue_id) VALUES (?)", d.issueID) + fmt.Printf(" Removed child→parent dependency: %s→%s\n", d.issueID, d.dependsOnID) + } + } + + fmt.Printf(" Fixed %d child→parent dependency anti-pattern(s)\n", len(badDeps)) + return nil +} + // openDB opens a SQLite database for read-write access func openDB(dbPath string) (*sql.DB, error) { return sql.Open("sqlite3", dbPath) diff --git a/cmd/bd/doctor/fix/validation_test.go b/cmd/bd/doctor/fix/validation_test.go index d155a778..83196df1 100644 --- a/cmd/bd/doctor/fix/validation_test.go +++ b/cmd/bd/doctor/fix/validation_test.go @@ -1,7 +1,12 @@ package fix import ( + "os" + "path/filepath" "testing" + + _ "github.com/ncruces/go-sqlite3/driver" + _ "github.com/ncruces/go-sqlite3/embed" ) // TestFixFunctions_RequireBeadsDir verifies all fix functions properly validate @@ -22,6 +27,7 @@ func TestFixFunctions_RequireBeadsDir(t *testing.T) { {"SyncBranchHealth", func(dir string) error { return SyncBranchHealth(dir, "beads-sync") }}, {"UntrackedJSONL", UntrackedJSONL}, {"MigrateTombstones", MigrateTombstones}, + {"ChildParentDependencies", ChildParentDependencies}, } for _, tc := range funcs { @@ -35,3 +41,100 @@ func TestFixFunctions_RequireBeadsDir(t *testing.T) { }) } } + +func TestChildParentDependencies_NoBadDeps(t *testing.T) { + // Set up test database with no child→parent deps + dir := t.TempDir() + beadsDir := filepath.Join(dir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "beads.db") + db, err := openDB(dbPath) + if err != nil { + t.Fatal(err) + } + + // Create minimal schema + _, err = db.Exec(` + CREATE TABLE issues (id TEXT PRIMARY KEY); + CREATE TABLE dependencies (issue_id TEXT, depends_on_id TEXT, type TEXT); + CREATE TABLE dirty_issues (issue_id TEXT PRIMARY KEY); + INSERT INTO issues (id) VALUES ('bd-abc'), ('bd-abc.1'), ('bd-xyz'); + INSERT INTO dependencies (issue_id, depends_on_id, type) VALUES ('bd-abc.1', 'bd-xyz', 'blocks'); + `) + if err != nil { + t.Fatal(err) + } + db.Close() + + // Run fix - should find no bad deps + err = ChildParentDependencies(dir) + if err != nil { + t.Errorf("ChildParentDependencies failed: %v", err) + } + + // Verify the good dependency still exists + db, _ = openDB(dbPath) + defer db.Close() + var count int + db.QueryRow("SELECT COUNT(*) FROM dependencies").Scan(&count) + if count != 1 { + t.Errorf("Expected 1 dependency, got %d", count) + } +} + +func TestChildParentDependencies_FixesBadDeps(t *testing.T) { + // Set up test database with child→parent deps + dir := t.TempDir() + beadsDir := filepath.Join(dir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(beadsDir, "beads.db") + db, err := openDB(dbPath) + if err != nil { + t.Fatal(err) + } + + // Create minimal schema with child→parent dependency + _, err = db.Exec(` + CREATE TABLE issues (id TEXT PRIMARY KEY); + CREATE TABLE dependencies (issue_id TEXT, depends_on_id TEXT, type TEXT); + CREATE TABLE dirty_issues (issue_id TEXT PRIMARY KEY); + INSERT INTO issues (id) VALUES ('bd-abc'), ('bd-abc.1'), ('bd-abc.1.2'); + INSERT INTO dependencies (issue_id, depends_on_id, type) VALUES + ('bd-abc.1', 'bd-abc', 'blocks'), + ('bd-abc.1.2', 'bd-abc', 'blocks'), + ('bd-abc.1.2', 'bd-abc.1', 'blocks'); + `) + if err != nil { + t.Fatal(err) + } + db.Close() + + // Run fix + err = ChildParentDependencies(dir) + if err != nil { + t.Errorf("ChildParentDependencies failed: %v", err) + } + + // Verify all bad dependencies were removed + db, _ = openDB(dbPath) + defer db.Close() + var count int + db.QueryRow("SELECT COUNT(*) FROM dependencies").Scan(&count) + if count != 0 { + t.Errorf("Expected 0 dependencies after fix, got %d", count) + } + + // Verify dirty_issues was updated for affected issues + // Note: 2 unique issue_ids (bd-abc.1 appears once, bd-abc.1.2 appears twice but INSERT OR IGNORE dedupes) + var dirtyCount int + db.QueryRow("SELECT COUNT(*) FROM dirty_issues").Scan(&dirtyCount) + if dirtyCount != 2 { + t.Errorf("Expected 2 dirty issues (unique issue_ids from removed deps), got %d", dirtyCount) + } +} diff --git a/cmd/bd/doctor/validation.go b/cmd/bd/doctor/validation.go index 861d6094..8509bcf6 100644 --- a/cmd/bd/doctor/validation.go +++ b/cmd/bd/doctor/validation.go @@ -308,6 +308,79 @@ func CheckTestPollution(path string) DoctorCheck { } } +// CheckChildParentDependencies detects the child→parent dependency anti-pattern. +// This creates a deadlock: child can't start (parent open), parent can't close (children not done). +func CheckChildParentDependencies(path string) DoctorCheck { + beadsDir := filepath.Join(path, ".beads") + dbPath := filepath.Join(beadsDir, beads.CanonicalDatabaseName) + + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + return DoctorCheck{ + Name: "Child-Parent Dependencies", + Status: "ok", + Message: "N/A (no database)", + } + } + + db, err := openDBReadOnly(dbPath) + if err != nil { + return DoctorCheck{ + Name: "Child-Parent Dependencies", + Status: "ok", + Message: "N/A (unable to open database)", + } + } + defer db.Close() + + // Query for child→parent dependencies where issue_id starts with depends_on_id + "." + // This uses SQLite's LIKE pattern matching + query := ` + SELECT d.issue_id, d.depends_on_id + FROM dependencies d + WHERE d.issue_id LIKE d.depends_on_id || '.%' + ` + rows, err := db.Query(query) + if err != nil { + return DoctorCheck{ + Name: "Child-Parent Dependencies", + Status: "ok", + Message: "N/A (query failed)", + } + } + defer rows.Close() + + var badDeps []string + for rows.Next() { + var issueID, dependsOnID string + if err := rows.Scan(&issueID, &dependsOnID); err == nil { + badDeps = append(badDeps, fmt.Sprintf("%s→%s", issueID, dependsOnID)) + } + } + + if len(badDeps) == 0 { + return DoctorCheck{ + Name: "Child-Parent Dependencies", + Status: "ok", + Message: "No child→parent dependencies", + Category: CategoryMetadata, + } + } + + detail := strings.Join(badDeps, ", ") + if len(detail) > 200 { + detail = detail[:200] + "..." + } + + return DoctorCheck{ + Name: "Child-Parent Dependencies", + Status: "warning", + Message: fmt.Sprintf("%d child→parent dependency anti-pattern(s) detected", len(badDeps)), + Detail: detail, + Fix: "Run 'bd doctor --fix' to remove child→parent dependencies", + Category: CategoryMetadata, + } +} + // CheckGitConflicts detects git conflict markers in JSONL file. func CheckGitConflicts(path string) DoctorCheck { beadsDir := filepath.Join(path, ".beads") diff --git a/internal/rpc/server_labels_deps_comments.go b/internal/rpc/server_labels_deps_comments.go index f0510131..b84782e6 100644 --- a/internal/rpc/server_labels_deps_comments.go +++ b/internal/rpc/server_labels_deps_comments.go @@ -4,11 +4,25 @@ import ( "context" "encoding/json" "fmt" + "strings" "github.com/steveyegge/beads/internal/storage" "github.com/steveyegge/beads/internal/types" ) +// isChildOf returns true if childID is a hierarchical child of parentID. +// For example, "bd-abc.1" is a child of "bd-abc", and "bd-abc.1.2" is a child of "bd-abc.1". +func isChildOf(childID, parentID string) bool { + _, actualParentID, depth := types.ParseHierarchicalID(childID) + if depth == 0 { + return false // Not a hierarchical ID + } + if actualParentID == parentID { + return true + } + return strings.HasPrefix(childID, parentID+".") +} + func (s *Server) handleDepAdd(req *Request) Response { var depArgs DepAddArgs if err := json.Unmarshal(req.Args, &depArgs); err != nil { @@ -18,6 +32,15 @@ func (s *Server) handleDepAdd(req *Request) Response { } } + // Check for child→parent dependency anti-pattern (bd-nim5) + // This creates a deadlock: child can't start (parent open), parent can't close (children not done) + if isChildOf(depArgs.FromID, depArgs.ToID) { + return Response{ + Success: false, + Error: fmt.Sprintf("cannot add dependency: %s is already a child of %s (children inherit dependency via hierarchy)", depArgs.FromID, depArgs.ToID), + } + } + store := s.storage if store == nil { return Response{