.gitea/scripts/gitea-merge-queue.py

@@ -9,17 +9,33 @@ queue. This script provides the missing serialized policy in user space:
    candidate (REQUEST_CHANGES, mergeable!=True, insufficient genuine approvals,
    or red required CI) is SKIPPED so it cannot head-of-line block newer ready
    PRs; the scan continues to the next candidate.
-2. Refuse to act unless main's BP-required contexts are green.
+2. Refuse to act unless main's BP-required contexts are green. This is also
+   the serialized backstop for direct-merge (see below): after a direct merge,
+   main re-runs push CI and this gate PAUSES the queue if main goes red, so no
+   merge piles onto an unverified/red main (issue #2358).
 3. Refuse fork PRs; the queue may only mutate same-repo branches.
-4. If the PR branch does not contain current main, call Gitea's
-   /pulls/{n}/update endpoint and stop. CI must rerun on the updated head.
+4. DIRECT-MERGE when conflict-free (issue #2358). When Gitea reports the PR
+   conflict-free (mergeable is True) and the merge bar below is met, MERGE IT
+   DIRECTLY — even if its head does not contain current main. We do NOT call
+   /pulls/{n}/update first: branch protection does not require strict
+   up-to-date, so behind-main conflict-free PRs merge cleanly, and calling
+   /update would trigger Gitea dismiss_stale_approvals (dismissing the genuine
+   approvals and forcing a re-review every tick — the rebase-churn bottleneck).
+   The /update path is used ONLY when the PR is DEFINITIVELY not mergeable
+   (mergeable is literal False) AND its head lacks current main — refreshing the
+   branch may resolve a behind-main non-conflict; a real conflict returns HTTP
+   409 and the PR is HELD per #2352. mergeable=None/missing (Gitea STILL
+   COMPUTING conflict state) is a distinct fail-closed WAIT: never merged AND
+   never /update'd — calling /update during the compute window would dismiss the
+   PR's genuine approvals (dismiss_stale_approvals) and re-introduce the exact
+   rebase-churn this queue eliminates. None is re-checked next tick.
 5. Merge ONLY when, on the PR's CURRENT head sha:
      - >= REQUIRED_APPROVALS distinct GENUINE official APPROVED reviews from
        the recognised reviewer set (not stale, not dismissed, commit_id ==
        current head), AND
      - no open official REQUEST_CHANGES on the current head, AND
      - every BP-required status context is green, AND
-     - the PR is mergeable.
+     - the PR is mergeable (Gitea reports it conflict-free).
 
 Authoritative gates (fail-closed):
   - The REQUIRED status contexts come from BRANCH PROTECTION
@@ -622,29 +638,32 @@ def evaluate_merge_readiness(
     approvers: set[str],
     request_changes: list[str],
     pr_has_current_base: bool,
-    mergeable: bool,
+    mergeable: bool | None,
     pr_labels: set[str] | None = None,
 ) -> MergeDecision:
     # 1) Main's push-required contexts must be green. Combined state can be
     #    "failure" due to non-blocking jobs (continue-on-error: true) that do
     #    not gate merges, so check the explicit required set, not combined.
+    #
+    #    This main-green gate is ALSO the serialized backstop that makes the
+    #    direct-merge (no update) path safe (issue #2358): after a direct merge
+    #    of a behind-main PR, main re-runs its push CI; if a semantic main-break
+    #    slips through (PR green standalone but broken when combined with newer
+    #    main), main's required contexts go red and this gate PAUSES the queue —
+    #    no further merge piles onto an unverified/red main until it is green.
     main_latest = latest_statuses_by_context(main_status.get("statuses") or [])
     main_ok, main_bad = required_contexts_green(main_latest, push_required_contexts())
     if not main_ok:
         return MergeDecision(False, "pause", "main required contexts not green: " + ", ".join(main_bad))
 
-    # 2) PR head must contain current main.
-    if not pr_has_current_base:
-        return MergeDecision(False, "update", "PR head does not contain current main")
-
-    # 3) No open official REQUEST_CHANGES on the current head.
+    # 2) No open official REQUEST_CHANGES on the current head.
     if request_changes:
         return MergeDecision(
             False, "wait",
             "open REQUEST_CHANGES on current head from: " + ", ".join(sorted(request_changes)),
         )
 
-    # 4) Enough distinct genuine official approvals on the current head.
+    # 3) Enough distinct genuine official approvals on the current head.
     if len(approvers) < required_approvals:
         return MergeDecision(
             False, "wait",
@@ -653,7 +672,7 @@ def evaluate_merge_readiness(
             f"need {required_approvals}",
         )
 
-    # 5) Every BRANCH-PROTECTION-REQUIRED status context must be green. This is
+    # 4) Every BRANCH-PROTECTION-REQUIRED status context must be green. This is
     #    the authoritative status gate — NON-required reds (qa-review,
     #    security-review, sop-tier/sop-checklist when not BP-required, E2E Chat,
     #    Staging SaaS, ci-arm64-advisory, continue-on-error jobs) are NOT
@@ -663,16 +682,53 @@ def evaluate_merge_readiness(
     if not ok:
         return MergeDecision(False, "wait", "required contexts not green: " + ", ".join(missing_or_bad))
 
-    # 6) Gitea must consider the PR mergeable (no conflicts).
-    if not mergeable:
-        return MergeDecision(False, "wait", "PR is not mergeable (conflicts)")
+    # 5) DIRECT-MERGE when conflict-free (issue #2358 — throughput fix).
+    #    If Gitea reports the PR conflict-free (mergeable is True), MERGE IT
+    #    DIRECTLY even if its head does not yet contain current main. Branch
+    #    protection does NOT require strict up-to-date, so a behind-main but
+    #    conflict-free PR merges cleanly. We deliberately do NOT call
+    #    /pulls/{n}/update first: update triggers Gitea dismiss_stale_approvals,
+    #    which would dismiss the PR's genuine approvals and force a full
+    #    re-review every tick — the rebase-churn bottleneck that collapsed
+    #    throughput to ~0/hr with dozens of mergeable PRs open.
+    #
+    #    The merge bar is UNCHANGED: we only reach here with main green +
+    #    >= required genuine approvals on the current head + no open
+    #    REQUEST_CHANGES + every BP-required context green. The trade-off is
+    #    that the PR's CI ran on a possibly-behind base, so a SEMANTIC main-break
+    #    is caught by POST-merge main CI (step 1's pause backstop) rather than
+    #    pre-merge. force_merge is used ONLY for missing-but-non-required
+    #    governance reds (required are green + approvals genuine), never to
+    #    bypass a failing required context or an approval shortfall.
+    if mergeable is True:
+        force = _non_required_red_present(latest, required_contexts)
+        return MergeDecision(True, "merge", "ready", force=force)
 
-    # Ready. Use force_merge ONLY if the merge would otherwise be blocked by
-    # missing-but-non-required governance contexts. Required are green and
-    # approvals are genuine, so force only bypasses non-required reds — never a
-    # failing required context or missing approval.
-    force = _non_required_red_present(latest, required_contexts)
-    return MergeDecision(True, "merge", "ready", force=force)
+    # 6) NOT (yet) mergeable. TRI-STATE, fail-closed — never merge on an unknown.
+    #    We MUST distinguish "still computing" (None/missing) from a "definitive
+    #    conflict" (False); collapsing them would route a behind-main but
+    #    STILL-COMPUTING PR into the /update path, whose dismiss_stale_approvals
+    #    is the rebase-churn this change eliminates.
+    #
+    #    mergeable is None  → Gitea has NOT finished computing conflict state.
+    #    WAIT: do nothing this tick — never /update (would dismiss genuine
+    #    approvals during the compute window → churn), never merge. Re-check next
+    #    tick once Gitea reports a decisive True/False.
+    if mergeable is None:
+        return MergeDecision(
+            False, "wait",
+            "PR mergeability is still being computed (mergeable=None) — waiting",
+        )
+
+    # mergeable is False → DEFINITIVE not-mergeable. If the head also does not
+    #    contain current main, try the /update path to refresh the branch (this
+    #    may resolve a behind-main non-conflict; a real conflict returns HTTP 409
+    #    and process_once HOLDs the PR per #2352). If the head already contains
+    #    current main yet Gitea still reports not-mergeable, there is nothing the
+    #    queue can do (genuine conflict against current main) — WAIT.
+    if not pr_has_current_base:
+        return MergeDecision(False, "update", "PR not mergeable and head does not contain current main")
+    return MergeDecision(False, "wait", "PR is not mergeable (conflicts)")
 
 
 def get_branch_head(branch: str) -> str:
@@ -1076,12 +1132,20 @@ def _evaluate_candidate(
     # never treated as green).
     pr_status = get_combined_status(head_sha)
     pr_labels = label_names(pr)
-    # FAIL-CLOSED: Gitea returns mergeable=None (or omits the field) while it is
-    # still COMPUTING conflict state. Only the literal True is decisive proof the
-    # PR is conflict-free; None and False both mean "not (yet) mergeable". We must
-    # NOT autonomously merge on an unknown — treat anything but True as not-yet-
-    # mergeable so evaluate_merge_readiness returns a "wait" decision.
-    mergeable = pr.get("mergeable") is True
+    # FAIL-CLOSED, TRI-STATE: Gitea returns mergeable=None (or omits the field)
+    # while it is still COMPUTING conflict state, mergeable=False for a definitive
+    # conflict, and mergeable=True only when it has proven the PR conflict-free.
+    # We preserve all THREE states (do NOT collapse None/missing into False):
+    #   - True            → direct-merge eligible (step 5).
+    #   - None / missing  → still computing → WAIT (never merge, never update,
+    #                       never dismiss approvals); re-check next tick.
+    #   - False           → definitive conflict → the update/hold path (step 6).
+    # Collapsing None→False would route a behind-main but STILL-COMPUTING PR into
+    # the /update path, which triggers dismiss_stale_approvals — the exact
+    # rebase-churn this change eliminates. Normalize only to the literal True /
+    # False / None set (some Gitea versions omit the key entirely → None).
+    raw_mergeable = pr.get("mergeable")
+    mergeable: bool | None = raw_mergeable if isinstance(raw_mergeable, bool) else None
 
     reviews = get_pull_reviews(pr_number)
     approvers, request_changes = genuine_approvals(

.gitea/scripts/tests/test_gitea_merge_queue.py

@@ -143,13 +143,72 @@ def test_merge_decision_requires_main_green_pr_green_and_current_base():
     assert decision.force is False  # no non-required reds present
 
 
-def test_merge_decision_updates_stale_pr_before_merge():
-    decision = mq.evaluate_merge_readiness(**_ready_kwargs(pr_has_current_base=False))
+def test_behind_main_but_mergeable_pr_merges_directly():
+    """§SOP-22 (#2358): a behind-main but CONFLICT-FREE PR (mergeable is True)
+    merges DIRECTLY — no update step. Branch protection does not require strict
+    up-to-date, and calling /update would dismiss the genuine approvals
+    (dismiss_stale_approvals), forcing re-review every tick (the throughput
+    bottleneck). This replaces the old update-before-merge behavior."""
+    decision = mq.evaluate_merge_readiness(
+        **_ready_kwargs(pr_has_current_base=False, mergeable=True)
+    )
+
+    assert decision.ready is True
+    assert decision.action == "merge"
+
+
+def test_behind_main_and_not_mergeable_pr_updates():
+    """The /update path is reached ONLY when the PR is NOT mergeable AND its head
+    lacks current main — refreshing the branch may resolve a behind-main
+    non-conflict; a real conflict 409s and is held (#2352)."""
+    decision = mq.evaluate_merge_readiness(
+        **_ready_kwargs(pr_has_current_base=False, mergeable=False)
+    )
 
     assert decision.ready is False
     assert decision.action == "update"
 
 
+def test_current_base_but_not_mergeable_pr_waits():
+    """Up-to-date with main yet Gitea reports not-mergeable → genuine conflict
+    against current main (or still computing). The queue cannot act: WAIT,
+    never update (update would not help) and never merge (fail-closed)."""
+    decision = mq.evaluate_merge_readiness(
+        **_ready_kwargs(pr_has_current_base=True, mergeable=False)
+    )
+
+    assert decision.ready is False
+    assert decision.action == "wait"
+    assert "not mergeable" in decision.reason
+
+
+def test_behind_main_and_mergeable_none_waits_not_update():
+    """§SOP-22 (CR2 #2374) — the churn-residual fix. A BEHIND-MAIN PR whose
+    mergeability Gitea is STILL COMPUTING (mergeable is None) must WAIT, NOT take
+    the /update path. The old code collapsed None→False, so a behind-main +
+    None PR returned action="update" → /pulls/{n}/update → dismiss_stale_approvals
+    → the exact rebase-churn this change eliminates, fired during the compute
+    window. None and False are now DISTINCT: None waits, False updates."""
+    decision = mq.evaluate_merge_readiness(
+        **_ready_kwargs(pr_has_current_base=False, mergeable=None)
+    )
+
+    assert decision.ready is False
+    assert decision.action == "wait"  # NOT "update" — no churn during compute
+    assert "computed" in decision.reason
+
+
+def test_current_base_and_mergeable_none_waits():
+    """Up-to-date with main + mergeable None (still computing) → WAIT (unchanged
+    fail-closed; just confirming None is never merged regardless of base)."""
+    decision = mq.evaluate_merge_readiness(
+        **_ready_kwargs(pr_has_current_base=True, mergeable=None)
+    )
+
+    assert decision.ready is False
+    assert decision.action == "wait"
+
+
 def test_MergePermissionError_inherits_from_ApiError():
     assert issubclass(mq.MergePermissionError, mq.ApiError)
 
@@ -506,6 +565,131 @@ def test_process_once_merges_when_mergeable_is_true(monkeypatch):
     assert calls["hold_label"] is None
 
 
+def test_process_once_behind_main_mergeable_none_waits_no_update(monkeypatch):
+    """§SOP-22 (CR2 #2374) — end-to-end churn-residual regression. A BEHIND-MAIN
+    PR (commits do NOT contain main_sha) whose mergeability Gitea is STILL
+    COMPUTING (mergeable=None) must WAIT: process_once returns 0 and NEVER calls
+    update_pull (which dismisses genuine approvals via dismiss_stale_approvals)
+    NOR merge_pull NOR hold. The old None→False collapse routed this exact case
+    into the /update path → approval-dismissing rebase churn during the compute
+    window. This proves the durable churn elimination: no update, approvals
+    preserved, re-checked next tick."""
+    calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
+    _fully_ready_process_once_monkeypatch(monkeypatch, mergeable=None, calls=calls)
+    # Make the head BEHIND main: commits do NOT contain main_sha. This is the
+    # case the bug missed (the prior None test had current base, masking it).
+    behind_head = "a" * 40
+    monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": behind_head}])
+
+    rc = mq.process_once(dry_run=False)
+
+    assert rc == 0
+    assert calls["updated"] is False  # NO /update → approvals NOT dismissed
+    assert calls["merge_attempts"] == 0  # never merge on an unknown
+    assert calls["hold_label"] is None  # transient → not held, retried next tick
+
+
+# --------------------------------------------------------------------------
+# §SOP-22: DIRECT-MERGE throughput fix (#2358). A conflict-free 2-genuine PR
+# merges WITHOUT a pre-merge /update call, so its approvals are NOT dismissed by
+# dismiss_stale_approvals. The merge bar (2-genuine-on-current-head +
+# BP-required green + mergeable + no RC + opt-out) is UNCHANGED; only the
+# unnecessary update-before-merge churn is removed. The /update path survives
+# for the genuine case it is needed (not-mergeable + behind-main), where a real
+# conflict 409s and is held per #2352. mergeable=None stays fail-closed.
+# --------------------------------------------------------------------------
+
+
+def test_process_once_merges_conflict_free_pr_without_update(monkeypatch):
+    """§SOP-22(a) — the core throughput fix. A conflict-free, fully-approved PR
+    merges WITHOUT update_pull ever being called. The old behavior called
+    /update first whenever the head lacked current main, which dismissed the 2
+    genuine approvals (dismiss_stale_approvals) and forced re-review every tick.
+    Assert update_pull is NOT invoked and merge_pull IS invoked."""
+    calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
+    _fully_ready_process_once_monkeypatch(monkeypatch, mergeable=True, calls=calls)
+    # Make the head BEHIND main: commits do NOT contain main_sha. Under the old
+    # logic this alone forced an update_pull; under the fix it merges directly.
+    head_sha = "a" * 40
+    monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": head_sha}])
+
+    rc = mq.process_once(dry_run=False)
+
+    assert rc == 0
+    assert calls["merge_attempts"] == 1  # merged directly
+    assert calls["updated"] is False  # NO update_pull → approvals NOT dismissed
+    assert calls["hold_label"] is None
+
+
+def test_process_once_behind_main_conflict_free_merges_directly(monkeypatch):
+    """§SOP-22(b) — explicit behind-main + conflict-free case: it still merges
+    directly (branch protection does not require strict up-to-date)."""
+    calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
+    _fully_ready_process_once_monkeypatch(monkeypatch, mergeable=True, calls=calls)
+    behind_head = "a" * 40
+    monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": behind_head}])
+
+    rc = mq.process_once(dry_run=False)
+
+    assert rc == 0
+    assert calls["merge_attempts"] == 1
+    assert calls["updated"] is False
+
+
+def test_process_once_pauses_when_main_not_green_no_direct_merge(monkeypatch):
+    """§SOP-22 backstop — the serialized safety that makes direct-merge safe:
+    when main's required push contexts are NOT green (e.g. a prior direct merge
+    introduced a semantic main-break caught by post-merge main CI), the queue
+    PAUSES — it does NOT merge the next PR onto an unverified/red main."""
+    calls = {"merge_attempts": 0, "hold_label": None, "updated": False}
+    _fully_ready_process_once_monkeypatch(monkeypatch, mergeable=True, calls=calls)
+    main_sha = "b" * 40
+
+    def red_main_combined(sha):
+        if sha == main_sha:
+            return {"state": "failure",
+                    "statuses": [{"context": "CI / all-required (push)", "status": "failure"}]}
+        return {"state": "success",
+                "statuses": [{"context": "CI / all-required (pull_request)", "status": "success"}]}
+    monkeypatch.setattr(mq, "get_combined_status", red_main_combined)
+
+    rc = mq.process_once(dry_run=False)
+
+    assert rc == 0
+    assert calls["merge_attempts"] == 0  # paused — no merge onto red main
+    assert calls["updated"] is False
+
+
+def test_direct_merge_bar_unchanged_behind_main(monkeypatch):
+    """§SOP-22(d) — the merge bar is UNCHANGED on the new direct-merge path. A
+    behind-main + conflict-free PR is still rejected (no merge) when ANY gate
+    fails: insufficient genuine approvals, red required context, open
+    REQUEST_CHANGES, or opt-out label. Direct-merge removes the update churn, it
+    does NOT weaken the bar — fail-closed on every gate."""
+    head_sha = "a" * 40
+    behind_main = dict(pr_has_current_base=False, mergeable=True)
+
+    # <2 genuine approvals → wait, not merge.
+    d = mq.evaluate_merge_readiness(
+        **_ready_kwargs(approvers={"agent-researcher"}, **behind_main)
+    )
+    assert d.action == "wait" and d.ready is False
+
+    # Red required context → wait, not merge.
+    red_required = {"state": "failure", "statuses": [
+        {"context": "CI / all-required (pull_request)", "status": "failure"}]}
+    d = mq.evaluate_merge_readiness(
+        **_ready_kwargs(pr_status=red_required, **behind_main)
+    )
+    assert d.action == "wait" and d.ready is False
+
+    # Open REQUEST_CHANGES on current head → wait, not merge.
+    d = mq.evaluate_merge_readiness(
+        **_ready_kwargs(request_changes=["agent-reviewer-cr2"], **behind_main)
+    )
+    assert d.action == "wait" and d.ready is False
+
+
 # --------------------------------------------------------------------------
 # Fix 3: status fetch is fail-closed (failed fetch != green)
 # --------------------------------------------------------------------------
@@ -707,13 +891,17 @@ def _stale_pr_update_409_monkeypatch(monkeypatch, queued_issues, calls):
     # Scan-loop process_once enumerates candidates via list_candidate_issues.
     monkeypatch.setattr(mq, "list_candidate_issues", lambda *, auto_discover: queued_issues)
     monkeypatch.setattr(mq, "get_pull", lambda n: {
-        "state": "open", "number": n, "mergeable": True,
+        "state": "open", "number": n, "mergeable": False,
         "base": {"ref": "main", "repo_id": 1},
         "head": {"sha": head_sha, "repo_id": 1},
         "labels": [{"name": "merge-queue"}],
     })
-    # NOTE: commits do NOT contain main_sha → pr_has_current_base is False →
-    # decision.action == "update".
+    # NOTE: mergeable is False (real conflict) AND commits do NOT contain
+    # main_sha → pr_has_current_base is False → decision.action == "update".
+    # Under the #2358 direct-merge fix the update path is reached ONLY when the
+    # PR is NOT mergeable; a mergeable=True behind-main PR would merge directly,
+    # so this fixture sets mergeable=False to exercise the #2352 409-on-update
+    # hold path.
     monkeypatch.setattr(mq, "get_pull_commits", lambda n: [{"sha": head_sha}])
     monkeypatch.setattr(mq, "get_pull_reviews", lambda n: [
         {"state": "APPROVED", "user": {"login": "agent-researcher"},