You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A Unity Tests run that is superseded mid-flight keeps holding its concurrency group, and the run for the commit that actually matters cannot start until every one of the stale run's remaining legs has been handed a self-hosted runner purely so it can abort on the head guard.
#311 removed the case where a run is superseded before it starts (the head-check job skips the matrix). This is the residual case #327 names in passing: "a push that lands after a leg has started, where the per-leg head guard fires mid-flight."
cancel-in-progress: false is deliberate and correct -- it exists so an in-flight benchmark or a leg holding the organization Unity lock is never aborted mid-lease. But it also means run 3 sat pending with no jobs at all while run 1 waited on runners for legs whose only remaining act was to fail Require current PR head before setup.
In run 1, every already-scheduled leg had done exactly that:
failure Require current PR head before setup
skipped ... everything through Require acquired Unity lock ...
success Require confirmed Unity cleanup
So the work being queued for was guaranteed to be discarded, while the head that needed validating could not be scheduled. Cancelling run 1 by hand moved run 3 from pending to queued immediately.
Why it is worth fixing
It serializes badly with max-parallel: 1. Each doomed leg waits its turn for a scarce self-hosted Windows runner before aborting in seconds.
It makes a PR look green when it is not.gh pr checks reports no Unity entries at all for a run in pending (no check runs exist yet), so a reader -- human or automation -- sees only the passing checks and can conclude the PR settled. That is how this was nearly missed here.
Do not flip cancel-in-progress to true. A leg holding the organization Unity lock must not be cancelled mid-lease; that is the seat-leak the four-layer license-return guarantee exists to prevent, and Delete Tests/Runtime/WallstopStudios.DxMessaging.Tests.Runtime.csproj… #83 shows what happens when a lease is disturbed.
The fix must distinguish "leg has acquired the lock" from "leg is still queued and has acquired nothing." Only the latter is safe to drop.
Candidate approaches
Let the head guard cancel the run rather than fail the leg. When Require current PR head before setup fires on a leg that has not acquired the lock, call gh run cancel on its own run id. The already-running licensed legs finish their lease normally; the queued ones vanish, and the group frees as soon as the last live leg completes.
Split the concurrency group by phase, so unlicensed setup legs and licensed legs do not share one group. More invasive, and risks weakening the serialization that protects the lock.
A pre-leg freshness gate at the job level (if: on the matrix job reading a fresh live-head lookup) so a superseded leg is skipped rather than scheduled and failed. A skipped job needs no runner, which removes the queueing entirely. This looks like the smallest change with the largest effect, and it composes with ci: a superseded push keeps the Unity concurrency group, so the current SHA cannot start #311's existing head-check output.
Option 3 first; option 1 as the fallback if a job-level if: cannot see a fresh enough head.
Acceptance criteria
A run superseded mid-flight does not make the next run sit in pending with zero jobs.
A leg that has acquired the organization Unity lock is never cancelled or skipped by this mechanism.
A superseded leg that has not acquired the lock consumes no self-hosted runner time.
Problem
A Unity Tests run that is superseded mid-flight keeps holding its concurrency group, and the run for the commit that actually matters cannot start until every one of the stale run's remaining legs has been handed a self-hosted runner purely so it can abort on the head guard.
#311 removed the case where a run is superseded before it starts (the
head-checkjob skips the matrix). This is the residual case #327 names in passing: "a push that lands after a leg has started, where the per-leg head guard fires mid-flight."Evidence
PR #331, three pushes in close succession.
bc3764fcqueuede40abaee967df6ae(current)unity-tests.ymldeclares:cancel-in-progress: falseis deliberate and correct -- it exists so an in-flight benchmark or a leg holding the organization Unity lock is never aborted mid-lease. But it also means run 3 satpendingwith no jobs at all while run 1 waited on runners for legs whose only remaining act was to failRequire current PR head before setup.In run 1, every already-scheduled leg had done exactly that:
So the work being queued for was guaranteed to be discarded, while the head that needed validating could not be scheduled. Cancelling run 1 by hand moved run 3 from
pendingtoqueuedimmediately.Why it is worth fixing
max-parallel: 1. Each doomed leg waits its turn for a scarce self-hosted Windows runner before aborting in seconds.gh pr checksreports no Unity entries at all for a run inpending(no check runs exist yet), so a reader -- human or automation -- sees only the passing checks and can conclude the PR settled. That is how this was nearly missed here.fast-labelled benchmark legs compete for, which is the resource ci: the stuck-job watchdog reports success while it cannot read runner inventory #328 and [Feature]: Optimize CI #298 are both about.Constraints on any fix
cancel-in-progresstotrue. A leg holding the organization Unity lock must not be cancelled mid-lease; that is the seat-leak the four-layer license-return guarantee exists to prevent, and Delete Tests/Runtime/WallstopStudios.DxMessaging.Tests.Runtime.csproj… #83 shows what happens when a lease is disturbed.Candidate approaches
Require current PR head before setupfires on a leg that has not acquired the lock, callgh run cancelon its own run id. The already-running licensed legs finish their lease normally; the queued ones vanish, and the group frees as soon as the last live leg completes.if:on the matrix job reading a fresh live-head lookup) so a superseded leg is skipped rather than scheduled and failed. A skipped job needs no runner, which removes the queueing entirely. This looks like the smallest change with the largest effect, and it composes with ci: a superseded push keeps the Unity concurrency group, so the current SHA cannot start #311's existinghead-checkoutput.Option 3 first; option 1 as the fallback if a job-level
if:cannot see a fresh enough head.Acceptance criteria
pendingwith zero jobs.Related