fix: make direct workflow retry atomic - #3086
Merged
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 #3085.
Root cause
RetryJobAbilityhandled a processing direct job with no live action by terminalizing and reopening it before calling the general enqueuer. That bypassed the newer atomic missing-action recovery path. Separately,DirectJobEnqueuerand the transactional recovery primitive treated any positiveas_schedule_single_action()return as success without confirming that the action row was durably queryable.The retry path itself reuses the original job ID. The two fresh production IDs were downstream direct submissions emitted after the retried workflow resumed, not replacement rows created by
jobs retry; each could be left pathless because positive scheduler IDs were accepted without durable receipt confirmation. The command then reported success based on the original enqueue result before scheduler ownership was proven end to end.Atomicity and exactly-once behavior
For a processing direct operation whose recorded action is missing and whose effects have not begun, manual retry now calls
commit_missing_direct_operation_requeue(). That method locks the original row, schedules a unique next-generation action inside the same database transaction, verifies the action receipt by primary key, updates the fenced operation owner, and commits. Scheduler failure, an unusable receipt, ownership drift, or commit failure rolls back without creating or reopening another processing job.The generation/token fence prevents concurrent retries from scheduling duplicate owner generations. If
operation_effects_begun_atis present, retry fails instead of replaying potentially completed side effects. Existing failed-job retry behavior remains unchanged.Authorization and compatibility
Authorization and ownership checks remain in
RetryJobAbilitybefore any mutation. Anonymous CLI access remains denied. The Action Scheduler contract is tightened only for success reporting: callers now receive a retryable failure when a returned action ID has no durable receipt. The optional receipt callback added toDirectJobEnqueuerpreserves existing constructor call compatibility.Verification
php tests/direct-job-generation-smoke.phppassed: 15 assertionsvendor/bin/phpcspassedhomeboy review lint data-machine --changed-only --summary --placement=localpassed with zero findings, runccbc845a-b5e8-42cd-9781-63f1204dc161homeboy review --summary --placement=local --changed-since=origin/main data-machine: audit passed with no introduced findings; lint passed with zero findings; test provisioning failed before executing tests because the managedwordpress-databaseMySQL 8.4 Docker service could not become ready, runbe54de2f-6e0e-4f22-8eb7-4e8070babb99RuntimeServiceProvisionError: Managed runtime service failed: wordpress-databasegit diff --check origin/main...HEADpassedExisting historical jobs
After this ships, reconcile
649593,649599, and649600one at a time. First rerun liveness and confirm the recorded action is still missing, no current-generation scheduler path exists, andoperation_effects_begun=false. Then run the authorized retry for only that job and immediately verify that the same job ID is pending with a new generation and a durable scheduler action before proceeding to the next. If effects have begun, do not retry; use stuck-job recovery to terminalize and account for it instead. A failed retry is now safe to inspect because it cannot leave a fresh processing orphan.