fix(ready): exclude molecule steps from bd ready by default (#1246)
* fix(ready): exclude molecule steps from bd ready by default (GH#1239) Add ID prefix constants (IDPrefixMol, IDPrefixWisp) to types.go as single source of truth. Update pour.go and wisp.go to use these constants. GetReadyWork now excludes issues with -mol- in their ID when no explicit type filter is specified. Users can still see mol steps with --type=task. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(ready): config-driven ID pattern exclusion (GH#1239) Add ready.exclude_id_patterns config for excluding IDs from bd ready. Default patterns: -mol-, -wisp- (molecule steps and wisps). Changes: - Add IncludeMolSteps to WorkFilter for internal callers - Update findGateReadyMolecules and getMoleculeCurrentStep to use it - Make exclusion patterns config-driven via ready.exclude_id_patterns - Remove hardcoded MolStepIDPattern() in favor of config - Add test for custom patterns (e.g., gastown's -role-) Usage: bd config set ready.exclude_id_patterns "-mol-,-wisp-,-role-" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: remove -role- example from ready.go comments Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: remove GH#1239 references from code comments Issue references belong in commit messages, not code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -184,8 +184,9 @@ func getMoleculeProgress(ctx context.Context, s storage.Storage, moleculeID stri
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Get ready issues for this molecule
|
// Get ready issues for this molecule
|
||||||
|
// IncludeMolSteps: true because we specifically need to see molecule steps here
|
||||||
readyIDs := make(map[string]bool)
|
readyIDs := make(map[string]bool)
|
||||||
readyIssues, err := s.GetReadyWork(ctx, types.WorkFilter{})
|
readyIssues, err := s.GetReadyWork(ctx, types.WorkFilter{IncludeMolSteps: true})
|
||||||
if err == nil {
|
if err == nil {
|
||||||
for _, issue := range readyIssues {
|
for _, issue := range readyIssues {
|
||||||
readyIDs[issue.ID] = true
|
readyIDs[issue.ID] = true
|
||||||
|
|||||||
@@ -131,7 +131,8 @@ func findGateReadyMolecules(ctx context.Context, s storage.Storage) ([]*GatedMol
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Step 2: Get ready work to check which steps are ready
|
// Step 2: Get ready work to check which steps are ready
|
||||||
readyIssues, err := s.GetReadyWork(ctx, types.WorkFilter{Limit: 500})
|
// IncludeMolSteps: true because we specifically need to see molecule steps here
|
||||||
|
readyIssues, err := s.GetReadyWork(ctx, types.WorkFilter{Limit: 500, IncludeMolSteps: true})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("getting ready work: %w", err)
|
return nil, fmt.Errorf("getting ready work: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2682,7 +2682,7 @@ func TestPourRootTitleDescSubstitution(t *testing.T) {
|
|||||||
"desc": "My description",
|
"desc": "My description",
|
||||||
}
|
}
|
||||||
|
|
||||||
result, err := spawnMolecule(ctx, s, subgraph, vars, "", "test", false, "mol")
|
result, err := spawnMolecule(ctx, s, subgraph, vars, "", "test", false, types.IDPrefixMol)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("spawnMolecule failed: %v", err)
|
t.Fatalf("spawnMolecule failed: %v", err)
|
||||||
}
|
}
|
||||||
@@ -2755,7 +2755,7 @@ func TestPourRootTitleOnly(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
vars := map[string]string{"title": "Custom Title"}
|
vars := map[string]string{"title": "Custom Title"}
|
||||||
result, err := spawnMolecule(ctx, s, subgraph, vars, "", "test", false, "mol")
|
result, err := spawnMolecule(ctx, s, subgraph, vars, "", "test", false, types.IDPrefixMol)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("spawnMolecule failed: %v", err)
|
t.Fatalf("spawnMolecule failed: %v", err)
|
||||||
}
|
}
|
||||||
@@ -2811,7 +2811,7 @@ func TestPourRootNoVars(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
vars := map[string]string{"version": "1.2.3"}
|
vars := map[string]string{"version": "1.2.3"}
|
||||||
result, err := spawnMolecule(ctx, s, subgraph, vars, "", "test", false, "mol")
|
result, err := spawnMolecule(ctx, s, subgraph, vars, "", "test", false, types.IDPrefixMol)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("spawnMolecule failed: %v", err)
|
t.Fatalf("spawnMolecule failed: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -226,8 +226,8 @@ func runPour(cmd *cobra.Command, args []string) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Spawn as persistent mol (ephemeral=false)
|
// Spawn as persistent mol (ephemeral=false)
|
||||||
// Use "mol" prefix for distinct visual recognition
|
// Use mol prefix for distinct visual recognition (see types.IDPrefixMol)
|
||||||
result, err := spawnMolecule(ctx, store, subgraph, vars, assignee, actor, false, "mol")
|
result, err := spawnMolecule(ctx, store, subgraph, vars, assignee, actor, false, types.IDPrefixMol)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error pouring proto: %v\n", err)
|
fmt.Fprintf(os.Stderr, "Error pouring proto: %v\n", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
|
|||||||
@@ -262,8 +262,8 @@ func runWispCreate(cmd *cobra.Command, args []string) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Spawn as ephemeral in main database (Ephemeral=true, skips JSONL export)
|
// Spawn as ephemeral in main database (Ephemeral=true, skips JSONL export)
|
||||||
// Use "wisp" prefix for distinct visual recognition
|
// Use wisp prefix for distinct visual recognition (see types.IDPrefixWisp)
|
||||||
result, err := spawnMolecule(ctx, store, subgraph, vars, "", actor, true, "wisp")
|
result, err := spawnMolecule(ctx, store, subgraph, vars, "", actor, true, types.IDPrefixWisp)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Fprintf(os.Stderr, "Error creating wisp: %v\n", err)
|
fmt.Fprintf(os.Stderr, "Error creating wisp: %v\n", err)
|
||||||
os.Exit(1)
|
os.Exit(1)
|
||||||
|
|||||||
@@ -18,8 +18,7 @@ import (
|
|||||||
func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilter) ([]*types.Issue, error) {
|
func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilter) ([]*types.Issue, error) {
|
||||||
whereClauses := []string{
|
whereClauses := []string{
|
||||||
"i.pinned = 0", // Exclude pinned issues
|
"i.pinned = 0", // Exclude pinned issues
|
||||||
"(i.ephemeral = 0 OR i.ephemeral IS NULL)", // Exclude wisps
|
"(i.ephemeral = 0 OR i.ephemeral IS NULL)", // Exclude wisps by ephemeral flag
|
||||||
"i.id NOT LIKE '%-wisp-%'", // Defense in depth: exclude wisp IDs even if ephemeral flag missing
|
|
||||||
}
|
}
|
||||||
args := []interface{}{}
|
args := []interface{}{}
|
||||||
|
|
||||||
@@ -46,6 +45,16 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte
|
|||||||
// - role: agent role definitions (reference metadata)
|
// - role: agent role definitions (reference metadata)
|
||||||
// - rig: rig identity beads (reference metadata)
|
// - rig: rig identity beads (reference metadata)
|
||||||
whereClauses = append(whereClauses, "i.issue_type NOT IN ('merge-request', 'gate', 'molecule', 'message', 'agent', 'role', 'rig')")
|
whereClauses = append(whereClauses, "i.issue_type NOT IN ('merge-request', 'gate', 'molecule', 'message', 'agent', 'role', 'rig')")
|
||||||
|
// Exclude IDs matching configured patterns
|
||||||
|
// Default patterns: -mol- (molecule steps), -wisp- (ephemeral wisps)
|
||||||
|
// Configure with: bd config set ready.exclude_id_patterns "-mol-,-wisp-"
|
||||||
|
// Use --type=task to explicitly include them, or IncludeMolSteps for internal callers
|
||||||
|
if !filter.IncludeMolSteps {
|
||||||
|
patterns := s.getExcludeIDPatterns(ctx)
|
||||||
|
for _, pattern := range patterns {
|
||||||
|
whereClauses = append(whereClauses, "i.id NOT LIKE '%"+pattern+"%'")
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if filter.Priority != nil {
|
if filter.Priority != nil {
|
||||||
@@ -824,3 +833,35 @@ func buildOrderByClause(policy types.SortPolicy) string {
|
|||||||
i.created_at ASC`
|
i.created_at ASC`
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ExcludeIDPatternsConfigKey is the config key for ID exclusion patterns in GetReadyWork
|
||||||
|
const ExcludeIDPatternsConfigKey = "ready.exclude_id_patterns"
|
||||||
|
|
||||||
|
// DefaultExcludeIDPatterns are the default patterns to exclude from GetReadyWork
|
||||||
|
// These exclude molecule steps (-mol-) and wisps (-wisp-) which are internal workflow items
|
||||||
|
var DefaultExcludeIDPatterns = []string{"-mol-", "-wisp-"}
|
||||||
|
|
||||||
|
// getExcludeIDPatterns returns the ID patterns to exclude from GetReadyWork.
|
||||||
|
// Reads from ready.exclude_id_patterns config, defaults to DefaultExcludeIDPatterns.
|
||||||
|
// Config format: comma-separated patterns, e.g., "-mol-,-wisp-"
|
||||||
|
func (s *SQLiteStorage) getExcludeIDPatterns(ctx context.Context) []string {
|
||||||
|
value, err := s.GetConfig(ctx, ExcludeIDPatternsConfigKey)
|
||||||
|
if err != nil || value == "" {
|
||||||
|
return DefaultExcludeIDPatterns
|
||||||
|
}
|
||||||
|
|
||||||
|
// Parse comma-separated patterns
|
||||||
|
parts := strings.Split(value, ",")
|
||||||
|
patterns := make([]string, 0, len(parts))
|
||||||
|
for _, p := range parts {
|
||||||
|
p = strings.TrimSpace(p)
|
||||||
|
if p != "" {
|
||||||
|
patterns = append(patterns, p)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(patterns) == 0 {
|
||||||
|
return DefaultExcludeIDPatterns
|
||||||
|
}
|
||||||
|
return patterns
|
||||||
|
}
|
||||||
|
|||||||
@@ -1905,3 +1905,128 @@ func TestIsBlocked(t *testing.T) {
|
|||||||
t.Errorf("Expected issue3 to NOT be blocked (blocker is closed), got blocked=true with blockers=%v", blockers)
|
t.Errorf("Expected issue3 to NOT be blocked (blocker is closed), got blocked=true with blockers=%v", blockers)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGetReadyWorkExcludesMolSteps tests that molecule steps (IDs containing -mol-) are
|
||||||
|
// excluded from bd ready by default, but included when filtering by explicit type.
|
||||||
|
func TestGetReadyWorkExcludesMolSteps(t *testing.T) {
|
||||||
|
env := newTestEnv(t)
|
||||||
|
|
||||||
|
// Create regular tasks
|
||||||
|
regularTask := env.CreateIssue("Regular task")
|
||||||
|
|
||||||
|
// Create molecule steps (IDs contain -mol-)
|
||||||
|
molStep1 := env.CreateIssueWithID("bd-mol-abc", "Mol step 1")
|
||||||
|
molStep2 := env.CreateIssueWithID("bd-mol-xyz", "Mol step 2")
|
||||||
|
|
||||||
|
// Default query should exclude mol steps
|
||||||
|
ready := env.GetReadyWork(types.WorkFilter{})
|
||||||
|
readyIDs := make(map[string]bool)
|
||||||
|
for _, issue := range ready {
|
||||||
|
readyIDs[issue.ID] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
// Regular task should be included
|
||||||
|
if !readyIDs[regularTask.ID] {
|
||||||
|
t.Errorf("Expected regular task %s to be in ready work", regularTask.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mol steps should be excluded
|
||||||
|
if readyIDs[molStep1.ID] {
|
||||||
|
t.Errorf("Expected mol step %s to be EXCLUDED from ready work by default", molStep1.ID)
|
||||||
|
}
|
||||||
|
if readyIDs[molStep2.ID] {
|
||||||
|
t.Errorf("Expected mol step %s to be EXCLUDED from ready work by default", molStep2.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Explicit type=task filter should include mol steps
|
||||||
|
readyWithType := env.GetReadyWork(types.WorkFilter{Type: "task"})
|
||||||
|
readyWithTypeIDs := make(map[string]bool)
|
||||||
|
for _, issue := range readyWithType {
|
||||||
|
readyWithTypeIDs[issue.ID] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
// All tasks should be included when filtering by type
|
||||||
|
if !readyWithTypeIDs[regularTask.ID] {
|
||||||
|
t.Errorf("Expected regular task %s to be in ready work with --type=task", regularTask.ID)
|
||||||
|
}
|
||||||
|
if !readyWithTypeIDs[molStep1.ID] {
|
||||||
|
t.Errorf("Expected mol step %s to be INCLUDED with --type=task filter", molStep1.ID)
|
||||||
|
}
|
||||||
|
if !readyWithTypeIDs[molStep2.ID] {
|
||||||
|
t.Errorf("Expected mol step %s to be INCLUDED with --type=task filter", molStep2.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGetReadyWorkExcludeIDPatternsConfig tests custom exclusion patterns via config.
|
||||||
|
func TestGetReadyWorkExcludeIDPatternsConfig(t *testing.T) {
|
||||||
|
env := newTestEnv(t)
|
||||||
|
ctx := env.Ctx
|
||||||
|
|
||||||
|
// Create issues with various ID patterns
|
||||||
|
regularTask := env.CreateIssue("Regular task")
|
||||||
|
molStep := env.CreateIssueWithID("bd-mol-abc", "Mol step")
|
||||||
|
roleStep := env.CreateIssueWithID("bd-role-xyz", "Role step")
|
||||||
|
wispStep := env.CreateIssueWithID("bd-wisp-123", "Wisp step")
|
||||||
|
|
||||||
|
// Default config excludes -mol- and -wisp-
|
||||||
|
ready := env.GetReadyWork(types.WorkFilter{})
|
||||||
|
readyIDs := make(map[string]bool)
|
||||||
|
for _, issue := range ready {
|
||||||
|
readyIDs[issue.ID] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
if !readyIDs[regularTask.ID] {
|
||||||
|
t.Errorf("Expected regular task to be included")
|
||||||
|
}
|
||||||
|
if readyIDs[molStep.ID] {
|
||||||
|
t.Errorf("Expected mol step to be excluded by default")
|
||||||
|
}
|
||||||
|
if readyIDs[wispStep.ID] {
|
||||||
|
t.Errorf("Expected wisp step to be excluded by default")
|
||||||
|
}
|
||||||
|
if !readyIDs[roleStep.ID] {
|
||||||
|
t.Errorf("Expected role step to be included (not in default patterns)")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Configure custom patterns to also exclude -role-
|
||||||
|
if err := env.Store.SetConfig(ctx, ExcludeIDPatternsConfigKey, "-mol-,-wisp-,-role-"); err != nil {
|
||||||
|
t.Fatalf("SetConfig failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ready2 := env.GetReadyWork(types.WorkFilter{})
|
||||||
|
readyIDs2 := make(map[string]bool)
|
||||||
|
for _, issue := range ready2 {
|
||||||
|
readyIDs2[issue.ID] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
if !readyIDs2[regularTask.ID] {
|
||||||
|
t.Errorf("Expected regular task to be included with custom config")
|
||||||
|
}
|
||||||
|
if readyIDs2[molStep.ID] {
|
||||||
|
t.Errorf("Expected mol step to be excluded with custom config")
|
||||||
|
}
|
||||||
|
if readyIDs2[wispStep.ID] {
|
||||||
|
t.Errorf("Expected wisp step to be excluded with custom config")
|
||||||
|
}
|
||||||
|
if readyIDs2[roleStep.ID] {
|
||||||
|
t.Errorf("Expected role step to be excluded with custom config")
|
||||||
|
}
|
||||||
|
|
||||||
|
// IncludeMolSteps should bypass all pattern exclusions
|
||||||
|
ready3 := env.GetReadyWork(types.WorkFilter{IncludeMolSteps: true})
|
||||||
|
readyIDs3 := make(map[string]bool)
|
||||||
|
for _, issue := range ready3 {
|
||||||
|
readyIDs3[issue.ID] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
if !readyIDs3[regularTask.ID] {
|
||||||
|
t.Errorf("Expected regular task with IncludeMolSteps")
|
||||||
|
}
|
||||||
|
if !readyIDs3[molStep.ID] {
|
||||||
|
t.Errorf("Expected mol step with IncludeMolSteps: true")
|
||||||
|
}
|
||||||
|
if !readyIDs3[roleStep.ID] {
|
||||||
|
t.Errorf("Expected role step with IncludeMolSteps: true")
|
||||||
|
}
|
||||||
|
// Note: wisp step is excluded by ephemeral flag, not pattern, so it stays excluded
|
||||||
|
}
|
||||||
|
|||||||
@@ -149,6 +149,23 @@ func (e *testEnv) AssertBlocked(issue *types.Issue) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// CreateIssueWithID creates a test issue with an explicit ID.
|
||||||
|
// Useful for testing ID-based filtering (e.g., mol step exclusion).
|
||||||
|
func (e *testEnv) CreateIssueWithID(id, title string) *types.Issue {
|
||||||
|
e.t.Helper()
|
||||||
|
issue := &types.Issue{
|
||||||
|
ID: id,
|
||||||
|
Title: title,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 2,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := e.Store.CreateIssue(e.Ctx, issue, "test-user"); err != nil {
|
||||||
|
e.t.Fatalf("CreateIssue(%q, %q) failed: %v", id, title, err)
|
||||||
|
}
|
||||||
|
return issue
|
||||||
|
}
|
||||||
|
|
||||||
// newTestStore creates a SQLiteStorage with issue_prefix configured (bd-166)
|
// newTestStore creates a SQLiteStorage with issue_prefix configured (bd-166)
|
||||||
// This prevents "database not initialized" errors in tests
|
// This prevents "database not initialized" errors in tests
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -1007,6 +1007,11 @@ type WorkFilter struct {
|
|||||||
|
|
||||||
// Time-based deferral filtering (GH#820)
|
// Time-based deferral filtering (GH#820)
|
||||||
IncludeDeferred bool // If true, include issues with future defer_until timestamps
|
IncludeDeferred bool // If true, include issues with future defer_until timestamps
|
||||||
|
|
||||||
|
// Molecule step filtering
|
||||||
|
// By default, GetReadyWork excludes mol/wisp steps (IDs containing -mol- or -wisp-)
|
||||||
|
// Set to true for internal callers that need to see mol steps (e.g., findGateReadyMolecules)
|
||||||
|
IncludeMolSteps bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// StaleFilter is used to filter stale issue queries
|
// StaleFilter is used to filter stale issue queries
|
||||||
@@ -1041,6 +1046,15 @@ const (
|
|||||||
BondTypeRoot = "root" // Marks the primary/root component
|
BondTypeRoot = "root" // Marks the primary/root component
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ID prefix constants for molecule/wisp instantiation.
|
||||||
|
// These prefixes are inserted into issue IDs: <project>-<prefix>-<id>
|
||||||
|
// Used by: cmd/bd/pour.go, cmd/bd/wisp.go (ID generation)
|
||||||
|
// Exclusion from bd ready is config-driven via ready.exclude_id_patterns (default: -mol-,-wisp-)
|
||||||
|
const (
|
||||||
|
IDPrefixMol = "mol" // Persistent molecules (bd-mol-xxx)
|
||||||
|
IDPrefixWisp = "wisp" // Ephemeral wisps (bd-wisp-xxx)
|
||||||
|
)
|
||||||
|
|
||||||
// IsCompound returns true if this issue is a compound (bonded from multiple sources).
|
// IsCompound returns true if this issue is a compound (bonded from multiple sources).
|
||||||
func (i *Issue) IsCompound() bool {
|
func (i *Issue) IsCompound() bool {
|
||||||
return len(i.BondedFrom) > 0
|
return len(i.BondedFrom) > 0
|
||||||
|
|||||||
Reference in New Issue
Block a user