Auto-merge: a transient failure while handling the queue item permanently loses the scheduled merge

[willemkokke]

Description

A single transient failure while handling a pr_auto_merge queue item permanently loses the scheduled merge. The queue offers a retry mechanism, but the auto-merge handler opts out of it, and every one of its error paths — including plainly transient ones like a database read or a git repository open — reports "handled" to the queue. The user-visible result: a PR that is green, has auto-merge armed, and simply never merges, with nothing on the PR page and only a single server-side log line to hint why.

Verified by inspection at main (83af7aa) and v1.27.2; first analysed on v1.27.1 where we hit it in production on a self-hosted instance.

The queue has a retry path; the handler never uses it

modules/queue/workergroup.go (L103–L124 at main): safeHandler returns the items the handler could not deal with, and the worker re-pushes each one:

unhandled := q.safeHandler(batch...)
// ... back off if the whole batch failed ...
for _, item := range unhandled {
    if err := q.Push(item); err != nil { ... }
}

services/automerge/automerge.go (L44–L56): the handler unconditionally returns nil:

func handler(items ...string) []string {
 for _, s := range items {
  ...
  handlePullRequestAutoMerge(id, sha)
 }
 return nil          // always "everything was handled"
}

handlePullRequestAutoMerge returns nothing at all. Its body (L156–L278) has ~10 log.Error(...); return bail-outs, and several are plainly transient:

line (main @ 83af7aa)	failure	transient?
163	GetPullRequestByID	yes — DB
170	GetScheduledMergeByPullID	yes — DB
178	LoadBaseRepo	yes — DB
185	git.OpenRepository	yes — git/fs
192	GetRefCommitID	yes — git/fs
204	LoadHeadRepo	yes — DB
232	IsPullCommitStatusPass	yes — DB
243	GetUserByID (doer)	yes — DB
249	GetDoerRepoPermission	yes — DB
258	CheckPullMergeable (non-sentinel error)	often

Every one of them reports success to the queue. The item is gone, and nothing will ever re-enqueue it unless another status/event happens to arrive for that same head SHA. On a PR whose CI has already settled green, no such event is coming — the scheduled merge is silently lost while the pull_auto_merge row (the durable record of intent) still exists.

The mid-function assumption at L195–L198 ("Just ignore this request because a new request expected in the queue") compounds this: if the enqueue for the new head SHA is itself lost to any path above, the PR is stranded with no retry and no warning.

A second, smaller bug: seven distinct refusal reasons are all logged as "unauthorized user"

services/automerge/automerge.go L253–L257:

if err := pull_service.CheckPullMergeable(...); err != nil {
    if errors.Is(err, pull_service.ErrNotReadyToMerge) {
        log.Info("%-v was scheduled to automerge by an unauthorized user", pr)
        return
    }

ErrNotReadyToMerge is wrapped with a precise reason at seven sites in services/pull/merge.go (L603–L629): required status checks, approvals, requested changes, official review requests, head branch behind base, missing code owner reviews, changed protected files. The handler discards err — the one value that already holds the answer — and logs a message that sends anyone debugging from logs chasing a permissions problem that does not exist. (In our case the real cause was "head branch is behind the base branch".)

Reproduction

By inspection this needs no race: schedule auto-merge on a PR, make any one of the DB/git calls in the table fail once (e.g. restart the database for a beat) while the final commit-status event is being processed, and the PR stays green, armed, and unmerged forever.

In production we observed exactly this class on a self-hosted 1.27.1 instance under normal operation: a PR green and armed, updated_unix on the pull_auto_merge row untouched, and the merge never happening until a human intervened.

Proposed fix

I have a PR ready that:

1. makes handlePullRequestAutoMerge return an error for transient infrastructure failures (and nil for everything genuinely handled, including definite refusals like ErrNotReadyToMerge, a deleted PR, or a vanished head branch), and has handler return those items so the queue's existing retry path engages;
2. bounds the retries per item (the queue's requeue loop has no retry limit and no dead-letter — we have separately watched a poison item in another queue spin for days at workergroup.go L110, at one point dominating total log volume — so unbounded requeueing would trade a silent drop for a permanent spin);
3. logs the precise CheckPullMergeable refusal reason instead of "scheduled to automerge by an unauthorized user".

A complementary (heavier) hardening, not in that PR, would be a periodic reconciler over pull_auto_merge rows that re-enqueues any still-open scheduled merge — that would also self-heal losses from restarts and missed events. Happy to discuss.

Gitea Version

Observed on 1.27.1 (self-hosted); code verified unchanged at v1.27.2 and main (83af7aa).

Can you reproduce the bug on the Gitea demo site?

No (requires inducing a transient DB/git failure at the right moment; the analysis above is by source inspection).

Log Gist

Not applicable — the point of the report is that the only trace is a single generic log.Error line (e.g. GetRefCommitID: ...) with nothing on the PR itself.

Operating System / How are you running Gitea?

Self-hosted, Linux, Docker (rootless image), PostgreSQL. Not relevant to the finding — the code path is platform-independent.