4323527699
A commit that landed while a build of the same pool was still PENDING or RUNNING was skipped -- and then never built. The watcher read a branch's dueness from its lagging local ref, but fast-forwarded that very ref at the end of the same cycle, so from the next cycle on the skipped branch no longer looked changed and the "later cycle" its doc comment promised never came. The branch stayed behind origin until another push arrived or recoverOnStartup healed it at the next restart. The dueness is now remembered where it is decided: RepoWatch.deferredBranches holds the branches startBuildIfDue refused for a busy pool, and the next enqueue takes that set over and reconsiders them alongside the changed ones. A branch still waiting is put back by that same call, one gone from origin yields no head commit and drops out -- so the memory clears itself. WatcherTest covered only that *other* branches keep going while a build runs, never the next cycle of the same branch; both the queue-behind case and the forgetting of a vanished branch are covered now. release 1.11.2 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #57
6.1 KiB
WARNING: This document describes only the change applied in this PR. It may already be outdated once the next PR is merged. Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
The Problem
A commit that lands while a build of the same pool is still PENDING or RUNNING was never built.
The watcher's poll cycle does three things in this order: it works out which branches are due, it enqueues them, and it then fast-forwards the primary checkout's local branch refs to origin. Dueness was read from those very refs: a local ref lagging behind its origin counterpart meant "this branch has new commits".
startBuildIfDue refuses a branch whose pool is still busy, and its doc comment promised that the branch waits for a later cycle.
But the fast-forward at the end of the same cycle moves the skipped branch's ref up to origin regardless.
From the next cycle on, the branch no longer looks changed, nothing calls startBuildIfDue for it, and the promised later cycle never comes.
The only record that the branch was due had just been erased by the step that was supposed to be harmless.
The branch then stays behind origin until either another push arrives or the instance restarts — recoverOnStartup heals the state, which is why the bug was easy to miss.
Observed on repository werkjournal, 2026-09-12:
| Commit | Pushed | Result |
|---|---|---|
3494793 |
17:30 | built, green, deployed |
6406853 |
~40 s later, while 3494793 was building |
never built |
f20fc73 |
later, pool idle | enqueued immediately |
The existing comment on fastForwardLocalRefs shows the coupling was known:
it explains why syncing earlier in the cycle would suppress the build it prepares.
That reasoning covers syncing too early within one cycle, not syncing after an enqueue was skipped.
Non-Goals
- No change to the queue-behind policy itself: one build per pool at a time stays as it is, and a commit still waits rather than starting a second build.
- No change to
fastForwardLocalRefs: the local refs keep catching up to origin every cycle, which build tools in the shared.gitrely on. - No persistence of the new memory across a restart —
recoverOnStartupalready re-enqueues branches whose latest build never finished. - No queue of intermediate commits: as before, the branch is built at its current origin head, not once per skipped commit.
The Scenarios
Feature: a push during a running build is not lost
Background
- A pool is one branch and one build definition (
BuildDefinition.poolName); the watcher runs at most one build per pool at a time. - Fast-forwarding local refs moves the primary checkout's
refs/heads/*up torefs/remotes/origin/*at the end of each poll cycle. - A branch is deferred when its enqueue was refused solely because its pool was still
PENDINGorRUNNING.
Scenario#57.01: a commit pushed during a running build is built afterwards
So that no push is silently dropped just because it arrived at a busy moment.
- Given branch
mainhas aRUNNINGbuild at commitcommit-1- and origin's head of
mainiscommit-2
- and origin's head of
- When a poll cycle runs
- Then no build is enqueued
- And when the running build has finished
- and the local ref of
mainhas meanwhile been fast-forwarded to origin, so the ref comparison no longer reports new commits - and a further poll cycle runs
- and the local ref of
- Then a build of
mainatcommit-2is enqueued
Verified by
Scenario#57.02: a deferred branch that disappears is forgotten
So that the new memory cannot grow without bound or keep asking about branches that no longer exist.
- Given branch
mainwas deferred behind a running build - When
mainis gone from origin- and further poll cycles run
- Then no build is enqueued for it
- and the watcher stops carrying it as deferred
Verified by
The Solution
The dueness of a branch is no longer derived from the local ref alone; it is remembered where it is decided.
Watcher.RepoWatch — the per-repository poll memory — gains deferredBranches.
startBuildIfDue adds the branch to it in exactly the one branch of the method that refuses for a busy pool, and nowhere else:
a refusal for an already-built commit, a missing origin head or the pull-request gate is final, not a wait.
enqueueDueBranches takes the set over at the start of each cycle and clears it, then adds those branches to the changed ones.
They run through the same definition selection and the same startBuildIfDue as any changed branch, so nothing about the decision differs — they are merely reconsidered.
A branch whose pool is still busy is put back by that very call; a branch that is gone from origin yields no head commit, is refused, and simply drops out.
That makes the memory self-clearing: a branch is remembered exactly as long as it is waiting.
This is the second of the two options the bug report weighed, and it was chosen over excluding the skipped branches from the fast-forward.
It states the invariant instead of protecting an accidental one:
the watcher now knows a branch is due, rather than inferring it from a ref that other things are allowed to move —
which also holds with watcher.fastForwardLocalRefs: false and against ref movements the watcher did not cause.
The doc comments of startBuildIfDue and fastForwardLocalRefs are corrected in the same move.
The first promised a later cycle it could not keep; the second explained only half of its ordering constraint.
Additional Changes
- The test gap the bug report names is closed:
WatcherTest's "a poll cycle completes while a build is running and still enqueues other branches" only ever checked that other branches keep going, never the next cycle of the same branch.
Follow-up PRs
- None.