Add terminal signal dependency states (CANCELED, FAILED) behind a flag - #223
Merged
derek-miller merged 1 commit intoJun 26, 2026
Merged
Conversation
derek-miller
force-pushed
the
derek_miller/signal-terminal-states
branch
2 times, most recently
from
June 22, 2026 18:57
86771ab to
4e32790
Compare
derek-miller
marked this pull request as ready for review
June 24, 2026 13:36
Signal dependencies only had MATCHED/PENDING/SKIPPED, so a dependency that never matched stayed PENDING forever once its owning step went terminal, and a dependency that could not be resolved had no way to surface the error. Add two terminal states to StepDependencyMatchStatus: - CANCELED: the owning step reached a terminal state while the dependency was still pending. Set by SignalHandler.onTermination (a new default no-op hook the engine calls from MaestroTask termination); MaestroSignalHandler marks still-pending dependencies CANCELED. - FAILED: resolving the dependency raised a non-retryable error, carried in the dependency details. Set via SignalDependency.markFailed(Details). The built-in matching only finds a signal or leaves it pending, so core never produces FAILED; it is the mechanism for handlers that can detect a non-retryable resolution failure. Gated by maestro.signal.terminal-states-enabled (default false): with it off, behavior is unchanged and only MATCHED/PENDING/SKIPPED are ever emitted, so existing readers are unaffected until a fleet opts in.
derek-miller
force-pushed
the
derek_miller/signal-terminal-states
branch
from
June 24, 2026 22:44
4e32790 to
67e5743
Compare
derek-miller
changed the title
akashdw
approved these changes
Jun 25, 2026
praneethy91
approved these changes
Jun 26, 2026
praneethy91
left a comment
praneethy91
left a comment
Collaborator
There was a problem hiding this comment.
Well tested and focused change. LGTM as it is backwards compatible
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Signal dependencies only have
MATCHED/PENDING/SKIPPED. A dependency that never matched staysPENDINGforever once its owning step goes terminal, so it looks like it might still match. A dependency that fails to resolve has no way to carry the error. This PR adds two terminal states toStepDependencyMatchStatus, gated behind a flag.CANCELED: the owning step reached a terminal state while the dependency was still pending. A newSignalHandler.onTermination(...)default hook, called fromMaestroTask's termination path, lets a handler finalize pending dependencies.MaestroSignalHandlermarks themCANCELED.FAILED: resolving the dependency raised a non-retryable error, carried in a newSignalDependency.details(Details) and set viaSignalDependency.markFailed(Details). The built-in matching only finds a signal (MATCHED) or does not find one yet (PENDING), so core has no path that producesFAILEDtoday. It is there for handlers that can detect a non-retryable resolution failure, such as a handler that resolves dependencies against an external service and gets back a non-retryable error.Backwards compatibility
Gated by
maestro.signal.terminal-states-enabled(defaultfalse). With it off, the engine emits onlyMATCHED/PENDING/SKIPPEDand behavior is unchanged, so existing deployments need to do nothing.detailsis@JsonInclude(NON_EMPTY), so it never appears.Enabling the flag is optional. Since deserialization stays strict, anyone who turns it on should first make sure their readers and consumers understand the new states.
Testing
SignalDependenciesTest:markPendingAsCanceledflips onlyPENDINGand leavesMATCHED/SKIPPED/FAILEDuntouched, plus a no-change case andmarkFailedwith details.MaestroSignalHandlerTest:onTerminationmarksCANCELEDwhen enabled and is a no-op when disabled.