Skip to content

fix(tasks,runners): critical runner/task requeue and finalization bugs#4013

Closed
cursor[bot] wants to merge 2 commits into
developfrom
cursor/critical-bug-investigation-39f2
Closed

fix(tasks,runners): critical runner/task requeue and finalization bugs#4013
cursor[bot] wants to merge 2 commits into
developfrom
cursor/critical-bug-investigation-39f2

Conversation

@cursor

@cursor cursor Bot commented Jul 7, 2026

Copy link
Copy Markdown

Summary

Cherry-picks validated fixes for three critical correctness bugs still present on develop. No new behavioral commits since yesterday's inspection (only CI/test skips: 75c98980, ab623b13, 48cb4741, fbebe1a1).

Bugs fixed

1. Duplicate dispatch on ErrAllRunnersBusy

Impact: When all runners are at capacity, a task requeued via ErrAllRunnersBusy was enqueued while still in the running set; EventTypeRequeued was deferred. A concurrent queue tick could ClaimAndDequeue the same task and start a second dispatch goroutine on the same TaskRunner.

Root cause: TaskRunner.run called state.Enqueue before releasing running/active bookkeeping.

Fix: Call onTaskStop before enqueue and emit EventTypeRequeued synchronously (matching reconciler requeue paths).

2. Runner client overwrites stopped → failed on terminated_jobs

Impact: When the server returns terminated_jobs and emergency-stops a job, Run() unwinds with an error. The error path overwrote an already-terminal stopped status with failed, misreporting task outcome to the server.

Root cause: Unconditional failure status assignment in job_pool.go after Run() error.

Fix: Add finalizeAfterRun that respects an already-finished status.

3. HA pool state leak in failTaskRunnerLost

Impact: In HA mode, when the reconciler wins the finalize lock but the DB already has a terminal status (e.g. runner reported success on another node), it returned early without completing finalization. Pool/Redis state (running set, claims, autorun children) leaked.

Root cause: Early return in failTaskRunnerLost when tsk.Task.Status.IsFinished() without calling finalizeRemoteTaskLocked. Also TaskPool.finalizeRemoteTaskLocked only checked End != nil under HA.

Fix: Complete finalization when DB already finished; harden offline requeue with pre-mutation DB re-check and rollback on persist failure; release stale pool state when End is already set (non-HA too).

Validation

  • go test ./services/tasks/... ./services/runners/... — all pass
  • New tests: TestTaskRunner_ErrAllRunnersBusy_ReleasesRunningBeforeEnqueue, TestRunningJob_finalizeAfterRun_*, reconciler HA finalization tests
Open in Web View Automation 

cursoragent and others added 2 commits July 7, 2026 11:01
When every runner is at capacity, ErrAllRunnersBusy requeued tasks by
enqueueing them while they still sat in the running set, then sending
EventTypeRequeued from a defer. A periodic queue tick could
ClaimAndDequeue the task in that window and start a second dispatch on
the same TaskRunner.

Release running/active bookkeeping before enqueueing and notify the pool
synchronously, matching the reconciler requeue paths.

Co-authored-by: Denis Gukov <fiftin@outlook.com>
…finalization

Runner client: when applyTerminatedJobs emergency-stops a job and Run()
unwinds with an error, the error path overwrote stopped with failed.
Add finalizeAfterRun that respects an already-finished status.

HA reconciler: when failTaskRunnerLost wins the finalize lock but the DB
already has a terminal status, complete finalization instead of bailing
so pool/Redis state and autorun children are not leaked. Harden offline
requeue with a pre-mutation DB re-check and rollback on persist failure.

Co-authored-by: Denis Gukov <fiftin@outlook.com>
@fiftin fiftin closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants