fix(runners): preserve terminal status when Run returns after emergency stop#3972
Draft
cursor[bot] wants to merge 1 commit into
Draft
fix(runners): preserve terminal status when Run returns after emergency stop#3972cursor[bot] wants to merge 1 commit into
cursor[bot] wants to merge 1 commit into
Conversation
…cy stop Commit 2a156b3 moved the IsFinished() early return into the success-only branch of the post-Run handler. When applyTerminatedJobs kills a job and sets stopped while Run() is still unwinding, the error path overwrote the correct terminal status with failed. Restore the finished-status guard for both branches via finalizeAfterRun. Co-authored-by: Denis Gukov <fiftin@outlook.com>
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.
Bug and impact
When a remote runner job is emergency-stopped via
terminated_jobs(force stop, reassignment, or server-side terminal status),applyTerminatedJobskills the process and sets the job tostopped. Ifjob.Run()then returns a non-nil error—as happens when the process is killed—the post-run handler could overwrite the correct terminal status withfailed.This can cause incorrect task status to be reported on the next progress sync, leading to user-facing status corruption (stopped/success tasks shown as failed) and incorrect downstream behavior (alerts, workflow progression).
Root cause
Commit
2a156b30(fix(runners): logs) moved theIsFinished()early return into the success-only branch of the post-Run()handler. The error path no longer respected an already-terminal status set byapplyTerminatedJobs.Fix
finalizeAfterRunonrunningJobwith anIsFinished()guard that applies to both error and success paths.Validation
go test ./services/runners/...