johno/beads
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge upstream/main into subtle-ux-improvements

Browse Source 
Resolves conflicts and converts new defer/undefer commands from
fatih/color to the lipgloss semantic color system.

Key changes:
- Added StatusDeferred case in graph.go with ui.RenderAccent
- Converted status.go to use ui package for colorized output
- Converted defer.go/undefer.go to use ui package
- Merged GroupID and Aliases for status command
- Updated pre-commit hook version to 0.31.0
- Ran go mod tidy to remove fatih/color dependency
This commit is contained in:
Ryan Snodgrass
2025-12-20 17:22:43 -08:00
parent acfdcebc0f 79191bf7d1
commit 84b4562fda
45 changed files with 850 additions and 316 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
internal/beads/beads.go
View File

@@ -205,8 +205,9 @@ type (
const (
StatusOpen = types.StatusOpen
StatusInProgress = types.StatusInProgress
StatusClosed = types.StatusClosed
StatusBlocked = types.StatusBlocked
StatusDeferred = types.StatusDeferred
StatusClosed = types.StatusClosed
)
// IssueType constants

13
internal/storage/memory/memory.go
View File

@@ -1033,7 +1033,7 @@ func (m *MemoryStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte
return results, nil
}
// getOpenBlockers returns the IDs of blockers that are currently open/in_progress/blocked.
// getOpenBlockers returns the IDs of blockers that are currently open/in_progress/blocked/deferred.
// The caller must hold at least a read lock.
func (m *MemoryStorage) getOpenBlockers(issueID string) []string {
deps := m.dependencies[issueID]
@@ -1053,7 +1053,7 @@ func (m *MemoryStorage) getOpenBlockers(issueID string) []string {
continue
}
switch blocker.Status {
case types.StatusOpen, types.StatusInProgress, types.StatusBlocked:
case types.StatusOpen, types.StatusInProgress, types.StatusBlocked, types.StatusDeferred:
blockers = append(blockers, blocker.ID)
}
}
@@ -1082,7 +1082,8 @@ func (m *MemoryStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedI
}
blockers := m.getOpenBlockers(issue.ID)
if issue.Status != types.StatusBlocked && len(blockers) == 0 {
// Issue is "blocked" if: status is blocked, status is deferred, or has open blockers
if issue.Status != types.StatusBlocked && issue.Status != types.StatusDeferred && len(blockers) == 0 {
continue
}
@@ -1219,13 +1220,17 @@ func (m *MemoryStorage) GetStatistics(ctx context.Context) (*types.Statistics, e
stats.InProgressIssues++
case types.StatusClosed:
stats.ClosedIssues++
case types.StatusDeferred:
stats.DeferredIssues++
case types.StatusTombstone:
stats.TombstoneIssues++
case types.StatusPinned:
stats.PinnedIssues++
}
}
// TotalIssues excludes tombstones (matches SQLite behavior)
stats.TotalIssues = stats.OpenIssues + stats.InProgressIssues + stats.ClosedIssues
stats.TotalIssues = stats.OpenIssues + stats.InProgressIssues + stats.ClosedIssues + stats.DeferredIssues + stats.PinnedIssues
// Second pass: calculate blocked and ready issues based on dependencies
// An issue is blocked if it has open blockers (uses same logic as GetBlockedIssues)

2
internal/storage/sqlite/blocked_cache.go
View File

@@ -121,7 +121,7 @@ func (s *SQLiteStorage) rebuildBlockedCache(ctx context.Context, exec execer) er
FROM dependencies d
JOIN issues blocker ON d.depends_on_id = blocker.id
WHERE d.type = 'blocks'
AND blocker.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
),
-- Step 2: Propagate blockage to all descendants via parent-child

4
internal/storage/sqlite/compact.go
View File

@@ -78,7 +78,7 @@ func (s *SQLiteStorage) GetTier1Candidates(ctx context.Context) ([]*CompactionCa
COUNT(DISTINCT dt.dependent_id) as dependent_count
FROM issues i
LEFT JOIN dependent_tree dt ON i.id = dt.issue_id
AND dt.dependent_status IN ('open', 'in_progress', 'blocked')
AND dt.dependent_status IN ('open', 'in_progress', 'blocked', 'deferred')
AND dt.depth <= ?
WHERE i.status = 'closed'
AND i.closed_at IS NOT NULL
@@ -163,7 +163,7 @@ func (s *SQLiteStorage) GetTier2Candidates(ctx context.Context) ([]*CompactionCa
JOIN issues dep ON d.issue_id = dep.id
WHERE d.depends_on_id = i.id
AND d.type = 'blocks'
AND dep.status IN ('open', 'in_progress', 'blocked')
AND dep.status IN ('open', 'in_progress', 'blocked', 'deferred')
)
ORDER BY i.closed_at ASC
`

138
internal/storage/sqlite/cycle_detection_test.go
View File

@@ -505,3 +505,141 @@ func TestDetectCyclesMixedTypes(t *testing.T) {
t.Errorf("Expected cycle of length 3, got %d", len(cycle))
}
}
// TestDetectCyclesRelatesToNotACycle tests that bidirectional relates-to links are NOT reported as cycles
// This is the fix for GitHub issue #661: relates-to relationships should be excluded from cycle detection
// because they are inherently bidirectional ("see also" links) and don't affect work ordering.
func TestDetectCyclesRelatesToNotACycle(t *testing.T) {
store, cleanup := setupTestDB(t)
defer cleanup()
ctx := context.Background()
// Create two issues
issue1 := &types.Issue{Title: "Todo 1", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
issue2 := &types.Issue{Title: "Todo 2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
if err := store.CreateIssue(ctx, issue1, "test-user"); err != nil {
t.Fatalf("CreateIssue failed: %v", err)
}
if err := store.CreateIssue(ctx, issue2, "test-user"); err != nil {
t.Fatalf("CreateIssue failed: %v", err)
}
// Create bidirectional relates_to links (simulating what bd relate does)
// issue1 relates_to issue2
_, err := store.db.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_by, created_at)
VALUES (?, ?, ?, 'test-user', CURRENT_TIMESTAMP)
`, issue1.ID, issue2.ID, types.DepRelatesTo)
if err != nil {
t.Fatalf("Insert relates_to dependency failed: %v", err)
}
// issue2 relates_to issue1 (the reverse link)
_, err = store.db.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_by, created_at)
VALUES (?, ?, ?, 'test-user', CURRENT_TIMESTAMP)
`, issue2.ID, issue1.ID, types.DepRelatesTo)
if err != nil {
t.Fatalf("Insert relates_to dependency failed: %v", err)
}
// Detect cycles - should find NONE because relates_to is excluded
cycles, err := store.DetectCycles(ctx)
if err != nil {
t.Fatalf("DetectCycles failed: %v", err)
}
if len(cycles) != 0 {
t.Errorf("Expected no cycles for relates_to bidirectional links, but found %d cycles", len(cycles))
for i, cycle := range cycles {
t.Logf("Cycle %d:", i+1)
for _, issue := range cycle {
t.Logf(" - %s: %s", issue.ID, issue.Title)
}
}
}
}
// TestDetectCyclesRelatesToWithOtherCycle tests that relates-to is excluded but other cycles are still detected
func TestDetectCyclesRelatesToWithOtherCycle(t *testing.T) {
store, cleanup := setupTestDB(t)
defer cleanup()
ctx := context.Background()
// Create three issues
issue1 := &types.Issue{Title: "Issue A", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
issue2 := &types.Issue{Title: "Issue B", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
issue3 := &types.Issue{Title: "Issue C", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
if err := store.CreateIssue(ctx, issue1, "test-user"); err != nil {
t.Fatalf("CreateIssue failed: %v", err)
}
if err := store.CreateIssue(ctx, issue2, "test-user"); err != nil {
t.Fatalf("CreateIssue failed: %v", err)
}
if err := store.CreateIssue(ctx, issue3, "test-user"); err != nil {
t.Fatalf("CreateIssue failed: %v", err)
}
// Create bidirectional relates_to between issue1 and issue2 (should NOT trigger cycle)
_, err := store.db.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_by, created_at)
VALUES (?, ?, ?, 'test-user', CURRENT_TIMESTAMP)
`, issue1.ID, issue2.ID, types.DepRelatesTo)
if err != nil {
t.Fatalf("Insert relates_to dependency failed: %v", err)
}
_, err = store.db.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_by, created_at)
VALUES (?, ?, ?, 'test-user', CURRENT_TIMESTAMP)
`, issue2.ID, issue1.ID, types.DepRelatesTo)
if err != nil {
t.Fatalf("Insert relates_to dependency failed: %v", err)
}
// Create a real cycle with blocks: issue2 -> issue3 -> issue2
_, err = store.db.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_by, created_at)
VALUES (?, ?, ?, 'test-user', CURRENT_TIMESTAMP)
`, issue2.ID, issue3.ID, types.DepBlocks)
if err != nil {
t.Fatalf("Insert blocks dependency failed: %v", err)
}
_, err = store.db.ExecContext(ctx, `
INSERT INTO dependencies (issue_id, depends_on_id, type, created_by, created_at)
VALUES (?, ?, ?, 'test-user', CURRENT_TIMESTAMP)
`, issue3.ID, issue2.ID, types.DepBlocks)
if err != nil {
t.Fatalf("Insert blocks dependency failed: %v", err)
}
// Detect cycles - should find the blocks cycle but NOT the relates_to "cycle"
cycles, err := store.DetectCycles(ctx)
if err != nil {
t.Fatalf("DetectCycles failed: %v", err)
}
if len(cycles) == 0 {
t.Fatal("Expected to find the blocks cycle, but found none")
}
// Verify the cycle contains issue2 and issue3, but NOT issue1
foundIDs := make(map[string]bool)
for _, cycle := range cycles {
for _, issue := range cycle {
foundIDs[issue.ID] = true
}
}
if !foundIDs[issue2.ID] || !foundIDs[issue3.ID] {
t.Errorf("Expected cycle to contain issue2 and issue3. Found: %v", foundIDs)
}
// Verify issue1 is NOT in the cycle (it's only connected via relates-to)
if foundIDs[issue1.ID] {
t.Errorf("issue1 should NOT be in cycle (only connected via relates-to), but was found")
}
}

7
internal/storage/sqlite/dependencies.go
View File

@@ -613,9 +613,12 @@ func (s *SQLiteStorage) GetDependencyTree(ctx context.Context, issueID string, m
}
// DetectCycles finds circular dependencies and returns the actual cycle paths
// Note: relates-to dependencies are excluded because they are intentionally bidirectional
// ("see also" relationships) and do not represent problematic cycles.
func (s *SQLiteStorage) DetectCycles(ctx context.Context) ([][]*types.Issue, error) {
// Use recursive CTE to find cycles with full paths
// We track the path as a string to work around SQLite's lack of arrays
// Exclude relates-to dependencies since they are inherently bidirectional
rows, err := s.db.QueryContext(ctx, `
WITH RECURSIVE paths AS (
SELECT
@@ -625,6 +628,7 @@ func (s *SQLiteStorage) DetectCycles(ctx context.Context) ([][]*types.Issue, err
issue_id || '→' || depends_on_id as path,
0 as depth
FROM dependencies
WHERE type != 'relates-to'
UNION ALL
@@ -636,7 +640,8 @@ func (s *SQLiteStorage) DetectCycles(ctx context.Context) ([][]*types.Issue, err
p.depth + 1
FROM dependencies d
JOIN paths p ON d.issue_id = p.depends_on_id
WHERE p.depth < ?
WHERE d.type != 'relates-to'
AND p.depth < ?
AND (d.depends_on_id = p.start_id OR p.path NOT LIKE '%' || d.depends_on_id || '→%')
)
SELECT DISTINCT path as cycle_path

18
internal/storage/sqlite/dependencies_test.go
View File

@@ -1527,9 +1527,10 @@ func TestDetectCycles_MultipleGraphs(t *testing.T) {
}
}
// TestDetectCycles_RelatedTypeAllowsAdditionButReportsDetection tests relates-to allows bidirectional links
// even though they're technically cycles. The cycle prevention only skips relates-to during AddDependency.
func TestDetectCycles_RelatedTypeAllowsAdditionButReportsDetection(t *testing.T) {
// TestDetectCycles_RelatesTypeAllowsBidirectionalWithoutCycleReport tests relates-to allows bidirectional links
// and DetectCycles correctly excludes them (they're "see also" links, not problematic cycles).
// This was fixed in GH#661 - relates-to is explicitly excluded from cycle detection.
func TestDetectCycles_RelatesTypeAllowsBidirectionalWithoutCycleReport(t *testing.T) {
store, cleanup := setupTestDB(t)
defer cleanup()
@@ -1552,7 +1553,7 @@ func TestDetectCycles_RelatedTypeAllowsAdditionButReportsDetection(t *testing.T)
t.Fatalf("AddDependency for relates-to failed: %v", err)
}
// Add B relates-to A (this should succeed despite creating a cycle because relates-to skips cycle detection)
// Add B relates-to A (this should succeed - relates-to skips cycle prevention)
err = store.AddDependency(ctx, &types.Dependency{
IssueID: issueB.ID,
DependsOnID: issueA.ID,
@@ -1562,15 +1563,16 @@ func TestDetectCycles_RelatedTypeAllowsAdditionButReportsDetection(t *testing.T)
t.Fatalf("AddDependency for reverse relates-to failed: %v", err)
}
// DetectCycles will report the cycle even though AddDependency allowed it
// DetectCycles should NOT report relates-to as cycles (GH#661 fix)
// relates-to is inherently bidirectional ("see also") and doesn't affect work ordering
cycles, err := store.DetectCycles(ctx)
if err != nil {
t.Fatalf("DetectCycles failed: %v", err)
}
// Relates-to bidirectional creates cycles (may report multiple entry points for same cycle)
if len(cycles) == 0 {
t.Error("Expected at least 1 cycle detected for bidirectional relates-to")
// relates-to bidirectional should NOT be reported as a cycle
if len(cycles) != 0 {
t.Errorf("Expected 0 cycles for bidirectional relates-to (GH#661 fix), got %d", len(cycles))
}
// Verify both directions exist

12
internal/storage/sqlite/events.go
View File

@@ -112,16 +112,18 @@ func (s *SQLiteStorage) GetStatistics(ctx context.Context) (*types.Statistics, e
// Get counts (bd-nyt: exclude tombstones from TotalIssues, report separately)
// (bd-6v2: also count pinned issues)
// (bd-4jr: also count deferred issues)
err := s.db.QueryRowContext(ctx, `
SELECT
COALESCE(SUM(CASE WHEN status != 'tombstone' THEN 1 ELSE 0 END), 0) as total,
COALESCE(SUM(CASE WHEN status = 'open' THEN 1 ELSE 0 END), 0) as open,
COALESCE(SUM(CASE WHEN status = 'in_progress' THEN 1 ELSE 0 END), 0) as in_progress,
COALESCE(SUM(CASE WHEN status = 'closed' THEN 1 ELSE 0 END), 0) as closed,
COALESCE(SUM(CASE WHEN status = 'deferred' THEN 1 ELSE 0 END), 0) as deferred,
COALESCE(SUM(CASE WHEN status = 'tombstone' THEN 1 ELSE 0 END), 0) as tombstone,
COALESCE(SUM(CASE WHEN status = 'pinned' THEN 1 ELSE 0 END), 0) as pinned
FROM issues
`).Scan(&stats.TotalIssues, &stats.OpenIssues, &stats.InProgressIssues, &stats.ClosedIssues, &stats.TombstoneIssues, &stats.PinnedIssues)
`).Scan(&stats.TotalIssues, &stats.OpenIssues, &stats.InProgressIssues, &stats.ClosedIssues, &stats.DeferredIssues, &stats.TombstoneIssues, &stats.PinnedIssues)
if err != nil {
return nil, fmt.Errorf("failed to get issue counts: %w", err)
}
@@ -132,9 +134,9 @@ func (s *SQLiteStorage) GetStatistics(ctx context.Context) (*types.Statistics, e
FROM issues i
JOIN dependencies d ON i.id = d.issue_id
JOIN issues blocker ON d.depends_on_id = blocker.id
WHERE i.status IN ('open', 'in_progress', 'blocked')
WHERE i.status IN ('open', 'in_progress', 'blocked', 'deferred')
AND d.type = 'blocks'
AND blocker.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
`).Scan(&stats.BlockedIssues)
if err != nil {
return nil, fmt.Errorf("failed to get blocked count: %w", err)
@@ -147,10 +149,10 @@ func (s *SQLiteStorage) GetStatistics(ctx context.Context) (*types.Statistics, e
WHERE i.status = 'open'
AND NOT EXISTS (
SELECT 1 FROM dependencies d
JOIN issues blocked ON d.depends_on_id = blocked.id
JOIN issues blocker ON d.depends_on_id = blocker.id
WHERE d.issue_id = i.id
AND d.type = 'blocks'
AND blocked.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
)
`).Scan(&stats.ReadyIssues)
if err != nil {

7
internal/storage/sqlite/ready.go
View File

@@ -293,18 +293,19 @@ func (s *SQLiteStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedI
AND EXISTS (
SELECT 1 FROM issues blocker
WHERE blocker.id = d.depends_on_id
AND blocker.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
)
WHERE i.status IN ('open', 'in_progress', 'blocked')
WHERE i.status IN ('open', 'in_progress', 'blocked', 'deferred')
AND i.pinned = 0
AND (
i.status = 'blocked'
OR i.status = 'deferred'
OR EXISTS (
SELECT 1 FROM dependencies d2
JOIN issues blocker ON d2.depends_on_id = blocker.id
WHERE d2.issue_id = i.id
AND d2.type = 'blocks'
AND blocker.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
)
)
GROUP BY i.id

6
internal/storage/sqlite/schema.go
View File

@@ -206,7 +206,7 @@ WITH RECURSIVE
FROM dependencies d
JOIN issues blocker ON d.depends_on_id = blocker.id
WHERE d.type = 'blocks'
AND blocker.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
),
-- Propagate blockage to all descendants via parent-child
blocked_transitively AS (
@@ -236,8 +236,8 @@ SELECT
FROM issues i
JOIN dependencies d ON i.id = d.issue_id
JOIN issues blocker ON d.depends_on_id = blocker.id
WHERE i.status IN ('open', 'in_progress', 'blocked')
WHERE i.status IN ('open', 'in_progress', 'blocked', 'deferred')
AND d.type = 'blocks'
AND blocker.status IN ('open', 'in_progress', 'blocked')
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
GROUP BY i.id;
`

4
internal/types/types.go
View File

@@ -219,6 +219,7 @@ const (
StatusOpen Status = "open"
StatusInProgress Status = "in_progress"
StatusBlocked Status = "blocked"
StatusDeferred Status = "deferred" // Deliberately put on ice for later (bd-4jr)
StatusClosed Status = "closed"
StatusTombstone Status = "tombstone" // Soft-deleted issue (bd-vw8)
StatusPinned Status = "pinned" // Persistent bead that stays open indefinitely (bd-6v2)
@@ -227,7 +228,7 @@ const (
// IsValid checks if the status value is valid (built-in statuses only)
func (s Status) IsValid() bool {
switch s {
case StatusOpen, StatusInProgress, StatusBlocked, StatusClosed, StatusTombstone, StatusPinned:
case StatusOpen, StatusInProgress, StatusBlocked, StatusDeferred, StatusClosed, StatusTombstone, StatusPinned:
return true
}
return false
@@ -425,6 +426,7 @@ type Statistics struct {
InProgressIssues int `json:"in_progress_issues"`
ClosedIssues int `json:"closed_issues"`
BlockedIssues int `json:"blocked_issues"`
DeferredIssues int `json:"deferred_issues"` // Issues on ice (bd-4jr)
ReadyIssues int `json:"ready_issues"`
TombstoneIssues int `json:"tombstone_issues"` // Soft-deleted issues (bd-nyt)
PinnedIssues int `json:"pinned_issues"` // Persistent issues (bd-6v2)