diff --git a/cmd/bd/close.go b/cmd/bd/close.go index 7affcabe..2bc2b917 100644 --- a/cmd/bd/close.go +++ b/cmd/bd/close.go @@ -122,6 +122,7 @@ create, update, show, or close operation).`, Reason: reason, Session: session, SuggestNext: suggestNext, + Force: force, } resp, err := daemonClient.CloseIssue(closeArgs) if err != nil { @@ -191,6 +192,21 @@ create, update, show, or close operation).`, continue } + // Check if issue has open blockers (GH#962) + if !force { + blocked, blockers, err := result.Store.IsBlocked(ctx, result.ResolvedID) + if err != nil { + result.Close() + fmt.Fprintf(os.Stderr, "Error checking blockers for %s: %v\n", id, err) + continue + } + if blocked && len(blockers) > 0 { + result.Close() + fmt.Fprintf(os.Stderr, "cannot close %s: blocked by open issues %v (use --force to override)\n", id, blockers) + continue + } + } + if err := result.Store.CloseIssue(ctx, result.ResolvedID, reason, actor, session); err != nil { result.Close() fmt.Fprintf(os.Stderr, "Error closing %s: %v\n", id, err) @@ -240,6 +256,19 @@ create, update, show, or close operation).`, continue } + // Check if issue has open blockers (GH#962) + if !force { + blocked, blockers, err := store.IsBlocked(ctx, id) + if err != nil { + fmt.Fprintf(os.Stderr, "Error checking blockers for %s: %v\n", id, err) + continue + } + if blocked && len(blockers) > 0 { + fmt.Fprintf(os.Stderr, "cannot close %s: blocked by open issues %v (use --force to override)\n", id, blockers) + continue + } + } + if err := store.CloseIssue(ctx, id, reason, actor, session); err != nil { fmt.Fprintf(os.Stderr, "Error closing %s: %v\n", id, err) continue @@ -283,6 +312,21 @@ create, update, show, or close operation).`, continue } + // Check if issue has open blockers (GH#962) + if !force { + blocked, blockers, err := result.Store.IsBlocked(ctx, result.ResolvedID) + if err != nil { + result.Close() + fmt.Fprintf(os.Stderr, "Error checking blockers for %s: %v\n", id, err) + continue + } + if blocked && len(blockers) > 0 { + result.Close() + fmt.Fprintf(os.Stderr, "cannot close %s: blocked by open issues %v (use --force to override)\n", id, blockers) + continue + } + } + if err := result.Store.CloseIssue(ctx, result.ResolvedID, reason, actor, session); err != nil { result.Close() fmt.Fprintf(os.Stderr, "Error closing %s: %v\n", id, err) diff --git a/internal/rpc/protocol.go b/internal/rpc/protocol.go index 362292cb..14612132 100644 --- a/internal/rpc/protocol.go +++ b/internal/rpc/protocol.go @@ -174,6 +174,7 @@ type CloseArgs struct { Reason string `json:"reason,omitempty"` Session string `json:"session,omitempty"` // Claude Code session ID that closed this issue SuggestNext bool `json:"suggest_next,omitempty"` // Return newly unblocked issues (GH#679) + Force bool `json:"force,omitempty"` // Force close even with open blockers (GH#962) } // CloseResult is returned when SuggestNext is true (GH#679) diff --git a/internal/rpc/server_issues_epics.go b/internal/rpc/server_issues_epics.go index 795a07b6..5dd4b3f0 100644 --- a/internal/rpc/server_issues_epics.go +++ b/internal/rpc/server_issues_epics.go @@ -831,6 +831,23 @@ func (s *Server) handleClose(req *Request) Response { } } + // Check if issue has open blockers (GH#962) + if !closeArgs.Force { + blocked, blockers, err := store.IsBlocked(ctx, closeArgs.ID) + if err != nil { + return Response{ + Success: false, + Error: fmt.Sprintf("failed to check blockers: %v", err), + } + } + if blocked && len(blockers) > 0 { + return Response{ + Success: false, + Error: fmt.Sprintf("cannot close %s: blocked by open issues %v (use --force to override)", closeArgs.ID, blockers), + } + } + } + // Capture old status for rich mutation event oldStatus := "" if issue != nil { diff --git a/internal/storage/memory/memory.go b/internal/storage/memory/memory.go index 51a76027..7c0549c0 100644 --- a/internal/storage/memory/memory.go +++ b/internal/storage/memory/memory.go @@ -1281,6 +1281,19 @@ func (m *MemoryStorage) GetBlockedIssues(ctx context.Context, filter types.WorkF return results, nil } +// IsBlocked checks if an issue is blocked by open dependencies (GH#962). +// Returns true if the issue has open blockers, along with the list of blocker IDs. +func (m *MemoryStorage) IsBlocked(ctx context.Context, issueID string) (bool, []string, error) { + m.mu.RLock() + defer m.mu.RUnlock() + + blockers := m.getOpenBlockers(issueID) + if len(blockers) == 0 { + return false, nil, nil + } + return true, blockers, nil +} + // getAllDescendants returns all descendant IDs of a parent issue recursively func (m *MemoryStorage) getAllDescendants(parentID string) map[string]bool { descendants := make(map[string]bool) diff --git a/internal/storage/sqlite/ready.go b/internal/storage/sqlite/ready.go index 45358376..60b05739 100644 --- a/internal/storage/sqlite/ready.go +++ b/internal/storage/sqlite/ready.go @@ -680,6 +680,52 @@ func filterBlockedByExternalDeps(ctx context.Context, blocked []*types.BlockedIs return result } +// IsBlocked checks if an issue is blocked by open dependencies (GH#962). +// Returns true if the issue is in the blocked_issues_cache, along with a list +// of issue IDs that are blocking it. +// This is used to prevent closing issues that still have open blockers. +func (s *SQLiteStorage) IsBlocked(ctx context.Context, issueID string) (bool, []string, error) { + // First check if the issue is in the blocked cache + var inCache bool + err := s.db.QueryRowContext(ctx, ` + SELECT EXISTS(SELECT 1 FROM blocked_issues_cache WHERE issue_id = ?) + `, issueID).Scan(&inCache) + if err != nil { + return false, nil, fmt.Errorf("failed to check blocked status: %w", err) + } + + if !inCache { + return false, nil, nil + } + + // Get the blocking issue IDs + // We query dependencies for 'blocks' type where the blocker is still open + rows, err := s.db.QueryContext(ctx, ` + SELECT d.depends_on_id + FROM dependencies d + JOIN issues blocker ON d.depends_on_id = blocker.id + WHERE d.issue_id = ? + AND d.type = 'blocks' + AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred', 'hooked') + ORDER BY blocker.priority ASC + `, issueID) + if err != nil { + return true, nil, fmt.Errorf("failed to get blockers: %w", err) + } + defer func() { _ = rows.Close() }() + + var blockers []string + for rows.Next() { + var blockerID string + if err := rows.Scan(&blockerID); err != nil { + return true, nil, fmt.Errorf("failed to scan blocker ID: %w", err) + } + blockers = append(blockers, blockerID) + } + + return true, blockers, rows.Err() +} + // GetNewlyUnblockedByClose returns issues that became unblocked when the given issue was closed. // This is used by the --suggest-next flag on bd close to show what work is now available. // An issue is "newly unblocked" if: diff --git a/internal/storage/sqlite/ready_test.go b/internal/storage/sqlite/ready_test.go index a07fa982..b1735036 100644 --- a/internal/storage/sqlite/ready_test.go +++ b/internal/storage/sqlite/ready_test.go @@ -1855,3 +1855,53 @@ func TestParentIDEmptyParent(t *testing.T) { t.Fatalf("Expected 0 ready issues for empty parent, got %d", len(ready)) } } + +// TestIsBlocked tests the IsBlocked method (GH#962) +func TestIsBlocked(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + + // Create issues: + // issue1: open, no dependencies → NOT BLOCKED + // issue2: open, depends on issue1 (open) → BLOCKED by issue1 + // issue3: open, depends on issue4 (closed) → NOT BLOCKED (blocker is closed) + issue1 := env.CreateIssue("Open No Deps") + issue2 := env.CreateIssue("Blocked by open") + issue3 := env.CreateIssue("Blocked by closed") + issue4 := env.CreateIssue("Will be closed") + + env.AddDep(issue2, issue1) // issue2 depends on issue1 (open) + env.AddDep(issue3, issue4) // issue3 depends on issue4 + + env.Close(issue4, "Done") // Close issue4 + + // Test issue1: not blocked + blocked, blockers, err := env.Store.IsBlocked(ctx, issue1.ID) + if err != nil { + t.Fatalf("IsBlocked failed: %v", err) + } + if blocked { + t.Errorf("Expected issue1 to NOT be blocked, got blocked=true with blockers=%v", blockers) + } + + // Test issue2: blocked by issue1 + blocked, blockers, err = env.Store.IsBlocked(ctx, issue2.ID) + if err != nil { + t.Fatalf("IsBlocked failed: %v", err) + } + if !blocked { + t.Error("Expected issue2 to be blocked") + } + if len(blockers) != 1 || blockers[0] != issue1.ID { + t.Errorf("Expected blockers=[%s], got %v", issue1.ID, blockers) + } + + // Test issue3: not blocked (issue4 is closed) + blocked, blockers, err = env.Store.IsBlocked(ctx, issue3.ID) + if err != nil { + t.Fatalf("IsBlocked failed: %v", err) + } + if blocked { + t.Errorf("Expected issue3 to NOT be blocked (blocker is closed), got blocked=true with blockers=%v", blockers) + } +} diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 9fc94a4b..9cca5dd3 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -110,6 +110,7 @@ type Storage interface { // Ready Work & Blocking GetReadyWork(ctx context.Context, filter types.WorkFilter) ([]*types.Issue, error) GetBlockedIssues(ctx context.Context, filter types.WorkFilter) ([]*types.BlockedIssue, error) + IsBlocked(ctx context.Context, issueID string) (bool, []string, error) // GH#962: Check if issue has open blockers GetEpicsEligibleForClosure(ctx context.Context) ([]*types.EpicStatus, error) GetStaleIssues(ctx context.Context, filter types.StaleFilter) ([]*types.Issue, error) GetNewlyUnblockedByClose(ctx context.Context, closedIssueID string) ([]*types.Issue, error) // GH#679 diff --git a/internal/storage/storage_test.go b/internal/storage/storage_test.go index 6d840f0c..a68ce621 100644 --- a/internal/storage/storage_test.go +++ b/internal/storage/storage_test.go @@ -98,6 +98,9 @@ func (m *mockStorage) GetReadyWork(ctx context.Context, filter types.WorkFilter) func (m *mockStorage) GetBlockedIssues(ctx context.Context, filter types.WorkFilter) ([]*types.BlockedIssue, error) { return nil, nil } +func (m *mockStorage) IsBlocked(ctx context.Context, issueID string) (bool, []string, error) { + return false, nil, nil +} func (m *mockStorage) GetEpicsEligibleForClosure(ctx context.Context) ([]*types.EpicStatus, error) { return nil, nil }