Add terminal signal dependency states (UNMATCHED, FAILED) behind a flag - #1
Closed
derek-miller wants to merge 1 commit into
Closed
Add terminal signal dependency states (UNMATCHED, FAILED) behind a flag#1derek-miller wants to merge 1 commit into
derek-miller wants to merge 1 commit into
Conversation
Signal dependencies only had MATCHED/PENDING/SKIPPED, so a dependency that never matched stayed PENDING forever once its step went terminal, and a dependency that could not be resolved had no way to surface the error. Add two terminal states to StepDependencyMatchStatus: - UNMATCHED: 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 UNMATCHED. - FAILED: resolving the dependency raised a non-retryable error, carried in the dependency details. Set via SignalDependency.markFailed(Details). The broker never fails per dependency, so core never produces FAILED; it is the mechanism for handlers that can detect a 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.
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
1 participant
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. So a dependency that never matched staysPENDINGforever once its owning step goes terminal (it reads as "might still match"), and a dependency that fails to resolve has no way to carry the error. This adds two terminal states toStepDependencyMatchStatus, gated behind a flag:UNMATCHED— the owning step reached a terminal state while the dependency was still pending. Produced by a newSignalHandler.onTermination(...)default hook (called fromMaestroTask's termination path);MaestroSignalHandlermarks still-pending dependenciesUNMATCHED.FAILED— resolving the dependency raised a non-retryable error, carried in a newSignalDependency.details(Details). Set viaSignalDependency.markFailed(Details).Backwards compatibility
Gated by
maestro.signal.terminal-states-enabled(defaultfalse). With it off, the engine emits onlyMATCHED/PENDING/SKIPPEDand behavior is unchanged;detailsis@JsonInclude(NON_EMPTY)so it never appears. Deserialization stays strict, so the intended rollout is: deploy fleet-wide with the flag off → confirm all readers/consumers understand the new values → then flip the flag.Design note I'd like a second opinion on
The broker only ever matches a dependency or leaves it pending, so core never produces
FAILED— it's the mechanism for handlers that can detect a non-retryable resolution failure (our internal table-partition/relay handler is the first user). SoFAILED+markFailed+detailsship as an extension point with no in-core producer (covered by unit tests). Open question: is that acceptable upstream, or would you preferUNMATCHED-only here?Testing
SignalDependenciesTest:markPendingAsUnmatchedflips onlyPENDING(leavesMATCHED/SKIPPED/FAILEDuntouched), no-change case, andmarkFailedwith details.MaestroSignalHandlerTest:onTerminationmarksUNMATCHEDwhen enabled, no-op when disabled.