From ba17898365091e6e67db7d0d47cc30722955b6a8 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Thu, 25 Dec 2025 17:33:37 -0800 Subject: [PATCH] Implement fanout gate: waits-for children aggregation (gt-8tmz.38) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add children-of(step-id) syntax for explicit spawner reference - Add WaitsForSpec type and ParseWaitsFor() helper - Update cook.go to create DepWaitsFor dependencies with metadata - Infer spawner from first needs entry when using all-children - Add validation tests for children-of() syntax - Add unit tests for ParseWaitsFor() This completes the Christmas Ornament aggregation pattern: - survey-workers does for-each → creates N children - aggregate waits-for children-of(survey-workers) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/cook.go | 31 ++++++++++++ internal/formula/parser_test.go | 84 +++++++++++++++++++++++++++++++++ internal/formula/types.go | 75 +++++++++++++++++++++++++---- 3 files changed, 182 insertions(+), 8 deletions(-) diff --git a/cmd/bd/cook.go b/cmd/bd/cook.go index 093f6f95..0befeb86 100644 --- a/cmd/bd/cook.go +++ b/cmd/bd/cook.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "fmt" "os" "strings" @@ -720,6 +721,36 @@ func collectDependencies(step *formula.Step, idMapping map[string]string, deps * }) } + // Process waits_for field (gt-8tmz.38) - fanout gate dependency + if step.WaitsFor != "" { + waitsForSpec := formula.ParseWaitsFor(step.WaitsFor) + if waitsForSpec != nil { + // Determine spawner ID + spawnerStepID := waitsForSpec.SpawnerID + if spawnerStepID == "" && len(step.Needs) > 0 { + // Infer spawner from first need + spawnerStepID = step.Needs[0] + } + + if spawnerStepID != "" { + if spawnerIssueID, ok := idMapping[spawnerStepID]; ok { + // Create WaitsFor dependency with metadata + meta := types.WaitsForMeta{ + Gate: waitsForSpec.Gate, + } + metaJSON, _ := json.Marshal(meta) + + *deps = append(*deps, &types.Dependency{ + IssueID: issueID, + DependsOnID: spawnerIssueID, + Type: types.DepWaitsFor, + Metadata: string(metaJSON), + }) + } + } + } + } + // Recursively handle children for _, child := range step.Children { collectDependencies(child, idMapping, deps) diff --git a/internal/formula/parser_test.go b/internal/formula/parser_test.go index 0998ab4c..4e5203cb 100644 --- a/internal/formula/parser_test.go +++ b/internal/formula/parser_test.go @@ -681,6 +681,90 @@ func TestValidate_WaitsForField(t *testing.T) { } } +// TestValidate_WaitsForChildrenOf tests the children-of(step) syntax (gt-8tmz.38) +func TestValidate_WaitsForChildrenOf(t *testing.T) { + // Valid children-of() syntax + formula := &Formula{ + Formula: "mol-children-of", + Version: 1, + Type: TypeWorkflow, + Steps: []*Step{ + {ID: "survey-workers", Title: "Survey Workers"}, + {ID: "aggregate", Title: "Aggregate", Needs: []string{"survey-workers"}, WaitsFor: "children-of(survey-workers)"}, + }, + } + + if err := formula.Validate(); err != nil { + t.Errorf("Validate failed for valid children-of(): %v", err) + } + + // Invalid: reference to unknown step + formulaBad := &Formula{ + Formula: "mol-bad-children-of", + Version: 1, + Type: TypeWorkflow, + Steps: []*Step{ + {ID: "step1", Title: "Step 1", WaitsFor: "children-of(unknown-step)"}, + }, + } + + if err := formulaBad.Validate(); err == nil { + t.Error("Validate should fail for children-of() with unknown step") + } + + // Invalid: empty step ID + formulaEmpty := &Formula{ + Formula: "mol-empty-children-of", + Version: 1, + Type: TypeWorkflow, + Steps: []*Step{ + {ID: "step1", Title: "Step 1", WaitsFor: "children-of()"}, + }, + } + + if err := formulaEmpty.Validate(); err == nil { + t.Error("Validate should fail for children-of() with empty step ID") + } +} + +// TestParseWaitsFor tests the ParseWaitsFor helper function (gt-8tmz.38) +func TestParseWaitsFor(t *testing.T) { + tests := []struct { + input string + wantGate string + wantSpawn string + wantNil bool + }{ + {"", "", "", true}, + {"all-children", "all-children", "", false}, + {"any-children", "any-children", "", false}, + {"children-of(survey)", "all-children", "survey", false}, + {"children-of(my-step)", "all-children", "my-step", false}, + {"invalid", "", "", true}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + spec := ParseWaitsFor(tt.input) + if tt.wantNil { + if spec != nil { + t.Errorf("ParseWaitsFor(%q) = %+v, want nil", tt.input, spec) + } + return + } + if spec == nil { + t.Fatalf("ParseWaitsFor(%q) = nil, want non-nil", tt.input) + } + if spec.Gate != tt.wantGate { + t.Errorf("ParseWaitsFor(%q).Gate = %q, want %q", tt.input, spec.Gate, tt.wantGate) + } + if spec.SpawnerID != tt.wantSpawn { + t.Errorf("ParseWaitsFor(%q).SpawnerID = %q, want %q", tt.input, spec.SpawnerID, tt.wantSpawn) + } + }) + } +} + // TestValidate_ChildNeedsAndWaitsFor tests needs and waits_for in child steps func TestValidate_ChildNeedsAndWaitsFor(t *testing.T) { formula := &Formula{ diff --git a/internal/formula/types.go b/internal/formula/types.go index 2c1517d8..58d5d375 100644 --- a/internal/formula/types.go +++ b/internal/formula/types.go @@ -522,11 +522,11 @@ func (f *Formula) Validate() error { errs = append(errs, fmt.Sprintf("steps[%d] (%s): needs references unknown step %q", i, step.ID, need)) } } - // Validate waits_for field (bd-j4cr) - must be a known gate type + // Validate waits_for field (bd-j4cr, gt-8tmz.38) + // Valid formats: "all-children", "any-children", "children-of(step-id)" if step.WaitsFor != "" { - validGates := map[string]bool{"all-children": true, "any-children": true} - if !validGates[step.WaitsFor] { - errs = append(errs, fmt.Sprintf("steps[%d] (%s): waits_for has invalid value %q (must be all-children or any-children)", i, step.ID, step.WaitsFor)) + if err := validateWaitsFor(step.WaitsFor, stepIDLocations); err != nil { + errs = append(errs, fmt.Sprintf("steps[%d] (%s): %s", i, step.ID, err.Error())) } } // Validate on_complete field (gt-8tmz.8) - runtime expansion @@ -603,6 +603,66 @@ func collectChildIDs(children []*Step, idLocations map[string]string, errs *[]st } } +// WaitsForSpec holds the parsed waits_for field (gt-8tmz.38). +type WaitsForSpec struct { + // Gate is the gate type: "all-children" or "any-children" + Gate string + // SpawnerID is the step ID whose children to wait for. + // Empty means infer from context (typically first step in needs). + SpawnerID string +} + +// ParseWaitsFor parses a waits_for value into its components (gt-8tmz.38). +// Returns nil if the value is empty. +func ParseWaitsFor(value string) *WaitsForSpec { + if value == "" { + return nil + } + + // Simple gate types - spawner inferred from needs + if value == "all-children" || value == "any-children" { + return &WaitsForSpec{Gate: value} + } + + // children-of(step-id) syntax + if strings.HasPrefix(value, "children-of(") && strings.HasSuffix(value, ")") { + stepID := value[len("children-of(") : len(value)-1] + return &WaitsForSpec{ + Gate: "all-children", // Default gate type + SpawnerID: stepID, + } + } + + // Invalid - return nil (validation should have caught this) + return nil +} + +// validateWaitsFor validates the waits_for field value (gt-8tmz.38). +// Valid formats: +// - "all-children": wait for all dynamically-bonded children +// - "any-children": wait for first child to complete +// - "children-of(step-id)": wait for children of a specific step +func validateWaitsFor(value string, stepIDLocations map[string]string) error { + // Simple gate types + if value == "all-children" || value == "any-children" { + return nil + } + + // children-of(step-id) syntax + if strings.HasPrefix(value, "children-of(") && strings.HasSuffix(value, ")") { + stepID := value[len("children-of(") : len(value)-1] + if stepID == "" { + return fmt.Errorf("waits_for children-of() requires a step ID") + } + if _, exists := stepIDLocations[stepID]; !exists { + return fmt.Errorf("waits_for references unknown step %q in children-of()", stepID) + } + return nil + } + + return fmt.Errorf("waits_for has invalid value %q (must be all-children, any-children, or children-of(step-id))", value) +} + // validateChildDependsOn recursively validates depends_on and needs references for children. func validateChildDependsOn(children []*Step, idLocations map[string]string, errs *[]string, prefix string) { for i, child := range children { @@ -618,11 +678,10 @@ func validateChildDependsOn(children []*Step, idLocations map[string]string, err *errs = append(*errs, fmt.Sprintf("%s (%s): needs references unknown step %q", childPrefix, child.ID, need)) } } - // Validate waits_for field (bd-j4cr) + // Validate waits_for field (bd-j4cr, gt-8tmz.38) if child.WaitsFor != "" { - validGates := map[string]bool{"all-children": true, "any-children": true} - if !validGates[child.WaitsFor] { - *errs = append(*errs, fmt.Sprintf("%s (%s): waits_for has invalid value %q (must be all-children or any-children)", childPrefix, child.ID, child.WaitsFor)) + if err := validateWaitsFor(child.WaitsFor, idLocations); err != nil { + *errs = append(*errs, fmt.Sprintf("%s (%s): %s", childPrefix, child.ID, err.Error())) } } // Validate on_complete field (gt-8tmz.8)