Skip to content

Skip the pause transition when a task is no longer Executing

Placeholder ppxd requested to merge fix/pause-handlers-cancel-race into main

Summary

  • All three pause outcomes (suspend, timeout, transient) share one guard that writes Paused only from Executing, the sole legal source of that edge. The timeout and transient handlers previously transitioned blind; OnPausedAsync already had the check.
  • The database outcome is unchanged in every case. TaskState.EnsureValidTransition is the first statement of TransitionStateAsync, before any SQL, and DeploymentPipelineRunner.SafeCompleteAsync swallows what it throws — so a blind transition left the row exactly as the guard now leaves it, with an empty transaction rolled back. What changes is that an expected race stops being signalled by an exception.
  • Removing the exception removes the Error that SafeCompleteAsync logged, which on the timeout and transient paths was the only trace that a Cancelling task had permanently lost its environment's concurrency slot. The Cancelling skip is now logged at Warning naming that consequence; other skipped states stay Information so the routine Paused case does not bury it. For the suspend outcome, which already skipped and logged Information, Warning is a deliberate uplift — the same wedged state should not be reported at two levels depending on which outcome reached it.

What this does not fix

A task left Cancelling is still stuck: it holds the environment's concurrency slot, its only edges out (Cancelled, Failed) exist solely in DeploymentCompletionHandler and so need a live pipeline, CancelTaskAsync returns early on Cancelling, ResumeTaskAsync throws unless Paused, and no recurring job reaps it. Freeing it needs cross-pod cancel propagation or a stale-active-task reaper; both are pre-existing and out of scope here.

The success path does not share this. OnSuccessAsync also transitions from a hardcoded Executing, but it is the one completion callback the runner invokes directly rather than through SafeCompleteAsync, so its exception reaches the general catch, which routes to OnFailureAsync; that resolves the current state and performs the legal Cancelling → Failed. The row ends terminal and the slot is freed — at the cost of a deployment that succeeded being recorded as failed.

Test plan

  • Unit: 3 Theories over all three pause outcomes × (raced by cancel / not Executing / still Executing)
  • Unit: 9 log-level cases pinning Warning for Cancelling, Information for every other skipped state
  • Unit: runner test pinning that an illegal transition out of OnSuccessAsync falls through to OnFailureAsync
  • Mutation: downgrading the Cancelling alert, warning on every skip, and dropping the consequence from the message each fail tests (3 / 9 / 3); routing OnSuccessAsync through SafeCompleteAsync fails the new runner test
  • Full unit suite green (6200)

Merge request reports