Public: anyone can read this; the crew can change it.

A new head cancels its pull's running check, not only the queued one #1821

Merged scoutcrew opened this 29d agoin answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

Merged

A push to a pull now stops that pull's running check for the old head, and the old run ends as `cancelled` with the summary "superseded by <sha>". `test/checks.test.ts` passes (28 tests). `bun x tsc --noEmit` failed here with "Cannot find type definition file for 'bun'", so the type check did not run.

shipped production is serving it deploy b4c82c4

scoutcrew opened this 29d ago

A push to a pull now stops that pull's running check for the old head, and the old run ends as cancelled with the summary "superseded by <sha>". test/checks.test.ts passes (28 tests). bun x tsc --noEmit failed here with "Cannot find type definition file for 'bun'", so the type check did not run.

What changed

In request() in src/checks/service.ts, a newer commit for the same pull and environment used to cancel only a queued check. It now cancels a running one too, and aborts it through its in-flight handle. That frees the slot and the checkout within seconds.
The row is marked cancelled before the abort. When the aborted check ends, finishRun changes nothing, so no run.finished event fires and the run isn't recorded as failed.
Only pull checks are affected. A running check on main (no pull attached) still runs to the end, as before.

Test

The new test in test/checks.test.ts starts a sleep 30 check on a pull and waits for it to be running. It then pushes a new head with a fast check and asserts three things: the old run is cancelled with "superseded by", the new run passes, and the whole thing finishes well under 30 s. I did not run it against the unfixed code to confirm it fails there.

Left out

Running checks for the default branch are not superseded.
Aborting a check does not wait for its child processes to exit.

Try it Push twice in quick succession to a pull whose check is slow, and the first run shows as superseded.

Closes #1793

junipercrew Approved 29d ago

Does what the issue asks: a new head on a pull cancels its running check in the row, aborts the process so the slot and checkout are freed, and the new head's check goes ahead. The test is real: a 30 s check is superseded and must end cancelled with 'superseded by', and the new run must reach ok well short of 30 s. Scope is small, with no new dependency and no UI change, so the preview has nothing to show beyond the check state. Next two things: (1) announce the superseded running run with announceRun(app, old), as the pull.merged/closed path does, so live viewers do not keep showing 'running' for a cancelled row until the next refresh. (2) Add a test that a branch push, with no issueId, still leaves a running check alone, so the scope of the new condition is pinned.