fix: make workflow fanout admission atomic - #535
Merged
Conversation
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
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
Fixes #531.
Root cause
The Action Scheduler branch executor persisted and enqueued siblings one at a time, but its failure path only forgot branch-store rows. Actions inserted before a later enqueue failure remained pending or could already be claimed, allowing branch effects to begin even though the parent dispatch returned an admission failure.
Behavior change
Each new fanout starts with a pending admission generation stored at one deterministic per-run option. Branch callbacks carrying a token execute only after that exact generation is admitted. A partial failure first rejects the token, fencing already-claimed workers, then unschedules all exact-payload actions and cancels their known action IDs before releasing branch-store rows.
A callback claimed while admission is pending schedules one delayed copy and re-checks admission after enqueue. If compensation won the race, it removes matching delayed copies. Runtimes without delayed scheduling fail closed rather than immediately self-requeueing and amplifying the queue. Legacy actions durably queued before this field existed contain no
admission_tokenkey and continue through the original execution path.Admission cleanup no longer depends on the mutable branch index: the generation lives at a deterministic per-run option ref and
forget_run()deletes it directly, including when a consumer owns branch payload cleanup.Tests
php tests/workflow-async-branch-payload-smoke.phpphp tests/workflow-as-branch-smoke.phpcomposer testcomposer phpstangit diff --checkFocused regressions cover:
admission_tokenexecuting normallyAI assistance
Implemented and tested with OpenAI OpenCode assistance. The agent inspected the issue, current implementation, Action Scheduler cancellation APIs, existing workflow smoke harness, and review feedback; authored the code and regression tests; and ran the test/static-analysis commands listed above. Human review is required before merge.