Skip to content

fix(runners,tasks): critical runner requeue and finalization bugs#4000

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

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

Conversation

@cursor

@cursor cursor Bot commented Jul 1, 2026

Copy link
Copy Markdown

Summary

Fixes three critical correctness bugs on develop in the task runner lifecycle that can cause duplicate task execution, incorrect terminal status, and leaked pool/Redis state in HA deployments.

Bugs and impact

1. Duplicate dispatch on ErrAllRunnersBusy

When all runners are busy, TaskRunner.run re-enqueued the task while it remained in the running set. A concurrent queue tick could ClaimAndDequeue and spawn a second run() goroutine, leading to duplicate Ansible/Terraform execution.

2. Stopped tasks reported as failed on runner

After the server sent terminated_jobs (e.g. force stop), applyTerminatedJobs set status to stopped and killed the job. When job.Run returned an error, the runner error path overwrote stopped with failed.

3. HA pool state leak in failTaskRunnerLost

When the DB already had a terminal status, failTaskRunnerLost returned early without calling finalizeRemoteTaskLocked, leaking running/active state and skipping autorun when the reconciler won the finalize lock race.

Root cause

  • Requeue paths enqueued before releasing running/active bookkeeping.
  • Runner job_pool.go did not preserve an already-finished status after Run returned.
  • failTaskRunnerLost treated "already finished in DB" as a full no-op instead of completing cleanup.

Fix

  • Call onTaskStop before Enqueue on all requeue paths (ErrAllRunnersBusy, reconciler offline/undispatched requeue).
  • Extract runningJob.finalizeAfterRun to leave terminal statuses unchanged.
  • Call finalizeRemoteTaskLocked when DB status is already finished; broaden finalizeRemoteTaskLocked End check outside HA-only branch.
  • Add HA re-check before reconciler requeue persist; roll back in-memory state on persist failure.

Validation

  • go test ./services/tasks/... ./services/runners/... — all pass
  • New regression tests for ErrAllRunnersBusy requeue, finalizeAfterRun, HA failTaskRunnerLost, and reconciler requeue edge cases
Open in Web View Automation 

cursoragent and others added 2 commits July 1, 2026 11:03
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