From a89e2680c9dc0f19811ab00f13ee0bc5ce7d2494 Mon Sep 17 00:00:00 2001 From: devops-engineer Date: Sat, 6 Jun 2026 11:25:55 -0700 Subject: [PATCH 1/2] merge-queue: direct-merge conflict-free PRs without update-churn (#2358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge-queue cron called /pulls/N/update on any PR whose head did not contain current main, BEFORE merging. Gitea's dismiss_stale_approvals then dismissed the PR's 2 genuine approvals on update, dropping it to 0-genuine and forcing a full re-review every tick. With ~51 mergeable PRs open this collapsed throughput to ~0/hr. Branch protection does NOT require strict up-to-date (behind-main PRs merge directly), so the update-before-merge was unnecessary churn. Fix (targeted, in evaluate_merge_readiness): - When the PR is conflict-free (mergeable is True) and the merge bar is met (main green + >=required genuine approvals on current head + no open REQUEST_CHANGES + every BP-required context green), MERGE IT DIRECTLY — even if behind main. No /update first → approvals are NOT dismissed → no re-review churn. - The /update path is now reached ONLY when the PR is NOT mergeable (mergeable False/None) AND its head lacks current main. A real conflict still 409s on update and is HELD per #2352 (no regression). A current-base not-mergeable PR WAITs. mergeable=None stays fail-closed (never merged). The merge bar is UNCHANGED. Safety backstop: step 1 (main required push contexts green) PAUSES the queue when main is red, so after a direct merge a semantic main-break caught by post-merge main CI stops the next merge (serialized safety) — this is what makes pre-merge-update-free safe. Tests (§SOP-22): conflict-free PR merges WITHOUT update_pull (assert update not invoked, merge invoked); behind-main conflict-free merges directly; not-mergeable behind-main still updates; not-mergeable current-base waits; 409-on-update fixture switched to mergeable=False (real conflict) and still holds per #2352; merge bar still rejects <2 genuine / red required / RC on the new direct path; pause-when-main-red backstop. 62 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/scripts/gitea-merge-queue.py | 78 +++++++--- .../scripts/tests/test_gitea_merge_queue.py | 147 +++++++++++++++++- 2 files changed, 200 insertions(+), 25 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 017cef6be..54c5c54d1 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -9,17 +9,29 @@ 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 NOT mergeable (mergeable + False/None) 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 is fail-closed (never merged). 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 @@ -628,23 +640,26 @@ def evaluate_merge_readiness( # 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 +668,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 +678,39 @@ 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: + 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 mergeable (mergeable is False/None — conflict or still computing). + # FAIL-CLOSED: never merge on an unknown. 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, or Gitea is still + # computing) — 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: diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 4f7be7fc6..4c1ef8a64 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -143,13 +143,45 @@ 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_MergePermissionError_inherits_from_ApiError(): assert issubclass(mq.MergePermissionError, mq.ApiError) @@ -506,6 +538,107 @@ def test_process_once_merges_when_mergeable_is_true(monkeypatch): assert calls["hold_label"] is None +# -------------------------------------------------------------------------- +# §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 +840,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"}, -- 2.52.0 From c90b44fc47ab3f4b0c68d521913ae2fe92c62455 Mon Sep 17 00:00:00 2001 From: devops-engineer Date: Sat, 6 Jun 2026 11:42:14 -0700 Subject: [PATCH 2/2] merge-queue: fail-closed WAIT (not update) on mergeable=None compute window (#2374 CR2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CR2's REQUEST_CHANGES on #2374: a residual approval-dismissing churn path remained in the NON-mergeable branch. The call site collapsed mergeable None/missing/False all to a single boolean False, and the update-decision returns action=update whenever the PR is behind main. So a behind-main PR while Gitea is STILL COMPUTING mergeability (mergeable=None) hit /pulls/{n}/update -> dismiss_stale_approvals -> the exact rebase-churn this PR eliminates, fired during the compute window. Fix: make the None-vs-False distinction explicit (tri-state), do not collapse. - mergeable is True -> direct-merge (unchanged, this PR's fix). - mergeable is None -> WAIT: never update, never merge, never dismiss approvals; re-check next tick once Gitea computes. - mergeable is False -> definitive conflict -> existing update/hold path (409-on-update holds+advances per #2352). Merge bar and all #2352/#2356 behavior preserved. The prior None regression test passed only because its fixture had current base, masking the behind-main case; new tests exercise behind-main + None directly (proven to fail against the old collapse). pytest 65 green (62 + 3 new §SOP-22). Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/scripts/gitea-merge-queue.py | 58 ++++++++++++++----- .../scripts/tests/test_gitea_merge_queue.py | 51 ++++++++++++++++ 2 files changed, 93 insertions(+), 16 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 54c5c54d1..ceea9855a 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -21,10 +21,14 @@ queue. This script provides the missing serialized policy in user space: 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 NOT mergeable (mergeable - False/None) 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 is fail-closed (never merged). + 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 == @@ -634,7 +638,7 @@ 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 @@ -696,18 +700,32 @@ def evaluate_merge_readiness( # 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: + if mergeable is True: force = _non_required_red_present(latest, required_contexts) return MergeDecision(True, "merge", "ready", force=force) - # 6) NOT mergeable (mergeable is False/None — conflict or still computing). - # FAIL-CLOSED: never merge on an unknown. If the head also does not + # 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, or Gitea is still - # computing) — WAIT. + # 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)") @@ -1114,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( diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 4c1ef8a64..6b8dd9e8e 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -182,6 +182,33 @@ def test_current_base_but_not_mergeable_pr_waits(): 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) @@ -538,6 +565,30 @@ 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 -- 2.52.0