3da0d5a7c8
* fix(molecule): use Dependencies from bd show instead of empty DependsOn Bug: Molecule step dependency checking was broken because bd list doesn't populate the DependsOn field (it's always empty). Only bd show returns dependency info in the Dependencies field. This caused all open steps to appear "ready" regardless of actual dependencies - the polecat would start blocked steps prematurely. Fix: Call ShowMultiple() after List() to fetch full issue details including Dependencies, then check Dependencies instead of DependsOn. Affected functions: - findNextReadyStep() in molecule_step.go - getMoleculeProgressInfo() in molecule_status.go - runMoleculeCurrent() in molecule_status.go Tests: - Added TestFindNextReadyStepWithBdListBehavior to verify fix - Added TestOldBuggyBehavior to demonstrate the bug - Updated existing tests to use fixed algorithm Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(molecule): use Dependencies from bd show instead of empty DependsOn Bug: Molecule step dependency checking was broken because bd list doesn't populate the DependsOn field (it's always empty). Only bd show returns dependency info in the Dependencies field. This caused all open steps to appear "ready" regardless of actual dependencies - the polecat would start blocked steps prematurely. Fix: Call ShowMultiple() after List() to fetch full issue details including Dependencies, then check Dependencies instead of DependsOn. Also filter to only check "blocks" type dependencies - ignore "parent-child" relationships which are just structural, not blocking. Affected functions: - findNextReadyStep() in molecule_step.go - getMoleculeProgressInfo() in molecule_status.go - runMoleculeCurrent() in molecule_status.go Tests: - Added TestFindNextReadyStepWithBdListBehavior to verify fix - Added TestOldBuggyBehavior to demonstrate the bug - Updated existing tests to use fixed algorithm Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: julianknutsen <julianknutsen@users.noreply.github> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
720 lines
22 KiB
Go
720 lines
22 KiB
Go
package cmd
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/steveyegge/gastown/internal/beads"
|
|
)
|
|
|
|
func TestExtractMoleculeIDFromStep(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
stepID string
|
|
expected string
|
|
}{
|
|
{
|
|
name: "simple step",
|
|
stepID: "gt-abc.1",
|
|
expected: "gt-abc",
|
|
},
|
|
{
|
|
name: "multi-digit step number",
|
|
stepID: "gt-xyz.12",
|
|
expected: "gt-xyz",
|
|
},
|
|
{
|
|
name: "molecule with dash",
|
|
stepID: "gt-my-mol.3",
|
|
expected: "gt-my-mol",
|
|
},
|
|
{
|
|
name: "bd prefix",
|
|
stepID: "bd-mol-abc.2",
|
|
expected: "bd-mol-abc",
|
|
},
|
|
{
|
|
name: "complex id",
|
|
stepID: "gt-some-complex-id.99",
|
|
expected: "gt-some-complex-id",
|
|
},
|
|
{
|
|
name: "not a step - no suffix",
|
|
stepID: "gt-5gq8r",
|
|
expected: "",
|
|
},
|
|
{
|
|
name: "not a step - non-numeric suffix",
|
|
stepID: "gt-abc.xyz",
|
|
expected: "",
|
|
},
|
|
{
|
|
name: "not a step - mixed suffix",
|
|
stepID: "gt-abc.1a",
|
|
expected: "",
|
|
},
|
|
{
|
|
name: "empty string",
|
|
stepID: "",
|
|
expected: "",
|
|
},
|
|
{
|
|
name: "just a dot",
|
|
stepID: ".",
|
|
expected: "",
|
|
},
|
|
{
|
|
name: "trailing dot",
|
|
stepID: "gt-abc.",
|
|
expected: "",
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
result := extractMoleculeIDFromStep(tt.stepID)
|
|
if result != tt.expected {
|
|
t.Errorf("extractMoleculeIDFromStep(%q) = %q, want %q", tt.stepID, result, tt.expected)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// mockBeadsForStep extends mockBeads with parent filtering for step tests.
|
|
// It simulates the real bd behavior where:
|
|
// - List() returns issues with DependsOn empty (bd list doesn't return deps)
|
|
// - Show()/ShowMultiple() returns issues with Dependencies populated (bd show does)
|
|
type mockBeadsForStep struct {
|
|
issues map[string]*beads.Issue
|
|
}
|
|
|
|
func newMockBeadsForStep() *mockBeadsForStep {
|
|
return &mockBeadsForStep{
|
|
issues: make(map[string]*beads.Issue),
|
|
}
|
|
}
|
|
|
|
func (m *mockBeadsForStep) addIssue(issue *beads.Issue) {
|
|
m.issues[issue.ID] = issue
|
|
}
|
|
|
|
func (m *mockBeadsForStep) Show(id string) (*beads.Issue, error) {
|
|
if issue, ok := m.issues[id]; ok {
|
|
return issue, nil
|
|
}
|
|
return nil, beads.ErrNotFound
|
|
}
|
|
|
|
// ShowMultiple simulates bd show with multiple IDs - returns full issue data including Dependencies
|
|
func (m *mockBeadsForStep) ShowMultiple(ids []string) (map[string]*beads.Issue, error) {
|
|
result := make(map[string]*beads.Issue)
|
|
for _, id := range ids {
|
|
if issue, ok := m.issues[id]; ok {
|
|
result[id] = issue
|
|
}
|
|
}
|
|
return result, nil
|
|
}
|
|
|
|
// List simulates bd list behavior - returns issues but with DependsOn EMPTY.
|
|
// This is the key behavior that caused the bug: bd list doesn't return dependency info.
|
|
func (m *mockBeadsForStep) List(opts beads.ListOptions) ([]*beads.Issue, error) {
|
|
var result []*beads.Issue
|
|
for _, issue := range m.issues {
|
|
// Filter by parent
|
|
if opts.Parent != "" && issue.Parent != opts.Parent {
|
|
continue
|
|
}
|
|
// Filter by status (unless "all")
|
|
if opts.Status != "" && opts.Status != "all" && issue.Status != opts.Status {
|
|
continue
|
|
}
|
|
// CRITICAL: Simulate bd list behavior - DependsOn is NOT populated
|
|
// Create a copy with empty DependsOn to simulate real bd list output
|
|
issueCopy := *issue
|
|
issueCopy.DependsOn = nil // bd list doesn't return this
|
|
result = append(result, &issueCopy)
|
|
}
|
|
return result, nil
|
|
}
|
|
|
|
func (m *mockBeadsForStep) Close(ids ...string) error {
|
|
for _, id := range ids {
|
|
if issue, ok := m.issues[id]; ok {
|
|
issue.Status = "closed"
|
|
} else {
|
|
return beads.ErrNotFound
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// makeStepIssue creates a test step issue with both DependsOn and Dependencies set.
|
|
// In real usage:
|
|
// - bd list returns issues with DependsOn empty
|
|
// - bd show returns issues with Dependencies populated (with DependencyType)
|
|
// The mock simulates this: List() clears DependsOn, Show() returns the full issue.
|
|
func makeStepIssue(id, title, parent, status string, dependsOn []string) *beads.Issue {
|
|
issue := &beads.Issue{
|
|
ID: id,
|
|
Title: title,
|
|
Type: "task",
|
|
Status: status,
|
|
Priority: 2,
|
|
Parent: parent,
|
|
DependsOn: dependsOn, // This gets cleared by mock List() to simulate bd list
|
|
CreatedAt: "2025-01-01T12:00:00Z",
|
|
UpdatedAt: "2025-01-01T12:00:00Z",
|
|
}
|
|
// Also set Dependencies (what bd show returns) for proper testing.
|
|
// Use "blocks" dependency type since that's what formula instantiation creates
|
|
// for inter-step dependencies (vs "parent-child" for parent relationships).
|
|
for _, depID := range dependsOn {
|
|
issue.Dependencies = append(issue.Dependencies, beads.IssueDep{
|
|
ID: depID,
|
|
Title: "Dependency " + depID,
|
|
DependencyType: "blocks", // Only "blocks" deps should block progress
|
|
})
|
|
}
|
|
return issue
|
|
}
|
|
|
|
func TestFindNextReadyStep(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
moleculeID string
|
|
setupFunc func(*mockBeadsForStep)
|
|
wantStepID string
|
|
wantComplete bool
|
|
wantNilStep bool
|
|
}{
|
|
{
|
|
name: "no steps - molecule complete",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
// Empty molecule - no children
|
|
},
|
|
wantComplete: true,
|
|
wantNilStep: true,
|
|
},
|
|
{
|
|
name: "all steps closed - molecule complete",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "closed", []string{"gt-mol.1"}))
|
|
},
|
|
wantComplete: true,
|
|
wantNilStep: true,
|
|
},
|
|
{
|
|
name: "first step ready - no dependencies",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantStepID: "gt-mol.1",
|
|
wantComplete: false,
|
|
},
|
|
{
|
|
name: "second step ready - first closed",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantStepID: "gt-mol.2",
|
|
wantComplete: false,
|
|
},
|
|
{
|
|
name: "all blocked - waiting on dependencies",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "in_progress", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Step 3", "gt-mol", "open", []string{"gt-mol.2"}))
|
|
},
|
|
wantComplete: false,
|
|
wantNilStep: true, // No ready steps (all blocked or in-progress)
|
|
},
|
|
{
|
|
name: "parallel steps - multiple ready",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
// Both step 1 and 2 have no deps, so both are ready
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Synthesis", "gt-mol", "open", []string{"gt-mol.1", "gt-mol.2"}))
|
|
},
|
|
wantComplete: false,
|
|
// Should return one of the ready steps (implementation returns first found)
|
|
},
|
|
{
|
|
name: "diamond dependency - synthesis blocked",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step A", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step B", "gt-mol", "open", nil)) // still open
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Synthesis", "gt-mol", "open", []string{"gt-mol.1", "gt-mol.2"}))
|
|
},
|
|
wantStepID: "gt-mol.2", // B is ready (no deps)
|
|
wantComplete: false,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
m := newMockBeadsForStep()
|
|
tt.setupFunc(m)
|
|
|
|
// Test the FIXED algorithm that uses ShowMultiple for dependency info
|
|
// (simulating the real findNextReadyStep behavior after the fix)
|
|
|
|
// Get children from mock (DependsOn will be empty - simulating bd list)
|
|
children, _ := m.List(beads.ListOptions{Parent: tt.moleculeID, Status: "all"})
|
|
|
|
// Build closed IDs set and collect open step IDs
|
|
closedIDs := make(map[string]bool)
|
|
var openStepIDs []string
|
|
hasNonClosedSteps := false
|
|
for _, child := range children {
|
|
switch child.Status {
|
|
case "closed":
|
|
closedIDs[child.ID] = true
|
|
case "open":
|
|
openStepIDs = append(openStepIDs, child.ID)
|
|
hasNonClosedSteps = true
|
|
default:
|
|
// in_progress or other - not closed, not available
|
|
hasNonClosedSteps = true
|
|
}
|
|
}
|
|
|
|
// Check complete
|
|
allComplete := !hasNonClosedSteps
|
|
|
|
if allComplete != tt.wantComplete {
|
|
t.Errorf("allComplete = %v, want %v", allComplete, tt.wantComplete)
|
|
}
|
|
|
|
if tt.wantComplete {
|
|
return
|
|
}
|
|
|
|
// Fetch full details for open steps (Dependencies will be populated)
|
|
openStepsMap, _ := m.ShowMultiple(openStepIDs)
|
|
|
|
// Find ready step using Dependencies (not DependsOn!)
|
|
// Only "blocks" type dependencies block progress - ignore "parent-child".
|
|
var readyStep *beads.Issue
|
|
for _, stepID := range openStepIDs {
|
|
step := openStepsMap[stepID]
|
|
if step == nil {
|
|
continue
|
|
}
|
|
|
|
// Use Dependencies (from bd show), NOT DependsOn (empty from bd list)
|
|
allDepsClosed := true
|
|
hasBlockingDeps := false
|
|
for _, dep := range step.Dependencies {
|
|
if dep.DependencyType != "blocks" {
|
|
continue // Skip parent-child and other non-blocking relationships
|
|
}
|
|
hasBlockingDeps = true
|
|
if !closedIDs[dep.ID] {
|
|
allDepsClosed = false
|
|
break
|
|
}
|
|
}
|
|
if !hasBlockingDeps || allDepsClosed {
|
|
readyStep = step
|
|
break
|
|
}
|
|
}
|
|
|
|
if tt.wantNilStep {
|
|
if readyStep != nil {
|
|
t.Errorf("expected nil step, got %s", readyStep.ID)
|
|
}
|
|
return
|
|
}
|
|
|
|
if readyStep == nil {
|
|
if tt.wantStepID != "" {
|
|
t.Errorf("expected step %s, got nil", tt.wantStepID)
|
|
}
|
|
return
|
|
}
|
|
|
|
if tt.wantStepID != "" && readyStep.ID != tt.wantStepID {
|
|
t.Errorf("readyStep.ID = %s, want %s", readyStep.ID, tt.wantStepID)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestStepDoneScenarios tests complete step-done scenarios
|
|
func TestStepDoneScenarios(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
stepID string
|
|
setupFunc func(*mockBeadsForStep)
|
|
wantAction string // "continue", "done", "no_more_ready"
|
|
wantNextStep string
|
|
}{
|
|
{
|
|
name: "complete step, continue to next",
|
|
stepID: "gt-mol.1",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantAction: "continue",
|
|
wantNextStep: "gt-mol.2",
|
|
},
|
|
{
|
|
name: "complete final step, molecule done",
|
|
stepID: "gt-mol.2",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantAction: "done",
|
|
},
|
|
{
|
|
name: "complete step, remaining blocked",
|
|
stepID: "gt-mol.1",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "in_progress", nil)) // another parallel task
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Synthesis", "gt-mol", "open", []string{"gt-mol.1", "gt-mol.2"}))
|
|
},
|
|
wantAction: "no_more_ready", // .2 is in_progress, .3 blocked
|
|
},
|
|
{
|
|
name: "parallel workflow - complete one, next ready",
|
|
stepID: "gt-mol.1",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Parallel A", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Parallel B", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Synthesis", "gt-mol", "open", []string{"gt-mol.1", "gt-mol.2"}))
|
|
},
|
|
wantAction: "continue",
|
|
wantNextStep: "gt-mol.2", // B is still ready
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
m := newMockBeadsForStep()
|
|
tt.setupFunc(m)
|
|
|
|
// Extract molecule ID
|
|
moleculeID := extractMoleculeIDFromStep(tt.stepID)
|
|
if moleculeID == "" {
|
|
t.Fatalf("could not extract molecule ID from %s", tt.stepID)
|
|
}
|
|
|
|
// Simulate closing the step
|
|
if err := m.Close(tt.stepID); err != nil {
|
|
t.Fatalf("failed to close step: %v", err)
|
|
}
|
|
|
|
// Now find next ready step using the FIXED algorithm
|
|
children, _ := m.List(beads.ListOptions{Parent: moleculeID, Status: "all"})
|
|
|
|
closedIDs := make(map[string]bool)
|
|
var openStepIDs []string
|
|
hasNonClosedSteps := false
|
|
for _, child := range children {
|
|
switch child.Status {
|
|
case "closed":
|
|
closedIDs[child.ID] = true
|
|
case "open":
|
|
openStepIDs = append(openStepIDs, child.ID)
|
|
hasNonClosedSteps = true
|
|
default:
|
|
// in_progress or other - not closed, not available
|
|
hasNonClosedSteps = true
|
|
}
|
|
}
|
|
|
|
allComplete := !hasNonClosedSteps
|
|
|
|
var action string
|
|
var nextStepID string
|
|
|
|
if allComplete {
|
|
action = "done"
|
|
} else {
|
|
// Fetch full details for open steps (Dependencies will be populated)
|
|
openStepsMap, _ := m.ShowMultiple(openStepIDs)
|
|
|
|
// Find ready step using Dependencies (not DependsOn!)
|
|
// Only "blocks" type dependencies block progress - ignore "parent-child".
|
|
var readyStep *beads.Issue
|
|
for _, stepID := range openStepIDs {
|
|
step := openStepsMap[stepID]
|
|
if step == nil {
|
|
continue
|
|
}
|
|
|
|
// Use Dependencies (from bd show), NOT DependsOn (empty from bd list)
|
|
allDepsClosed := true
|
|
hasBlockingDeps := false
|
|
for _, dep := range step.Dependencies {
|
|
if dep.DependencyType != "blocks" {
|
|
continue // Skip parent-child and other non-blocking relationships
|
|
}
|
|
hasBlockingDeps = true
|
|
if !closedIDs[dep.ID] {
|
|
allDepsClosed = false
|
|
break
|
|
}
|
|
}
|
|
if !hasBlockingDeps || allDepsClosed {
|
|
readyStep = step
|
|
break
|
|
}
|
|
}
|
|
|
|
if readyStep != nil {
|
|
action = "continue"
|
|
nextStepID = readyStep.ID
|
|
} else {
|
|
action = "no_more_ready"
|
|
}
|
|
}
|
|
|
|
if action != tt.wantAction {
|
|
t.Errorf("action = %s, want %s", action, tt.wantAction)
|
|
}
|
|
|
|
if tt.wantNextStep != "" && nextStepID != tt.wantNextStep {
|
|
t.Errorf("nextStep = %s, want %s", nextStepID, tt.wantNextStep)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestFindNextReadyStepWithBdListBehavior tests the fix for the bug where
|
|
// bd list doesn't return dependency info (DependsOn is always empty), but
|
|
// bd show returns Dependencies. The old code checked DependsOn (always empty),
|
|
// so all open steps looked "ready" even when blocked.
|
|
//
|
|
// This test simulates real bd behavior and verifies the fix works correctly.
|
|
func TestFindNextReadyStepWithBdListBehavior(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
moleculeID string
|
|
setupFunc func(*mockBeadsForStep)
|
|
wantStepID string // Expected ready step ID, or "" if none ready
|
|
wantComplete bool
|
|
wantBlocked bool // True if all remaining steps are blocked (none ready)
|
|
}{
|
|
{
|
|
name: "blocked step should NOT be ready - dependency not closed",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
// Step 1 is open (first step, no deps)
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "open", nil))
|
|
// Step 2 depends on Step 1, which is NOT closed
|
|
// BUG: Old code would mark Step 2 as ready because DependsOn is empty from bd list
|
|
// FIX: New code uses Dependencies from bd show
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantStepID: "gt-mol.1", // Only step 1 should be ready
|
|
wantComplete: false,
|
|
},
|
|
{
|
|
name: "step becomes ready when dependency closes",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantStepID: "gt-mol.2", // Step 2 is ready now that step 1 is closed
|
|
wantComplete: false,
|
|
},
|
|
{
|
|
name: "multiple blocked steps - none ready",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
// Step 1 is in_progress (not closed)
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "in_progress", nil))
|
|
// Steps 2 and 3 both depend on step 1
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Step 3", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
},
|
|
wantBlocked: true, // No open steps are ready (all blocked by step 1)
|
|
wantComplete: false,
|
|
},
|
|
{
|
|
name: "diamond dependency - synthesis blocked until both complete",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step A", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step B", "gt-mol", "open", nil))
|
|
// Synthesis depends on BOTH A and B
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Synthesis", "gt-mol", "open", []string{"gt-mol.1", "gt-mol.2"}))
|
|
},
|
|
wantStepID: "gt-mol.2", // B is ready (no deps), synthesis is blocked
|
|
wantComplete: false,
|
|
},
|
|
{
|
|
name: "diamond dependency - synthesis ready when both complete",
|
|
moleculeID: "gt-mol",
|
|
setupFunc: func(m *mockBeadsForStep) {
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step A", "gt-mol", "closed", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step B", "gt-mol", "closed", nil))
|
|
// Synthesis depends on BOTH A and B, both are now closed
|
|
m.addIssue(makeStepIssue("gt-mol.3", "Synthesis", "gt-mol", "open", []string{"gt-mol.1", "gt-mol.2"}))
|
|
},
|
|
wantStepID: "gt-mol.3", // Synthesis is now ready
|
|
wantComplete: false,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
m := newMockBeadsForStep()
|
|
tt.setupFunc(m)
|
|
|
|
// Simulate the FIXED algorithm that uses ShowMultiple for dependency info
|
|
// Step 1: List children (DependsOn will be empty - simulating bd list)
|
|
children, _ := m.List(beads.ListOptions{Parent: tt.moleculeID, Status: "all"})
|
|
|
|
// Build closed IDs and collect open step IDs
|
|
closedIDs := make(map[string]bool)
|
|
var openStepIDs []string
|
|
hasNonClosedSteps := false
|
|
|
|
for _, child := range children {
|
|
switch child.Status {
|
|
case "closed":
|
|
closedIDs[child.ID] = true
|
|
case "open":
|
|
openStepIDs = append(openStepIDs, child.ID)
|
|
hasNonClosedSteps = true
|
|
default:
|
|
hasNonClosedSteps = true
|
|
}
|
|
}
|
|
|
|
allComplete := !hasNonClosedSteps
|
|
if allComplete != tt.wantComplete {
|
|
t.Errorf("allComplete = %v, want %v", allComplete, tt.wantComplete)
|
|
}
|
|
|
|
if tt.wantComplete {
|
|
return
|
|
}
|
|
|
|
// Step 2: Fetch full details for open steps (Dependencies will be populated)
|
|
openStepsMap, _ := m.ShowMultiple(openStepIDs)
|
|
|
|
// Step 3: Find ready step using Dependencies (not DependsOn!)
|
|
// Only "blocks" type dependencies block progress - ignore "parent-child".
|
|
var readyStep *beads.Issue
|
|
for _, stepID := range openStepIDs {
|
|
step := openStepsMap[stepID]
|
|
if step == nil {
|
|
continue
|
|
}
|
|
|
|
// Use Dependencies (from bd show), NOT DependsOn (empty from bd list)
|
|
allDepsClosed := true
|
|
hasBlockingDeps := false
|
|
for _, dep := range step.Dependencies {
|
|
if dep.DependencyType != "blocks" {
|
|
continue // Skip parent-child and other non-blocking relationships
|
|
}
|
|
hasBlockingDeps = true
|
|
if !closedIDs[dep.ID] {
|
|
allDepsClosed = false
|
|
break
|
|
}
|
|
}
|
|
|
|
if !hasBlockingDeps || allDepsClosed {
|
|
readyStep = step
|
|
break
|
|
}
|
|
}
|
|
|
|
// Verify results
|
|
if tt.wantBlocked {
|
|
if readyStep != nil {
|
|
t.Errorf("expected no ready steps (all blocked), got %s", readyStep.ID)
|
|
}
|
|
return
|
|
}
|
|
|
|
if tt.wantStepID == "" {
|
|
if readyStep != nil {
|
|
t.Errorf("expected no ready step, got %s", readyStep.ID)
|
|
}
|
|
return
|
|
}
|
|
|
|
if readyStep == nil {
|
|
t.Errorf("expected ready step %s, got nil", tt.wantStepID)
|
|
return
|
|
}
|
|
|
|
if readyStep.ID != tt.wantStepID {
|
|
t.Errorf("ready step = %s, want %s", readyStep.ID, tt.wantStepID)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestOldBuggyBehavior demonstrates what the old buggy code would have done.
|
|
// With the old code, since DependsOn was always empty from bd list,
|
|
// ALL open steps would appear "ready" regardless of actual dependencies.
|
|
// This test verifies the bug exists when using the old approach.
|
|
func TestOldBuggyBehavior(t *testing.T) {
|
|
m := newMockBeadsForStep()
|
|
|
|
// Setup: Step 2 depends on Step 1, but Step 1 is NOT closed
|
|
m.addIssue(makeStepIssue("gt-mol.1", "Step 1", "gt-mol", "open", nil))
|
|
m.addIssue(makeStepIssue("gt-mol.2", "Step 2", "gt-mol", "open", []string{"gt-mol.1"}))
|
|
|
|
// Get children via List (simulates bd list - DependsOn is empty)
|
|
children, _ := m.List(beads.ListOptions{Parent: "gt-mol", Status: "all"})
|
|
|
|
// OLD BUGGY CODE: Check DependsOn (which is empty from bd list)
|
|
closedIDs := make(map[string]bool)
|
|
var openSteps []*beads.Issue
|
|
for _, child := range children {
|
|
if child.Status == "closed" {
|
|
closedIDs[child.ID] = true
|
|
} else if child.Status == "open" {
|
|
openSteps = append(openSteps, child)
|
|
}
|
|
}
|
|
|
|
// Count how many steps the OLD buggy code thinks are "ready"
|
|
readyCount := 0
|
|
for _, step := range openSteps {
|
|
allDepsClosed := true
|
|
for _, depID := range step.DependsOn { // BUG: This is always empty!
|
|
if !closedIDs[depID] {
|
|
allDepsClosed = false
|
|
break
|
|
}
|
|
}
|
|
if len(step.DependsOn) == 0 || allDepsClosed { // Always true since DependsOn is empty
|
|
readyCount++
|
|
}
|
|
}
|
|
|
|
// The bug: OLD code thinks BOTH steps are ready (2 ready)
|
|
// Correct behavior: Only Step 1 should be ready (1 ready)
|
|
if readyCount != 2 {
|
|
t.Errorf("Expected old buggy code to mark 2 steps as ready, got %d", readyCount)
|
|
}
|
|
|
|
t.Log("Old buggy behavior confirmed: both steps marked ready when only step 1 should be")
|
|
}
|