Skip to content

Commit 94f3302

Browse files
committed
fix(state): cascade parallel completion to enclosing compound's OnDone
When a parallel state completed inside a compound, settleParallelDone's upward-cascade arm called settleDone(parallel, ...), but settleDone gates on n.state.IsFinal and a parallel state is never IsFinal — so the call was a dead no-op. The enclosing compound's OnDone was silently dropped even though the compound was complete, an asymmetry with a nested compound reaching final (which did propagate). Route the cascade through completion semantics instead: a new settleEnclosingDone helper walks the ancestor spine from the parallel, running each enclosing compound's OnDone while it is stateComplete, innermost-first — the upward counterpart of settleInteriorDone, gating on stateComplete exactly as it does. The parallel's own OnDone still fires exactly once and is not re-emitted. Un-skips the regression pin TestParallel_NestedUnderCompound_SettlesParentDone and adds cases for two-compounds-deep recursion and the completion gate (no cascade when an enclosing ancestor is incomplete). settleParallelDone coverage rises from 68.8% to 86.7%. IR goldens are unaffected (they pin static machine structure, not runtime effect streams). Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
1 parent 1f76d07 commit 94f3302

2 files changed

Lines changed: 169 additions & 21 deletions

File tree

state/parallel.go

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -534,17 +534,56 @@ func (i *Instance[S, E, C]) settleParallelDone(parallel S, entity C, tr *Trace)
534534
if aerr != nil {
535535
return eff, aname, aerr
536536
}
537-
// Continue settling upward if the parallel state itself completes a parent.
538-
if pn.hasParent {
539-
up, uname, uerr := i.settleDone(parallel, entity, tr)
540-
eff = append(eff, up...)
541-
if uerr != nil {
542-
return eff, uname, uerr
543-
}
537+
// Continue settling upward through the enclosing spine. A parallel state is
538+
// never IsFinal, so settleDone (which gates on IsFinal) is a no-op here;
539+
// instead settle each enclosing compound through completion semantics, exactly
540+
// as settleInteriorDone does for compounds interior to a region. Walking from
541+
// the parallel up its ancestors, every enclosing compound that is now
542+
// stateComplete records a done microstep and runs its OnDone, innermost-first,
543+
// until an incomplete ancestor halts the cascade. The parallel's OWN OnDone
544+
// already ran above and is not re-emitted here.
545+
up, uname, uerr := i.settleEnclosingDone(parallel, entity, tr)
546+
eff = append(eff, up...)
547+
if uerr != nil {
548+
return eff, uname, uerr
544549
}
545550
return eff, "", nil
546551
}
547552

553+
// settleEnclosingDone settles the done/OnDone of every compound that ENCLOSES a
554+
// completed state, cascading innermost-first up the ancestor spine while each
555+
// enclosing ancestor is stateComplete. It is the upward counterpart shared by
556+
// settleParallelDone: a parallel state is never IsFinal, so settleDone cannot
557+
// carry the cascade past it; this routes through stateComplete instead, the same
558+
// completion gate settleInteriorDone uses. The starting state's own OnDone is the
559+
// caller's responsibility and is never run here.
560+
func (i *Instance[S, E, C]) settleEnclosingDone(start S, entity C, tr *Trace) (effects []Effect, name string, err error) {
561+
m := i.machine
562+
563+
cur := start
564+
for {
565+
cn, ok := m.resolveNode(cur)
566+
if !ok || !cn.hasParent {
567+
return effects, "", nil
568+
}
569+
parent := cn.parent
570+
pn, ok := m.resolveNode(parent)
571+
if !ok {
572+
return effects, "", nil
573+
}
574+
tr.note("done." + m.label(cur))
575+
if !i.stateComplete(parent) {
576+
return effects, "", nil
577+
}
578+
eff, aname, aerr := i.runActions(pn.state.OnDone, entity, tr)
579+
effects = append(effects, eff...)
580+
if aerr != nil {
581+
return effects, aname, aerr
582+
}
583+
cur = parent
584+
}
585+
}
586+
548587
// fireFromState resolves the event from an explicit state up through its
549588
// ancestors (used when no region handled the event in a parallel state). eventData
550589
// is the already-resolved payload the matched transition's Assign reads from

state/parallel_done_cascade_test.go

Lines changed: 123 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,21 +85,16 @@ func TestParallel_OnDoneActionError_SurfacesTaggedFailure(t *testing.T) {
8585
// the effect stream must contain BOTH, in inner-then-outer order, each exactly
8686
// once.
8787
//
88-
// FREEZE BLOCKER (BUG): this test is SKIPPED because it currently FAILS — it has
89-
// surfaced a real soundness bug in settleParallelDone's upward cascade. When a
90-
// parallel completes inside a compound, outer.OnDone is silently dropped (0 times,
91-
// want 1), even though stateComplete(outer) is true. The cascade arm calls
92-
// i.settleDone(parallel, ...), but settleDone guards on n.state.IsFinal and a
93-
// PARALLEL state is never IsFinal, so the call is a no-op. The sibling helper
94-
// settleInteriorDone correctly gates on stateComplete(parent) instead. A nested
95-
// *compound* reaching final DOES propagate (verified manually), so this is an
96-
// asymmetry, not intended behavior. The PIN is left in place (Skip, not deleted)
97-
// so it activates the moment the bug is fixed; it must NOT be deleted or weakened
98-
// to the buggy 0-count. See the agent return for full reproduction.
88+
// REGRESSION PIN (formerly a freeze blocker): settleParallelDone's upward cascade
89+
// once dropped the enclosing compound's OnDone because it routed through
90+
// settleDone, which gates on n.state.IsFinal — and a PARALLEL state is never
91+
// IsFinal, so the call was a dead no-op while stateComplete(outer) was true. The
92+
// fix routes the cascade through completion semantics (settleEnclosingDone, the
93+
// upward counterpart of settleInteriorDone, gating on stateComplete), so a nested
94+
// parallel now propagates done to its enclosing compound exactly as a nested
95+
// compound already did. This pin locks that emission and its inner-then-outer
96+
// ordering; it must NOT be weakened back to the buggy 0-count.
9997
func TestParallel_NestedUnderCompound_SettlesParentDone(t *testing.T) {
100-
t.Skip("FREEZE BLOCKER: settleParallelDone upward cascade drops the enclosing compound's OnDone " +
101-
"(settleDone gates on IsFinal, parallels are never IsFinal). Un-skip once fixed.")
102-
10398
note := func(s string) state.ActionFn[prCtx] {
10499
return func(state.ActionCtx[prCtx]) (state.Effect, error) { return s, nil }
105100
}
@@ -162,3 +157,117 @@ func TestParallel_NestedUnderCompound_SettlesParentDone(t *testing.T) {
162157
t.Fatalf("effect order = %v, want Pdone before Odone (inner done settles before the enclosing compound's done)", got)
163158
}
164159
}
160+
161+
// TestParallel_NestedTwoCompoundsDeep_CascadesAllDone pins the RECURSIVE arm of
162+
// the upward cascade: a parallel "par" nested inside compound "mid" nested inside
163+
// compound "outer". When "par" completes in one macrostep, the done settlement
164+
// must walk the whole enclosing spine innermost-first — par.OnDone, then
165+
// mid.OnDone, then outer.OnDone — because each enclosing ancestor in turn becomes
166+
// stateComplete. Each fires exactly once, in spine order.
167+
func TestParallel_NestedTwoCompoundsDeep_CascadesAllDone(t *testing.T) {
168+
note := func(s string) state.ActionFn[prCtx] {
169+
return func(state.ActionCtx[prCtx]) (state.Effect, error) { return s, nil }
170+
}
171+
m := state.Forge[string, string, prCtx]("par-nested-2deep").
172+
Action("Pdone", note("Pdone")).
173+
Action("Mdone", note("Mdone")).
174+
Action("Odone", note("Odone")).
175+
State("off").
176+
Transition("off").On("go").GoTo("par").
177+
SuperState("outer").OnDone("Odone").
178+
Initial("mid").
179+
SuperState("mid").OnDone("Mdone").
180+
Initial("par").
181+
SuperState("par").OnDone("Pdone").
182+
Region("a").Initial("a1").SubState("a1").SubState("af").Final().EndRegion().
183+
Region("b").Initial("b1").SubState("b1").SubState("bf").Final().EndRegion().
184+
EndSuperState(). // close par
185+
EndSuperState(). // close mid
186+
EndSuperState(). // close outer
187+
Initial("off").
188+
CurrentStateFn(func(prCtx) string { return "off" }).
189+
Transition("a1").On("e").GoTo("af").
190+
Transition("b1").On("e").GoTo("bf").
191+
Quench()
192+
193+
inst := m.Cast(prCtx{}, state.WithInitialState("off"))
194+
ctx := context.Background()
195+
if res := inst.Fire(ctx, "go"); res.Err != nil {
196+
t.Fatalf("entering nested parallel: %v", res.Err)
197+
}
198+
199+
res := inst.Fire(ctx, "e")
200+
if res.Err != nil {
201+
t.Fatalf("e errored: %v (config=%v)", res.Err, inst.Configuration())
202+
}
203+
204+
for _, want := range []string{"Pdone", "Mdone", "Odone"} {
205+
if got := countEffect(res.Effects, want); got != 1 {
206+
t.Fatalf("%s fired %d times, want exactly 1 (effects=%v)", want, got, stringEffects(res.Effects))
207+
}
208+
}
209+
210+
// Innermost-first spine order: Pdone < Mdone < Odone.
211+
idx := map[string]int{"Pdone": -1, "Mdone": -1, "Odone": -1}
212+
for i, e := range stringEffects(res.Effects) {
213+
if _, ok := idx[e]; ok {
214+
idx[e] = i
215+
}
216+
}
217+
if idx["Pdone"] >= idx["Mdone"] || idx["Mdone"] >= idx["Odone"] {
218+
t.Fatalf("effect order = %v, want Pdone < Mdone < Odone (innermost-first up the spine)", stringEffects(res.Effects))
219+
}
220+
}
221+
222+
// TestParallel_NestedUnderIncompleteCompound_DoesNotCascade pins the cascade's
223+
// completion GATE: the enclosing compound "outer" also has a sibling child "side"
224+
// that stays active, so "outer" is NOT stateComplete when "par" completes. The
225+
// parallel's own OnDone must still fire, but the enclosing compound's OnDone must
226+
// NOT — the cascade halts at the first incomplete ancestor. This guards against a
227+
// fix that unconditionally walks the spine instead of gating on stateComplete.
228+
func TestParallel_NestedUnderIncompleteCompound_DoesNotCascade(t *testing.T) {
229+
note := func(s string) state.ActionFn[prCtx] {
230+
return func(state.ActionCtx[prCtx]) (state.Effect, error) { return s, nil }
231+
}
232+
m := state.Forge[string, string, prCtx]("par-nested-incomplete").
233+
Action("Pdone", note("Pdone")).
234+
Action("Odone", note("Odone")).
235+
State("off").
236+
Transition("off").On("go").GoTo("par").
237+
// "outer" is a parallel state with two regions: one holding "par", one
238+
// holding a non-final "side". When "par" completes, outer's other region
239+
// is still active and non-final, so outer is not stateComplete.
240+
SuperState("outer").OnDone("Odone").
241+
Region("main").Initial("par").
242+
SuperState("par").OnDone("Pdone").
243+
Region("a").Initial("a1").SubState("a1").SubState("af").Final().EndRegion().
244+
Region("b").Initial("b1").SubState("b1").SubState("bf").Final().EndRegion().
245+
EndSuperState(). // close par
246+
EndRegion(). // close main region
247+
Region("side").Initial("s1").SubState("s1").EndRegion().
248+
EndSuperState(). // close outer
249+
Initial("off").
250+
CurrentStateFn(func(prCtx) string { return "off" }).
251+
Transition("a1").On("e").GoTo("af").
252+
Transition("b1").On("e").GoTo("bf").
253+
Quench()
254+
255+
inst := m.Cast(prCtx{}, state.WithInitialState("off"))
256+
ctx := context.Background()
257+
if res := inst.Fire(ctx, "go"); res.Err != nil {
258+
t.Fatalf("entering nested parallel: %v", res.Err)
259+
}
260+
261+
res := inst.Fire(ctx, "e")
262+
if res.Err != nil {
263+
t.Fatalf("e errored: %v (config=%v)", res.Err, inst.Configuration())
264+
}
265+
266+
if got := countEffect(res.Effects, "Pdone"); got != 1 {
267+
t.Fatalf("par.OnDone fired %d times, want exactly 1 (effects=%v)", got, stringEffects(res.Effects))
268+
}
269+
if got := countEffect(res.Effects, "Odone"); got != 0 {
270+
t.Fatalf("outer.OnDone fired %d times, want 0 — outer is not complete (its 'side' region is still active) (effects=%v)",
271+
got, stringEffects(res.Effects))
272+
}
273+
}

0 commit comments

Comments
 (0)