feat: bd ready filters by external dep satisfaction (bd-zmmy)
GetReadyWork now lazily resolves external dependencies at query time: - External refs (external:project:capability) checked against target DB - Issues with unsatisfied external deps are filtered from ready list - Satisfaction = closed issue with provides:<capability> label in target Key changes: - Remove FK constraint on depends_on_id to allow external refs - Add migration 025 to drop FK and recreate views - Filter external deps in GetReadyWork, not in blocked_issues_cache - Add application-level validation for orphaned local deps - Comprehensive tests for external dep resolution 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -113,27 +113,19 @@ func (s *SQLiteStorage) rebuildBlockedCache(ctx context.Context, exec execer) er
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Rebuild using the recursive CTE logic
|
// Rebuild using the recursive CTE logic
|
||||||
// Includes both local blockers (open issues) and external refs (bd-om4a)
|
// Only includes local blockers (open issues) - external refs are resolved
|
||||||
|
// lazily at query time by GetReadyWork (bd-zmmy supersedes bd-om4a)
|
||||||
query := `
|
query := `
|
||||||
INSERT INTO blocked_issues_cache (issue_id)
|
INSERT INTO blocked_issues_cache (issue_id)
|
||||||
WITH RECURSIVE
|
WITH RECURSIVE
|
||||||
-- Step 1: Find issues blocked directly by dependencies
|
-- Step 1: Find issues blocked directly by LOCAL dependencies
|
||||||
-- Includes both local blockers (open issues) and external references
|
-- External refs (external:*) are excluded - they're resolved lazily by GetReadyWork
|
||||||
blocked_directly AS (
|
blocked_directly AS (
|
||||||
-- Local blockers: issues with open status
|
|
||||||
SELECT DISTINCT d.issue_id
|
SELECT DISTINCT d.issue_id
|
||||||
FROM dependencies d
|
FROM dependencies d
|
||||||
JOIN issues blocker ON d.depends_on_id = blocker.id
|
JOIN issues blocker ON d.depends_on_id = blocker.id
|
||||||
WHERE d.type = 'blocks'
|
WHERE d.type = 'blocks'
|
||||||
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
|
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
|
||||||
|
|
||||||
UNION
|
|
||||||
|
|
||||||
-- External blockers: always blocking until resolved (bd-om4a)
|
|
||||||
SELECT DISTINCT d.issue_id
|
|
||||||
FROM dependencies d
|
|
||||||
WHERE d.type = 'blocks'
|
|
||||||
AND d.depends_on_id LIKE 'external:%'
|
|
||||||
),
|
),
|
||||||
|
|
||||||
-- Step 2: Propagate blockage to all descendants via parent-child
|
-- Step 2: Propagate blockage to all descendants via parent-child
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency
|
|||||||
return fmt.Errorf("invalid dependency type: %q (must be non-empty string, max 50 chars)", dep.Type)
|
return fmt.Errorf("invalid dependency type: %q (must be non-empty string, max 50 chars)", dep.Type)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate that both issues exist
|
// Validate that source issue exists
|
||||||
issueExists, err := s.GetIssue(ctx, dep.IssueID)
|
issueExists, err := s.GetIssue(ctx, dep.IssueID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to check issue %s: %w", dep.IssueID, err)
|
return fmt.Errorf("failed to check issue %s: %w", dep.IssueID, err)
|
||||||
@@ -33,31 +33,38 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency
|
|||||||
return fmt.Errorf("issue %s not found", dep.IssueID)
|
return fmt.Errorf("issue %s not found", dep.IssueID)
|
||||||
}
|
}
|
||||||
|
|
||||||
dependsOnExists, err := s.GetIssue(ctx, dep.DependsOnID)
|
// External refs (external:<project>:<capability>) don't need target validation (bd-zmmy)
|
||||||
if err != nil {
|
// They are resolved lazily at query time by CheckExternalDep
|
||||||
return fmt.Errorf("failed to check dependency %s: %w", dep.DependsOnID, err)
|
isExternalRef := strings.HasPrefix(dep.DependsOnID, "external:")
|
||||||
}
|
|
||||||
if dependsOnExists == nil {
|
|
||||||
return fmt.Errorf("dependency target %s not found", dep.DependsOnID)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Prevent self-dependency
|
var dependsOnExists *types.Issue
|
||||||
if dep.IssueID == dep.DependsOnID {
|
if !isExternalRef {
|
||||||
return fmt.Errorf("issue cannot depend on itself")
|
dependsOnExists, err = s.GetIssue(ctx, dep.DependsOnID)
|
||||||
}
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check dependency %s: %w", dep.DependsOnID, err)
|
||||||
|
}
|
||||||
|
if dependsOnExists == nil {
|
||||||
|
return fmt.Errorf("dependency target %s not found", dep.DependsOnID)
|
||||||
|
}
|
||||||
|
|
||||||
// Validate parent-child dependency direction
|
// Prevent self-dependency (only for local deps)
|
||||||
// In parent-child relationships: child depends on parent (child is part of parent)
|
if dep.IssueID == dep.DependsOnID {
|
||||||
// Parent should NOT depend on child (semantically backwards)
|
return fmt.Errorf("issue cannot depend on itself")
|
||||||
// Consistent with dependency semantics: IssueID depends on DependsOnID
|
}
|
||||||
if dep.Type == types.DepParentChild {
|
|
||||||
// issueExists is the dependent (the one that depends on something)
|
// Validate parent-child dependency direction (only for local deps)
|
||||||
// dependsOnExists is what it depends on
|
// In parent-child relationships: child depends on parent (child is part of parent)
|
||||||
// Correct: Task (child) depends on Epic (parent) - child belongs to parent
|
// Parent should NOT depend on child (semantically backwards)
|
||||||
// Incorrect: Epic (parent) depends on Task (child) - backwards
|
// Consistent with dependency semantics: IssueID depends on DependsOnID
|
||||||
if issueExists.IssueType == types.TypeEpic && dependsOnExists.IssueType != types.TypeEpic {
|
if dep.Type == types.DepParentChild {
|
||||||
return fmt.Errorf("invalid parent-child dependency: parent (%s) cannot depend on child (%s). Use: bd dep add %s %s --type parent-child",
|
// issueExists is the dependent (the one that depends on something)
|
||||||
dep.IssueID, dep.DependsOnID, dep.DependsOnID, dep.IssueID)
|
// dependsOnExists is what it depends on
|
||||||
|
// Correct: Task (child) depends on Epic (parent) - child belongs to parent
|
||||||
|
// Incorrect: Epic (parent) depends on Task (child) - backwards
|
||||||
|
if issueExists.IssueType == types.TypeEpic && dependsOnExists.IssueType != types.TypeEpic {
|
||||||
|
return fmt.Errorf("invalid parent-child dependency: parent (%s) cannot depend on child (%s). Use: bd dep add %s %s --type parent-child",
|
||||||
|
dep.IssueID, dep.DependsOnID, dep.DependsOnID, dep.IssueID)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -152,9 +159,13 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency
|
|||||||
return fmt.Errorf("failed to record event: %w", err)
|
return fmt.Errorf("failed to record event: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mark both issues as dirty for incremental export
|
// Mark issues as dirty for incremental export
|
||||||
// (dependencies are exported with each issue, so both need updating)
|
// For external refs, only mark the source issue (target doesn't exist locally)
|
||||||
if err := markIssuesDirtyTx(ctx, tx, []string{dep.IssueID, dep.DependsOnID}); err != nil {
|
issueIDsToMark := []string{dep.IssueID}
|
||||||
|
if !isExternalRef {
|
||||||
|
issueIDsToMark = append(issueIDsToMark, dep.DependsOnID)
|
||||||
|
}
|
||||||
|
if err := markIssuesDirtyTx(ctx, tx, issueIDsToMark); err != nil {
|
||||||
return wrapDBError("mark issues dirty after adding dependency", err)
|
return wrapDBError("mark issues dirty after adding dependency", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ package sqlite
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"database/sql"
|
"database/sql"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
@@ -73,14 +74,27 @@ func CheckExternalDep(ctx context.Context, ref string) *ExternalDepStatus {
|
|||||||
|
|
||||||
dbPath := cfg.DatabasePath(beadsDir)
|
dbPath := cfg.DatabasePath(beadsDir)
|
||||||
|
|
||||||
// Open the external database (read-only)
|
// Verify database file exists
|
||||||
db, err := sql.Open("sqlite3", dbPath+"?mode=ro")
|
if _, err := os.Stat(dbPath); err != nil {
|
||||||
|
status.Reason = "database file not found: " + dbPath
|
||||||
|
return status
|
||||||
|
}
|
||||||
|
|
||||||
|
// Open the external database
|
||||||
|
// Use regular mode to ensure we can read from WAL-mode databases
|
||||||
|
db, err := sql.Open("sqlite3", dbPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
status.Reason = "cannot open project database"
|
status.Reason = "cannot open project database: " + err.Error()
|
||||||
return status
|
return status
|
||||||
}
|
}
|
||||||
defer func() { _ = db.Close() }()
|
defer func() { _ = db.Close() }()
|
||||||
|
|
||||||
|
// Verify we can ping the database
|
||||||
|
if err := db.Ping(); err != nil {
|
||||||
|
status.Reason = "cannot connect to project database: " + err.Error()
|
||||||
|
return status
|
||||||
|
}
|
||||||
|
|
||||||
// Check for a closed issue with provides:<capability> label
|
// Check for a closed issue with provides:<capability> label
|
||||||
providesLabel := "provides:" + status.Capability
|
providesLabel := "provides:" + status.Capability
|
||||||
var count int
|
var count int
|
||||||
@@ -92,7 +106,7 @@ func CheckExternalDep(ctx context.Context, ref string) *ExternalDepStatus {
|
|||||||
`, providesLabel).Scan(&count)
|
`, providesLabel).Scan(&count)
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
status.Reason = "database query failed"
|
status.Reason = "database query failed: " + err.Error()
|
||||||
return status
|
return status
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ var migrationsList = []Migration{
|
|||||||
{"drop_edge_columns", migrations.MigrateDropEdgeColumns},
|
{"drop_edge_columns", migrations.MigrateDropEdgeColumns},
|
||||||
{"pinned_column", migrations.MigratePinnedColumn},
|
{"pinned_column", migrations.MigratePinnedColumn},
|
||||||
{"is_template_column", migrations.MigrateIsTemplateColumn},
|
{"is_template_column", migrations.MigrateIsTemplateColumn},
|
||||||
|
{"remove_depends_on_fk", migrations.MigrateRemoveDependsOnFK},
|
||||||
}
|
}
|
||||||
|
|
||||||
// MigrationInfo contains metadata about a migration for inspection
|
// MigrationInfo contains metadata about a migration for inspection
|
||||||
@@ -89,6 +90,7 @@ func getMigrationDescription(name string) string {
|
|||||||
"drop_edge_columns": "Drops deprecated edge columns (replies_to, relates_to, duplicate_of, superseded_by) from issues table (Decision 004 Phase 4)",
|
"drop_edge_columns": "Drops deprecated edge columns (replies_to, relates_to, duplicate_of, superseded_by) from issues table (Decision 004 Phase 4)",
|
||||||
"pinned_column": "Adds pinned column for persistent context markers (bd-7h5)",
|
"pinned_column": "Adds pinned column for persistent context markers (bd-7h5)",
|
||||||
"is_template_column": "Adds is_template column for template molecules (beads-1ra)",
|
"is_template_column": "Adds is_template column for template molecules (beads-1ra)",
|
||||||
|
"remove_depends_on_fk": "Removes FK constraint on depends_on_id to allow external references (bd-zmmy)",
|
||||||
}
|
}
|
||||||
|
|
||||||
if desc, ok := descriptions[name]; ok {
|
if desc, ok := descriptions[name]; ok {
|
||||||
|
|||||||
154
internal/storage/sqlite/migrations/025_remove_depends_on_fk.go
Normal file
154
internal/storage/sqlite/migrations/025_remove_depends_on_fk.go
Normal file
@@ -0,0 +1,154 @@
|
|||||||
|
package migrations
|
||||||
|
|
||||||
|
import (
|
||||||
|
"database/sql"
|
||||||
|
)
|
||||||
|
|
||||||
|
// MigrateRemoveDependsOnFK removes the FOREIGN KEY constraint on depends_on_id
|
||||||
|
// to allow external dependencies (external:<project>:<capability>).
|
||||||
|
// See bd-zmmy for design context.
|
||||||
|
func MigrateRemoveDependsOnFK(db *sql.DB) error {
|
||||||
|
// Disable foreign keys for table recreation
|
||||||
|
if _, err := db.Exec(`PRAGMA foreign_keys = OFF`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
defer func() { _, _ = db.Exec(`PRAGMA foreign_keys = ON`) }()
|
||||||
|
|
||||||
|
// Begin transaction for atomic table recreation
|
||||||
|
tx, err := db.Begin()
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
defer func() {
|
||||||
|
if err != nil {
|
||||||
|
_ = tx.Rollback()
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
|
||||||
|
// Drop views that depend on the dependencies table
|
||||||
|
// They will be recreated after the table is rebuilt
|
||||||
|
if _, err = tx.Exec(`DROP VIEW IF EXISTS ready_issues`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if _, err = tx.Exec(`DROP VIEW IF EXISTS blocked_issues`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create new table without FK on depends_on_id
|
||||||
|
// Keep FK on issue_id (source must exist)
|
||||||
|
// Remove FK on depends_on_id (target can be external ref)
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE TABLE dependencies_new (
|
||||||
|
issue_id TEXT NOT NULL,
|
||||||
|
depends_on_id TEXT NOT NULL,
|
||||||
|
type TEXT NOT NULL DEFAULT 'blocks',
|
||||||
|
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||||
|
created_by TEXT NOT NULL,
|
||||||
|
metadata TEXT,
|
||||||
|
thread_id TEXT,
|
||||||
|
PRIMARY KEY (issue_id, depends_on_id, type),
|
||||||
|
FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE
|
||||||
|
)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Copy data from old table
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
INSERT INTO dependencies_new
|
||||||
|
SELECT issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id
|
||||||
|
FROM dependencies
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Drop old table
|
||||||
|
if _, err = tx.Exec(`DROP TABLE dependencies`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Rename new table
|
||||||
|
if _, err = tx.Exec(`ALTER TABLE dependencies_new RENAME TO dependencies`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Recreate indexes
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE INDEX idx_dependencies_issue_id ON dependencies(issue_id)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE INDEX idx_dependencies_depends_on ON dependencies(depends_on_id)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE INDEX idx_dependencies_type ON dependencies(type)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE INDEX idx_dependencies_depends_on_type ON dependencies(depends_on_id, type)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE INDEX idx_dependencies_depends_on_type_issue ON dependencies(depends_on_id, type, issue_id)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Recreate views
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE VIEW IF NOT EXISTS ready_issues AS
|
||||||
|
WITH RECURSIVE
|
||||||
|
blocked_directly AS (
|
||||||
|
SELECT DISTINCT d.issue_id
|
||||||
|
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', 'deferred')
|
||||||
|
),
|
||||||
|
blocked_transitively AS (
|
||||||
|
SELECT issue_id, 0 as depth
|
||||||
|
FROM blocked_directly
|
||||||
|
UNION ALL
|
||||||
|
SELECT d.issue_id, bt.depth + 1
|
||||||
|
FROM blocked_transitively bt
|
||||||
|
JOIN dependencies d ON d.depends_on_id = bt.issue_id
|
||||||
|
WHERE d.type = 'parent-child'
|
||||||
|
AND bt.depth < 50
|
||||||
|
)
|
||||||
|
SELECT i.*
|
||||||
|
FROM issues i
|
||||||
|
WHERE i.status = 'open'
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM blocked_transitively WHERE issue_id = i.id
|
||||||
|
)
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err = tx.Exec(`
|
||||||
|
CREATE VIEW IF NOT EXISTS blocked_issues AS
|
||||||
|
SELECT
|
||||||
|
i.*,
|
||||||
|
COUNT(d.depends_on_id) as blocked_by_count
|
||||||
|
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', 'deferred')
|
||||||
|
AND d.type = 'blocks'
|
||||||
|
AND blocker.status IN ('open', 'in_progress', 'blocked', 'deferred')
|
||||||
|
GROUP BY i.id
|
||||||
|
`); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
return tx.Commit()
|
||||||
|
}
|
||||||
@@ -217,6 +217,30 @@ func (s *SQLiteStorage) importJSONLFile(ctx context.Context, jsonlPath, sourceRe
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check for orphaned local dependencies (non-external refs) (bd-zmmy)
|
||||||
|
// The FK constraint on depends_on_id was removed to allow external:* refs,
|
||||||
|
// so we need to validate local deps manually.
|
||||||
|
orphanRows, err := conn.QueryContext(ctx, `
|
||||||
|
SELECT d.issue_id, d.depends_on_id
|
||||||
|
FROM dependencies d
|
||||||
|
LEFT JOIN issues i ON d.depends_on_id = i.id
|
||||||
|
WHERE i.id IS NULL
|
||||||
|
AND d.depends_on_id NOT LIKE 'external:%'
|
||||||
|
`)
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("failed to check orphaned dependencies: %w", err)
|
||||||
|
}
|
||||||
|
defer orphanRows.Close()
|
||||||
|
|
||||||
|
if orphanRows.Next() {
|
||||||
|
var issueID, dependsOnID string
|
||||||
|
_ = orphanRows.Scan(&issueID, &dependsOnID)
|
||||||
|
return 0, fmt.Errorf(
|
||||||
|
"foreign key violation: issue %s depends on non-existent issue %s",
|
||||||
|
issueID, dependsOnID,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
if err := tx.Commit(); err != nil {
|
if err := tx.Commit(); err != nil {
|
||||||
return 0, fmt.Errorf("failed to commit transaction: %w", err)
|
return 0, fmt.Errorf("failed to commit transaction: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
|
"github.com/steveyegge/beads/internal/config"
|
||||||
"github.com/steveyegge/beads/internal/types"
|
"github.com/steveyegge/beads/internal/types"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -126,7 +127,113 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte
|
|||||||
}
|
}
|
||||||
defer func() { _ = rows.Close() }()
|
defer func() { _ = rows.Close() }()
|
||||||
|
|
||||||
return s.scanIssues(ctx, rows)
|
issues, err := s.scanIssues(ctx, rows)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Filter out issues with unsatisfied external dependencies (bd-zmmy)
|
||||||
|
// Only check if external_projects are configured
|
||||||
|
if len(config.GetExternalProjects()) > 0 && len(issues) > 0 {
|
||||||
|
issues, err = s.filterByExternalDeps(ctx, issues)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to check external dependencies: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return issues, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// filterByExternalDeps removes issues that have unsatisfied external dependencies.
|
||||||
|
// External deps have format: external:<project>:<capability>
|
||||||
|
// They are satisfied when the target project has a closed issue with provides:<capability> label.
|
||||||
|
func (s *SQLiteStorage) filterByExternalDeps(ctx context.Context, issues []*types.Issue) ([]*types.Issue, error) {
|
||||||
|
if len(issues) == 0 {
|
||||||
|
return issues, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build list of issue IDs
|
||||||
|
issueIDs := make([]string, len(issues))
|
||||||
|
for i, issue := range issues {
|
||||||
|
issueIDs[i] = issue.ID
|
||||||
|
}
|
||||||
|
|
||||||
|
// Batch query: get all external deps for these issues
|
||||||
|
externalDeps, err := s.getExternalDepsForIssues(ctx, issueIDs)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
// If no external deps, return all issues
|
||||||
|
if len(externalDeps) == 0 {
|
||||||
|
return issues, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check each external dep and build set of blocked issue IDs
|
||||||
|
blockedIssues := make(map[string]bool)
|
||||||
|
for issueID, deps := range externalDeps {
|
||||||
|
for _, dep := range deps {
|
||||||
|
status := CheckExternalDep(ctx, dep)
|
||||||
|
if !status.Satisfied {
|
||||||
|
blockedIssues[issueID] = true
|
||||||
|
break // One unsatisfied dep is enough to block
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Filter out blocked issues
|
||||||
|
if len(blockedIssues) == 0 {
|
||||||
|
return issues, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
result := make([]*types.Issue, 0, len(issues)-len(blockedIssues))
|
||||||
|
for _, issue := range issues {
|
||||||
|
if !blockedIssues[issue.ID] {
|
||||||
|
result = append(result, issue)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// getExternalDepsForIssues returns a map of issue ID -> list of external dep refs
|
||||||
|
func (s *SQLiteStorage) getExternalDepsForIssues(ctx context.Context, issueIDs []string) (map[string][]string, error) {
|
||||||
|
if len(issueIDs) == 0 {
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build placeholders for IN clause
|
||||||
|
placeholders := make([]string, len(issueIDs))
|
||||||
|
args := make([]interface{}, len(issueIDs))
|
||||||
|
for i, id := range issueIDs {
|
||||||
|
placeholders[i] = "?"
|
||||||
|
args[i] = id
|
||||||
|
}
|
||||||
|
|
||||||
|
query := fmt.Sprintf(`
|
||||||
|
SELECT issue_id, depends_on_id
|
||||||
|
FROM dependencies
|
||||||
|
WHERE issue_id IN (%s)
|
||||||
|
AND type = 'blocks'
|
||||||
|
AND depends_on_id LIKE 'external:%%'
|
||||||
|
`, strings.Join(placeholders, ","))
|
||||||
|
|
||||||
|
rows, err := s.db.QueryContext(ctx, query, args...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to query external dependencies: %w", err)
|
||||||
|
}
|
||||||
|
defer func() { _ = rows.Close() }()
|
||||||
|
|
||||||
|
result := make(map[string][]string)
|
||||||
|
for rows.Next() {
|
||||||
|
var issueID, depRef string
|
||||||
|
if err := rows.Scan(&issueID, &depRef); err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to scan external dependency: %w", err)
|
||||||
|
}
|
||||||
|
result[issueID] = append(result[issueID], depRef)
|
||||||
|
}
|
||||||
|
|
||||||
|
return result, rows.Err()
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetStaleIssues returns issues that haven't been updated recently
|
// GetStaleIssues returns issues that haven't been updated recently
|
||||||
|
|||||||
@@ -2,9 +2,13 @@ package sqlite
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/steveyegge/beads/internal/config"
|
||||||
|
"github.com/steveyegge/beads/internal/configfile"
|
||||||
"github.com/steveyegge/beads/internal/types"
|
"github.com/steveyegge/beads/internal/types"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -1128,3 +1132,255 @@ func TestSortPolicyDefault(t *testing.T) {
|
|||||||
t.Errorf("Expected P2 second, got P%d", ready[1].Priority)
|
t.Errorf("Expected P2 second, got P%d", ready[1].Priority)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGetReadyWorkExternalDeps tests that GetReadyWork filters out issues
|
||||||
|
// with unsatisfied external dependencies (bd-zmmy)
|
||||||
|
func TestGetReadyWorkExternalDeps(t *testing.T) {
|
||||||
|
// Create main test database
|
||||||
|
mainStore, mainCleanup := setupTestDB(t)
|
||||||
|
defer mainCleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create external project directory with beads database
|
||||||
|
externalDir, err := os.MkdirTemp("", "beads-external-test-*")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create external temp dir: %v", err)
|
||||||
|
}
|
||||||
|
defer os.RemoveAll(externalDir)
|
||||||
|
|
||||||
|
// Create .beads directory and config in external project
|
||||||
|
beadsDir := filepath.Join(externalDir, ".beads")
|
||||||
|
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||||
|
t.Fatalf("failed to create .beads dir: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create config file for external project
|
||||||
|
cfg := configfile.DefaultConfig()
|
||||||
|
if err := cfg.Save(beadsDir); err != nil {
|
||||||
|
t.Fatalf("failed to save external config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create external database (must match configfile.DefaultConfig().Database)
|
||||||
|
externalDBPath := filepath.Join(beadsDir, "beads.db")
|
||||||
|
externalStore, err := New(ctx, externalDBPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create external store: %v", err)
|
||||||
|
}
|
||||||
|
defer externalStore.Close()
|
||||||
|
|
||||||
|
// Set issue_prefix in external store
|
||||||
|
if err := externalStore.SetConfig(ctx, "issue_prefix", "ext"); err != nil {
|
||||||
|
t.Fatalf("failed to set external issue_prefix: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Initialize config if not already done (required for Set to work)
|
||||||
|
if err := config.Initialize(); err != nil {
|
||||||
|
t.Fatalf("failed to initialize config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Configure external_projects to point to our temp external project
|
||||||
|
// Save current value to restore later
|
||||||
|
oldProjects := config.GetExternalProjects()
|
||||||
|
defer func() {
|
||||||
|
if oldProjects != nil {
|
||||||
|
config.Set("external_projects", oldProjects)
|
||||||
|
} else {
|
||||||
|
config.Set("external_projects", map[string]string{})
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
|
||||||
|
config.Set("external_projects", map[string]string{
|
||||||
|
"external-test": externalDir,
|
||||||
|
})
|
||||||
|
|
||||||
|
// Create an issue in main DB with external dependency
|
||||||
|
issueWithExtDep := &types.Issue{
|
||||||
|
Title: "Has external dep",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := mainStore.CreateIssue(ctx, issueWithExtDep, "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to create issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add external dependency
|
||||||
|
extDep := &types.Dependency{
|
||||||
|
IssueID: issueWithExtDep.ID,
|
||||||
|
DependsOnID: "external:external-test:test-capability",
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}
|
||||||
|
if err := mainStore.AddDependency(ctx, extDep, "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to add external dependency: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create a regular issue without external dep
|
||||||
|
regularIssue := &types.Issue{
|
||||||
|
Title: "Regular issue",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 2,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := mainStore.CreateIssue(ctx, regularIssue, "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to create regular issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Debug: check config
|
||||||
|
projects := config.GetExternalProjects()
|
||||||
|
t.Logf("External projects config: %v", projects)
|
||||||
|
|
||||||
|
resolvedPath := config.ResolveExternalProjectPath("external-test")
|
||||||
|
t.Logf("Resolved path for 'external-test': %s", resolvedPath)
|
||||||
|
|
||||||
|
// Test 1: External dep is not satisfied - issue should be blocked
|
||||||
|
ready, err := mainStore.GetReadyWork(ctx, types.WorkFilter{})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetReadyWork failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Debug: log what we got
|
||||||
|
for _, issue := range ready {
|
||||||
|
t.Logf("Ready issue: %s - %s", issue.ID, issue.Title)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should only have the regular issue (external dep not satisfied)
|
||||||
|
if len(ready) != 1 {
|
||||||
|
t.Errorf("Expected 1 ready issue (external dep not satisfied), got %d", len(ready))
|
||||||
|
}
|
||||||
|
if len(ready) > 0 && ready[0].ID != regularIssue.ID {
|
||||||
|
t.Errorf("Expected regular issue %s to be ready, got %s", regularIssue.ID, ready[0].ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test 2: Ship the capability in external project
|
||||||
|
// Create an issue with provides:test-capability label and close it
|
||||||
|
capabilityIssue := &types.Issue{
|
||||||
|
Title: "Ship test-capability",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := externalStore.CreateIssue(ctx, capabilityIssue, "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to create capability issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add the provides: label
|
||||||
|
if err := externalStore.AddLabel(ctx, capabilityIssue.ID, "provides:test-capability", "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to add provides label: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Close the capability issue
|
||||||
|
if err := externalStore.CloseIssue(ctx, capabilityIssue.ID, "Shipped", "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to close capability issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Debug: verify the capability issue was properly set up
|
||||||
|
capIssue, err := externalStore.GetIssue(ctx, capabilityIssue.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to get capability issue: %v", err)
|
||||||
|
}
|
||||||
|
t.Logf("Capability issue status: %s", capIssue.Status)
|
||||||
|
labels, _ := externalStore.GetLabels(ctx, capabilityIssue.ID)
|
||||||
|
t.Logf("Capability issue labels: %v", labels)
|
||||||
|
|
||||||
|
// Close external store to checkpoint WAL before read-only access
|
||||||
|
externalStore.Close()
|
||||||
|
|
||||||
|
// Debug: check what path configfile.Load returns
|
||||||
|
testCfg, _ := configfile.Load(beadsDir)
|
||||||
|
if testCfg != nil {
|
||||||
|
t.Logf("Config database path: %s", testCfg.DatabasePath(beadsDir))
|
||||||
|
t.Logf("External DB path we created: %s", externalDBPath)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Re-verify: manually check the external dep
|
||||||
|
status := CheckExternalDep(ctx, "external:external-test:test-capability")
|
||||||
|
t.Logf("External dep check: satisfied=%v, reason=%s", status.Satisfied, status.Reason)
|
||||||
|
|
||||||
|
// Now the external dep should be satisfied
|
||||||
|
ready, err = mainStore.GetReadyWork(ctx, types.WorkFilter{})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetReadyWork failed after shipping: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should now have both issues
|
||||||
|
if len(ready) != 2 {
|
||||||
|
t.Errorf("Expected 2 ready issues (external dep now satisfied), got %d", len(ready))
|
||||||
|
for _, issue := range ready {
|
||||||
|
t.Logf("Ready issue after shipping: %s - %s", issue.ID, issue.Title)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify both issues are present
|
||||||
|
foundExtDep := false
|
||||||
|
foundRegular := false
|
||||||
|
for _, issue := range ready {
|
||||||
|
if issue.ID == issueWithExtDep.ID {
|
||||||
|
foundExtDep = true
|
||||||
|
}
|
||||||
|
if issue.ID == regularIssue.ID {
|
||||||
|
foundRegular = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !foundExtDep {
|
||||||
|
t.Error("Issue with external dep should now be ready")
|
||||||
|
}
|
||||||
|
if !foundRegular {
|
||||||
|
t.Error("Regular issue should still be ready")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGetReadyWorkNoExternalProjectsConfigured tests that GetReadyWork
|
||||||
|
// works normally when no external_projects are configured (bd-zmmy)
|
||||||
|
func TestGetReadyWorkNoExternalProjectsConfigured(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Initialize config if not already done
|
||||||
|
if err := config.Initialize(); err != nil {
|
||||||
|
t.Fatalf("failed to initialize config: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Ensure no external_projects configured
|
||||||
|
oldProjects := config.GetExternalProjects()
|
||||||
|
defer func() {
|
||||||
|
if oldProjects != nil {
|
||||||
|
config.Set("external_projects", oldProjects)
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
config.Set("external_projects", map[string]string{})
|
||||||
|
|
||||||
|
// Create an issue with an external dependency (shouldn't matter since no config)
|
||||||
|
issue := &types.Issue{
|
||||||
|
Title: "Has external dep but no config",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, issue, "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to create issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add external dependency (will be ignored since no external_projects configured)
|
||||||
|
extDep := &types.Dependency{
|
||||||
|
IssueID: issue.ID,
|
||||||
|
DependsOnID: "external:unconfigured-project:some-capability",
|
||||||
|
Type: types.DepBlocks,
|
||||||
|
}
|
||||||
|
if err := store.AddDependency(ctx, extDep, "test-user"); err != nil {
|
||||||
|
t.Fatalf("failed to add external dependency: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should skip external dep checking since no external_projects configured
|
||||||
|
ready, err := store.GetReadyWork(ctx, types.WorkFilter{})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetReadyWork failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Issue should be ready (external deps skipped when no config)
|
||||||
|
if len(ready) != 1 {
|
||||||
|
t.Errorf("Expected 1 ready issue (external deps skipped), got %d", len(ready))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -600,7 +600,7 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
|
|||||||
return fmt.Errorf("invalid dependency type: %q (must be non-empty string, max 50 chars)", dep.Type)
|
return fmt.Errorf("invalid dependency type: %q (must be non-empty string, max 50 chars)", dep.Type)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate that both issues exist
|
// Validate that source issue exists
|
||||||
issueExists, err := t.GetIssue(ctx, dep.IssueID)
|
issueExists, err := t.GetIssue(ctx, dep.IssueID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to check issue %s: %w", dep.IssueID, err)
|
return fmt.Errorf("failed to check issue %s: %w", dep.IssueID, err)
|
||||||
@@ -609,24 +609,31 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
|
|||||||
return fmt.Errorf("issue %s not found", dep.IssueID)
|
return fmt.Errorf("issue %s not found", dep.IssueID)
|
||||||
}
|
}
|
||||||
|
|
||||||
dependsOnExists, err := t.GetIssue(ctx, dep.DependsOnID)
|
// External refs (external:<project>:<capability>) don't need target validation (bd-zmmy)
|
||||||
if err != nil {
|
// They are resolved lazily at query time by CheckExternalDep
|
||||||
return fmt.Errorf("failed to check dependency %s: %w", dep.DependsOnID, err)
|
isExternalRef := strings.HasPrefix(dep.DependsOnID, "external:")
|
||||||
}
|
|
||||||
if dependsOnExists == nil {
|
|
||||||
return fmt.Errorf("dependency target %s not found", dep.DependsOnID)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Prevent self-dependency
|
var dependsOnExists *types.Issue
|
||||||
if dep.IssueID == dep.DependsOnID {
|
if !isExternalRef {
|
||||||
return fmt.Errorf("issue cannot depend on itself")
|
dependsOnExists, err = t.GetIssue(ctx, dep.DependsOnID)
|
||||||
}
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check dependency %s: %w", dep.DependsOnID, err)
|
||||||
|
}
|
||||||
|
if dependsOnExists == nil {
|
||||||
|
return fmt.Errorf("dependency target %s not found", dep.DependsOnID)
|
||||||
|
}
|
||||||
|
|
||||||
// Validate parent-child dependency direction
|
// Prevent self-dependency (only for local deps)
|
||||||
if dep.Type == types.DepParentChild {
|
if dep.IssueID == dep.DependsOnID {
|
||||||
if issueExists.IssueType == types.TypeEpic && dependsOnExists.IssueType != types.TypeEpic {
|
return fmt.Errorf("issue cannot depend on itself")
|
||||||
return fmt.Errorf("invalid parent-child dependency: parent (%s) cannot depend on child (%s). Use: bd dep add %s %s --type parent-child",
|
}
|
||||||
dep.IssueID, dep.DependsOnID, dep.DependsOnID, dep.IssueID)
|
|
||||||
|
// Validate parent-child dependency direction (only for local deps)
|
||||||
|
if dep.Type == types.DepParentChild {
|
||||||
|
if issueExists.IssueType == types.TypeEpic && dependsOnExists.IssueType != types.TypeEpic {
|
||||||
|
return fmt.Errorf("invalid parent-child dependency: parent (%s) cannot depend on child (%s). Use: bd dep add %s %s --type parent-child",
|
||||||
|
dep.IssueID, dep.DependsOnID, dep.DependsOnID, dep.IssueID)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -695,12 +702,14 @@ func (t *sqliteTxStorage) AddDependency(ctx context.Context, dep *types.Dependen
|
|||||||
return fmt.Errorf("failed to record event: %w", err)
|
return fmt.Errorf("failed to record event: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Mark both issues as dirty
|
// Mark issues as dirty - for external refs, only mark the source issue
|
||||||
if err := markDirty(ctx, t.conn, dep.IssueID); err != nil {
|
if err := markDirty(ctx, t.conn, dep.IssueID); err != nil {
|
||||||
return fmt.Errorf("failed to mark issue dirty: %w", err)
|
return fmt.Errorf("failed to mark issue dirty: %w", err)
|
||||||
}
|
}
|
||||||
if err := markDirty(ctx, t.conn, dep.DependsOnID); err != nil {
|
if !isExternalRef {
|
||||||
return fmt.Errorf("failed to mark depends-on issue dirty: %w", err)
|
if err := markDirty(ctx, t.conn, dep.DependsOnID); err != nil {
|
||||||
|
return fmt.Errorf("failed to mark depends-on issue dirty: %w", err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Invalidate blocked cache for blocking dependencies (bd-1c4h)
|
// Invalidate blocked cache for blocking dependencies (bd-1c4h)
|
||||||
|
|||||||
Reference in New Issue
Block a user