feat(deps): detect/prevent child→parent dependency anti-pattern (bd-nim5)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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{
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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{
|
||||
|
||||
Reference in New Issue
Block a user