concurrency slot handling cannot recover from failed or ambiguous PipelineRun starts

[chmouel]

📝 Description

Three places in the reconciler take a concurrency slot for a queued
PipelineRun and then try to start it. When starting fails, each of them
guesses what happened instead of checking, and each guesses differently.

The rule #2890 established is sound: give the slot back only when the run is
provably not going. The code approximates "provably not going" with "the start
patch returned an error" (ErrPipelineRunNotStarted). That approximation has
a hole. A Kubernetes patch can be committed by the API server while the client
times out waiting for the answer. The client sees an error, the code concludes
the run never started, releases the slot, and the queue admits one run more
than the limit allows. That is the exact bug #2890 set out to fix, surviving
in a narrower window.

The three call sites, with their individual problems:

• pkg/reconciler/queue_pipelineruns.go:119. Initial admission. Releases the
  slot on ErrPipelineRunNotStarted, which is unsafe for the reason above.
• pkg/reconciler/reconciler.go:465. Promotion after a run completes. Drops
  the candidate from the queue on any Get error, including a short network
  blip, and nothing ever puts it back.
• pkg/reconciler/finalizer.go:80. Promotion after a run is deleted. Returns
  on error without doing anything, leaving a still-pending run holding a slot
  it can never give back. A repository at concurrency_limit: 1 is then stuck
  until the watcher restarts.

The queue API makes a correct fix awkward. RemoveAndTakeItemFromQueue
releases one slot and reserves the next in a single call, and there is no
operation to put a reserved run back where it was. The internal running set is
a map[string]bool, so a reservation loses its position in the queue the
moment it is taken.

Found during a post-merge review of #2890, with reviewer input across three
rounds. The queue-level stall reproduces in a unit test.

🛠️ Suggested fix

Decide from fresh cluster state, not from the error. After a failed start
patch, fetch the PipelineRun again:

• still queued and pending: put it back in the queue, at its original position
• gone, finished or cancelled: drop the reservation and try the next one
• started: keep the slot
• cannot tell: keep the slot and report an error, so the limit is never
  exceeded even if it costs a retry

Supporting work:

• keep the reserved queue item (with its ordering fields) instead of a bool,
  and add a queue operation that returns a reservation to its original place
• share one start-outcome helper across the three call sites
• in reportFinalStatus, release and promote before writing the final state
  annotation (pkg/reconciler/reconciler.go:395 writes it first today, and
  reconciler.go:137 then blocks any retry of a failed promotion)

🧪 Testing Strategy

• Unit tests: with a real queue manager, cover failure before the patch, after the patch, and with an ambiguous patch result, in all three paths; assert the limit is never exceeded
• End-to-end tests where feasible