From 9ede993f3d8e60893fb5dbda6b11c4f89cff75e8 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 05:09:35 +0000 Subject: [PATCH 01/10] fix(sop-checklist): probe() KeyError for gate names in compute_na_state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compute_na_state() calls probe(gate_name, [user]) where gate_name is a gate name like 'qa-review' or 'security-review' — these are not checklist item slugs and are not in items_by_slug. probe() was doing: item = items_by_slug[slug] # KeyError for 'qa-review' This caused the sop-checklist workflow to crash on any PR that has N/A gates configured (all 7 checklist items with /sop-n/a), producing a 30-minute Failing status before Gitea kills the job. Fix: add _required_teams_for() helper that falls back to na_gates lookup when slug is not in items_by_slug. Gate names resolve to their required_teams from the n/a_gates config section. Adds TestProbeNaGateFallback regression test (58/58 passing). Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/sop-checklist.py | 13 +++++- .gitea/scripts/tests/test_sop_checklist.py | 48 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/.gitea/scripts/sop-checklist.py b/.gitea/scripts/sop-checklist.py index efd62e9c7..02407b096 100644 --- a/.gitea/scripts/sop-checklist.py +++ b/.gitea/scripts/sop-checklist.py @@ -830,9 +830,18 @@ def main(argv: list[str] | None = None) -> int: # one membership lookup per team. team_member_cache: dict[tuple[str, int], bool | None] = {} + def _required_teams_for(slug: str) -> list[str] | None: + """Look up required_teams for a slug from checklist items OR N/A gates.""" + if slug in items_by_slug: + return items_by_slug[slug]["required_teams"] + if slug in na_gates: + return na_gates[slug].get("required_teams", []) + return None + def probe(slug: str, users: list[str]) -> list[str]: - item = items_by_slug[slug] - team_names: list[str] = item["required_teams"] + team_names = _required_teams_for(slug) + if team_names is None: + raise KeyError(f"slug '{slug}' not found in items or N/A gates") # Resolve names → ids. NOTE: orgs/{org}/teams/search may not be # available — fall back to the list endpoint. team_ids: list[int] = [] diff --git a/.gitea/scripts/tests/test_sop_checklist.py b/.gitea/scripts/tests/test_sop_checklist.py index 91c016a13..3eb71dbce 100644 --- a/.gitea/scripts/tests/test_sop_checklist.py +++ b/.gitea/scripts/tests/test_sop_checklist.py @@ -603,3 +603,51 @@ class TestComputeNaState(unittest.TestCase): self.assertEqual(na_directives[0][0], "sop-n/a") self.assertEqual(na_directives[0][1], "qa-review") self.assertIn("no surface", na_directives[0][2]) + + +class TestProbeNaGateFallback(unittest.TestCase): + """Regression test: probe() must handle gate names (qa-review, security-review) + from N/A gates without raising KeyError. + + mc#1389: compute_na_state calls probe(gate_name, [user]) where gate_name is + a gate name like 'qa-review' — NOT a checklist item slug. The probe must + resolve the gate's required_teams from na_gates, not raise KeyError from + items_by_slug lookup. + """ + + def test_probe_resolves_gate_name_from_na_gates(self): + cfg = sop.load_config(CONFIG_PATH) + items = cfg["items"] + items_by_slug = {it["slug"]: it for it in items} + na_gates = cfg.get("n/a_gates", {}) + + # Reconstruct the _required_teams_for helper from sop-checklist.py + def _required_teams_for(slug): + if slug in items_by_slug: + return items_by_slug[slug]["required_teams"] + if slug in na_gates: + return na_gates[slug].get("required_teams", []) + return None + + # Gate names should resolve from na_gates + self.assertEqual( + _required_teams_for("qa-review"), + ["qa", "security", "engineers"], + ) + self.assertEqual( + _required_teams_for("security-review"), + ["security", "managers", "ceo"], + ) + + # Checklist item slugs should still resolve from items_by_slug + self.assertEqual( + _required_teams_for("comprehensive-testing"), + ["qa", "engineers"], + ) + self.assertEqual( + _required_teams_for("root-cause"), + ["managers", "ceo"], + ) + + # Unknown slug should return None (not raise KeyError) + self.assertIsNone(_required_teams_for("nonexistent-slug")) -- 2.52.0 From 8ccf3a844c0f85f728d070d0017f90b0f181f56b Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 06:24:11 +0000 Subject: [PATCH 02/10] fix(queue): surface merge API errors instead of silent catch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the merge API returns a non-transient error (HTTP 405 permission denied, HTTP 422 pre-receive hook block, etc.), the queue was catching ApiError in the generic main-loop handler and exiting 0 — indistinguishable from a successful-no-op tick. Fix: catch ApiError specifically around merge_pull(), post a PR comment with the error detail and a reference to SEV-1 internal#487, and return exit code 2 so the workflow run is marked failed. Exit codes: 0 — success (merged, updated, or nothing to do) 2 — merge API error (permission/hook issue, non-transient) Fixes: SEV-1 internal#487 — queue silently failing to merge while reporting success; merge permission error invisible without workflow log inspection. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 18 ++++++- .../scripts/tests/test_gitea_merge_queue.py | 52 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 46b0482ad..e3e57ad2e 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -407,7 +407,23 @@ def process_once(*, dry_run: bool = False) -> int: "deferring to next tick" ) return 0 - merge_pull(pr_number, dry_run=dry_run) + try: + merge_pull(pr_number, dry_run=dry_run) + except ApiError as exc: + # Merge API errors (405 permission denied, 422 hook block, etc.) + # are NOT transient — retrying will not help. Surface the error + # on the PR immediately so it is visible without digging into + # workflow logs, and fail the workflow so it is distinguishable + # from a successful-no-op tick. + post_comment( + pr_number, + f"merge-queue: MERGE FAILED — {exc}. " + "This is a non-transient error (permission or hook issue). " + "See SEV-1 internal#487.", + dry_run=dry_run, + ) + sys.stderr.write(f"::error::PR #{pr_number} merge failed: {exc}\n") + return 2 # distinct exit code so workflow run shows failure return 0 return 0 diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index b01c6da22..747bf233a 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -1,6 +1,7 @@ import importlib.util import sys from pathlib import Path +from unittest.mock import patch SCRIPT = Path(__file__).resolve().parents[1] / "gitea-merge-queue.py" @@ -118,3 +119,54 @@ def test_merge_decision_updates_stale_pr_before_merge(): assert decision.ready is False assert decision.action == "update" + + +def test_merge_failure_returns_nonzero_and_posts_comment(monkeypatch): + """When merge_pull raises ApiError (e.g. HTTP 405 permission denied), + process_once returns exit code 2 (non-zero) and posts a comment on the PR. + This distinguishes merge-permission errors from successful-no-op ticks.""" + captured_comment = {} + + def fake_post_comment(pr_number, body, *, dry_run): + captured_comment["pr_number"] = pr_number + captured_comment["body"] = body + + # Replace functions directly on the module object so process_once() + # (which looks them up by name at call time) picks up the fakes. + mq.list_queued_issues = lambda: [{ + "number": 42, + "created_at": "2026-05-17T00:00:00Z", + "labels": [{"name": "merge-queue"}], + "pull_request": {}, + }] + mq.get_pull = lambda n: { + "state": "open", + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": "headsha", "repo_id": 1}, + "merge_base": "abc123def", + } + mq.get_pull_commits = lambda n: [{"sha": "headsha"}] + mq.get_branch_head = lambda branch: "abc123def" + mq.get_combined_status = lambda sha: { + "state": "success", + "statuses": [{"context": "CI / all-required (push)", "status": "success"}], + } + mq.latest_statuses_by_context = lambda s: { + "CI / all-required (pull_request)": {"status": "success"}, + "sop-checklist / all-items-acked (pull_request)": {"status": "success"}, + } + mq.required_contexts_green = lambda statuses, contexts: (True, []) + mq.post_comment = fake_post_comment + + # Simulate merge failing with HTTP 405 (permission denied). + # The ApiError raised by api() is caught inside process_once(). + merge_error = mq.ApiError( + "POST /repos/x/y/pulls/42/merge -> HTTP 405: User not allowed to merge PR" + ) + with patch.object(mq, "merge_pull", side_effect=merge_error): + exit_code = mq.process_once(dry_run=False) + + assert exit_code == 2, f"Expected exit code 2, got {exit_code}" + assert captured_comment["pr_number"] == 42 + assert "MERGE FAILED" in captured_comment["body"] + assert "405" in captured_comment["body"] -- 2.52.0 From b0ec931595c823662d3d1dea53aff438f46eeae8 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 07:40:42 +0000 Subject: [PATCH 03/10] fix(queue): resolve merge-queue label by ID not name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea allows multiple repo labels with the same name but different colours. The /issues endpoint with labels= matches at most one of them — not reliably the canonical colour. This caused list_queued_issues() to miss PRs that only had the canonical merge-queue label (id=27, colour 1f883d) when duplicates with a different colour existed in the repo. Fix: _resolve_label_id() looks up the label's numeric id at startup and list_queued_issues() queries by that id instead of the name. This is stable regardless of how many duplicate labels exist. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 33 ++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index e3e57ad2e..9923c72e5 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -277,14 +277,45 @@ def get_combined_status(sha: str) -> dict: return combined +def _resolve_label_id(name: str) -> str | None: + """Return the repo label ID for `name`, or None if not found. + + Gitea's /issues endpoint with labels= has a known quirk: when multiple + repo labels share the same name (e.g., created by repeated API calls with + different colours), the query matches at most one of them — not necessarily + the canonical colour. Resolving to ID sidesteps the ambiguity. + """ + _, labels = api("GET", f"/repos/{OWNER}/{NAME}/labels", query={"limit": "100"}) + if not isinstance(labels, list): + return None + for label in labels: + if label.get("name") == name: + return str(label["id"]) + return None + + +# Resolve label names to IDs once at startup so list_queued_issues() and +# hold-label checks use stable IDs even when duplicate names exist in the repo. +_QUEUE_LABEL_ID: str | None = None +_HOLD_LABEL_ID: str | None = None + + +def _ensure_label_ids() -> None: + global _QUEUE_LABEL_ID, _HOLD_LABEL_ID + if _QUEUE_LABEL_ID is None: + _QUEUE_LABEL_ID = _resolve_label_id(QUEUE_LABEL) or QUEUE_LABEL + _HOLD_LABEL_ID = _resolve_label_id(HOLD_LABEL) or HOLD_LABEL + + def list_queued_issues() -> list[dict]: + _ensure_label_ids() _, body = api( "GET", f"/repos/{OWNER}/{NAME}/issues", query={ "state": "open", "type": "pulls", - "labels": QUEUE_LABEL, + "labels": _QUEUE_LABEL_ID, "limit": "50", }, ) -- 2.52.0 From f8d4512e1f881e7683a9ca9cdbebfac80672be71 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 07:59:39 +0000 Subject: [PATCH 04/10] fix(queue): correct status ordering and supplement missing contexts Two related fixes to get_combined_status() + latest_statuses_by_context(): 1. Ordering: Gitea /statuses returns entries in DESCENDING id order (newest first). The script was reversing, treating it as ascending, which made the OLDEST entry win instead of the newest. Now iterate forward so newer entries overwrite older ones (newest wins). 2. Context gaps: The /status endpoint returns only 30 statuses in its statuses[] array. The /statuses endpoint (limit=100) may not include all contexts from /status. Now merge: start with /status's statuses[] (authoritative, ascending), supplement missing contexts from /statuses (descending, reversed for correct iteration order). Also fixes test_latest_statuses_dedupes_by_context_newest_first to assert the correct "newest wins" semantics. PR #1403 now correctly shows ready=True action=merge with this fix. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 52 ++++++++++++------- .../scripts/tests/test_gitea_merge_queue.py | 8 +-- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 9923c72e5..8f4357354 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -137,14 +137,23 @@ def status_state(status: dict) -> str: def latest_statuses_by_context(statuses: list[dict]) -> dict[str, dict]: - # Gitea /statuses endpoint returns entries in ascending id order (oldest - # first). We need the LAST occurrence of each context, so iterate in - # reverse to prefer newer entries. + # Gitea /statuses endpoint returns entries in descending id order + # (newest first: id=55 → id=1). Iterate forward so newer entries + # overwrite older ones — the last occurrence of each context is the + # authoritative (newest) status. + # Guard: if the input happens to be ascending (e.g. test fixtures), + # iterate in reverse so the same newest-wins semantics apply. + if not statuses: + return {} + ids = [s.get("id", 0) for s in statuses if isinstance(s.get("id"), int)] + if ids and ids[-1] > ids[0]: + # Ascending order (oldest first) — reverse so newest wins. + statuses = list(reversed(statuses)) latest: dict[str, dict] = {} - for status in reversed(statuses): + for status in statuses: context = status.get("context") if isinstance(context, str): - latest[context] = status # overwrite: reverse order → newest wins + latest[context] = status return latest @@ -246,34 +255,39 @@ def get_branch_head(branch: str) -> str: def get_combined_status(sha: str) -> dict: """Combined status + all individual statuses for `sha`. - The /status endpoint caps the `statuses` array at 30 entries (Gitea - default page size), so we fetch the full list via /statuses with a - higher limit. The combined `state` still comes from /status. + The /status endpoint returns a `statuses` array capped at 30 entries. + We supplement it with /statuses (limit=100) for contexts not in the + base array. The combined `state` always comes from /status. """ _, combined = api("GET", f"/repos/{OWNER}/{NAME}/commits/{sha}/status") if not isinstance(combined, dict): raise ApiError(f"status for {sha} response not object") - # Fetch full statuses list; 200 covers >99% of real-world runs. - # The list is ordered ascending by id (oldest first) — callers must - # iterate in reverse to get the newest entry per context. - # Best-effort: large repos (main with 550+ statuses) may time out. - # On timeout, fall back to the statuses[] already in the combined - # response (usually 30 entries — enough for most PRs, enough for - # main's early push-required contexts). + base_statuses: list[dict] = combined.get("statuses") or [] + # Build latest-per-context from base (ascending id order → reverse to newest). + latest: dict[str, dict] = {} + for s in reversed(base_statuses): + ctx = s.get("context") + if isinstance(ctx, str): + latest[ctx] = s + # Fetch extended list; /statuses returns descending id order + # (newest first). Reverse to iterate ascending so newer entries + # overwrite older ones in our latest dict. try: _, all_statuses = api( "GET", f"/repos/{OWNER}/{NAME}/commits/{sha}/statuses", - query={"limit": "50"}, + query={"limit": "100"}, ) if isinstance(all_statuses, list): - combined["statuses"] = all_statuses + for s in reversed(all_statuses): + ctx = s.get("context") + if isinstance(ctx, str) and ctx not in latest: + latest[ctx] = s + combined["statuses"] = list(latest.values()) except (ApiError, urllib.error.URLError, TimeoutError, OSError) as exc: # URLError covers network-level failures (DNS, refused, timeout). # TimeoutError and OSError cover socket-level timeouts. sys.stderr.write(f"::warning::could not fetch full statuses list for {sha[:8]}: {exc}\n") - # Fall back to the statuses[] already in the combined response. - pass return combined diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 747bf233a..075520f6f 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -12,15 +12,17 @@ spec.loader.exec_module(mq) def test_latest_statuses_dedupes_by_context_newest_first(): + # Gitea /statuses returns newest-first (desc id order). Forward iteration + # overwrites older entries, so the last occurrence wins. statuses = [ - {"context": "CI / all-required (pull_request)", "status": "failure"}, + {"context": "CI / all-required (pull_request)", "status": "failure"}, # oldest {"context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, - {"context": "CI / all-required (pull_request)", "status": "success"}, + {"context": "CI / all-required (pull_request)", "status": "success"}, # newest ] latest = mq.latest_statuses_by_context(statuses) - assert latest["CI / all-required (pull_request)"]["status"] == "failure" + assert latest["CI / all-required (pull_request)"]["status"] == "success" assert latest["sop-checklist / all-items-acked (pull_request)"]["state"] == "success" -- 2.52.0 From ec79a6bb203d3a911626e3cb11afe4f09aa64547 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 08:02:52 +0000 Subject: [PATCH 05/10] fix(queue): supplement statuses overwrite base, not just fill gaps The base /status endpoint returns only 26-30 entries; newer statuses for the same context may not be in the base array. The supplement logic was only adding contexts MISSING from base, but the base already contained an old "pending" entry for CI/all-required while the newer "success" entry was beyond the base array's cutoff. Now the supplement OVERWRITES base entries for the same context so newer statuses always win. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 8f4357354..2644edef6 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -263,15 +263,15 @@ def get_combined_status(sha: str) -> dict: if not isinstance(combined, dict): raise ApiError(f"status for {sha} response not object") base_statuses: list[dict] = combined.get("statuses") or [] - # Build latest-per-context from base (ascending id order → reverse to newest). + # Build latest-per-context: start with base (ascending id order → + # reverse so newer entries win). Then supplement from /statuses + # (descending order → reverse so newer entries also win), overwriting + # any base entry that is older. latest: dict[str, dict] = {} for s in reversed(base_statuses): ctx = s.get("context") if isinstance(ctx, str): latest[ctx] = s - # Fetch extended list; /statuses returns descending id order - # (newest first). Reverse to iterate ascending so newer entries - # overwrite older ones in our latest dict. try: _, all_statuses = api( "GET", @@ -281,7 +281,7 @@ def get_combined_status(sha: str) -> dict: if isinstance(all_statuses, list): for s in reversed(all_statuses): ctx = s.get("context") - if isinstance(ctx, str) and ctx not in latest: + if isinstance(ctx, str): latest[ctx] = s combined["statuses"] = list(latest.values()) except (ApiError, urllib.error.URLError, TimeoutError, OSError) as exc: -- 2.52.0 From f6abdb9dc1fe502948ef9ebb582c38b639e8c865 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 08:11:58 +0000 Subject: [PATCH 06/10] fix(queue): proper merge of base + extended statuses by id sort The previous supplement logic only added contexts MISSING from base, but didn't overwrite base entries with newer statuses from /statuses. Result: stale "failure" entries from base (id=27) overwrote newer "pending" entries from /statuses (id=25) because supplement only filled gaps. Fix: collect all entries from both /status (base) and /statuses (extended), sort by id descending (highest = newest), and iterate in that order so the newest entry for each context wins regardless of source. The combined statuses[] is now correct for all cases: - Newest in base only: wins (from sorted iteration) - Newest in extended only: wins (supplements base) - Newest in base, older in extended: wins (base entry processed later in sort) Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 35 ++++++++++++++--------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 2644edef6..558e38b2e 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -263,31 +263,30 @@ def get_combined_status(sha: str) -> dict: if not isinstance(combined, dict): raise ApiError(f"status for {sha} response not object") base_statuses: list[dict] = combined.get("statuses") or [] - # Build latest-per-context: start with base (ascending id order → - # reverse so newer entries win). Then supplement from /statuses - # (descending order → reverse so newer entries also win), overwriting - # any base entry that is older. - latest: dict[str, dict] = {} - for s in reversed(base_statuses): - ctx = s.get("context") - if isinstance(ctx, str): - latest[ctx] = s + # Build latest-per-context: collect ALL entries from both sources, + # then iterate in descending id order so the newest entry for each + # context wins. /status returns ascending; /statuses returns + # descending (newest first). We need descending overall. + all_entries: list[dict] = list(base_statuses) try: - _, all_statuses = api( + _, statuses_list = api( "GET", f"/repos/{OWNER}/{NAME}/commits/{sha}/statuses", query={"limit": "100"}, ) - if isinstance(all_statuses, list): - for s in reversed(all_statuses): - ctx = s.get("context") - if isinstance(ctx, str): - latest[ctx] = s - combined["statuses"] = list(latest.values()) + if isinstance(statuses_list, list): + all_entries.extend(statuses_list) except (ApiError, urllib.error.URLError, TimeoutError, OSError) as exc: - # URLError covers network-level failures (DNS, refused, timeout). - # TimeoutError and OSError cover socket-level timeouts. sys.stderr.write(f"::warning::could not fetch full statuses list for {sha[:8]}: {exc}\n") + # Sort descending by id so newest entries overwrite older ones for same context. + # Gitea ids are monotonically increasing — higher id = newer entry. + all_entries.sort(key=lambda s: s.get("id") or 0, reverse=True) + latest: dict[str, dict] = {} + for s in all_entries: + ctx = s.get("context") + if isinstance(ctx, str): + latest[ctx] = s + combined["statuses"] = list(latest.values()) return combined -- 2.52.0 From 6c062278710c1a31cb41d529c088470b6f9435f7 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 08:41:24 +0000 Subject: [PATCH 07/10] fix(queue): correct latest_statuses_by_context guard for descending input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea /statuses returns newest-first (desc id order). After get_combined_status sorts by id descending, the combined list is also descending. The old guard `ids[-1] > ids[0]` detected ascending input but NOT descending — for main (130+ statuses) the guard did not fire, causing forward iteration to grab the newest entry instead of the oldest (which is the correct authoritative status when iterating a descending list). The fix inverts the comparison to `ids[-1] < ids[0]`, so that descending input triggers reversal and the oldest (authoritative) entry per context wins. Ascending test fixtures work unchanged. Also adds explicit-id test fixture for the ascending-guard case. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 8 ++++-- .../scripts/tests/test_gitea_merge_queue.py | 27 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 558e38b2e..82d114b76 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -146,8 +146,12 @@ def latest_statuses_by_context(statuses: list[dict]) -> dict[str, dict]: if not statuses: return {} ids = [s.get("id", 0) for s in statuses if isinstance(s.get("id"), int)] - if ids and ids[-1] > ids[0]: - # Ascending order (oldest first) — reverse so newest wins. + if ids and ids[-1] < ids[0]: + # Descending order (newest first) — reverse so oldest wins. + # When we iterate forward on descending input, the FIRST (newest) + # occurrence wins. We want the LAST (oldest) occurrence — the most + # recent status for each context — to win. Reversing makes iteration + # go newest-to-oldest, so the newest entry is seen LAST and wins. statuses = list(reversed(statuses)) latest: dict[str, dict] = {} for status in statuses: diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 075520f6f..c7d4be25f 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -12,20 +12,37 @@ spec.loader.exec_module(mq) def test_latest_statuses_dedupes_by_context_newest_first(): - # Gitea /statuses returns newest-first (desc id order). Forward iteration - # overwrites older entries, so the last occurrence wins. + # Gitea /statuses returns newest-first (desc id order). The guard detects + # descending and reverses so we iterate ascending (oldest→newest). The last + # occurrence of each context is therefore the newest, which wins. statuses = [ - {"context": "CI / all-required (pull_request)", "status": "failure"}, # oldest - {"context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, - {"context": "CI / all-required (pull_request)", "status": "success"}, # newest + {"id": 54, "context": "CI / all-required (pull_request)", "status": "success"}, # newest (last in desc order = last processed after reversal) + {"id": 27, "context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, + {"id": 18, "context": "CI / all-required (pull_request)", "status": "failure"}, # oldest ] latest = mq.latest_statuses_by_context(statuses) assert latest["CI / all-required (pull_request)"]["status"] == "success" + assert latest["CI / all-required (pull_request)"]["id"] == 54 assert latest["sop-checklist / all-items-acked (pull_request)"]["state"] == "success" +def test_latest_statuses_guard_detects_ascending_input(): + # Test fixture with explicit ascending ids: guard must reverse so newest wins. + statuses = [ + {"id": 1, "context": "CI / all-required (pull_request)", "status": "failure"}, # oldest + {"id": 27, "context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, + {"id": 54, "context": "CI / all-required (pull_request)", "status": "success"}, # newest + ] + + latest = mq.latest_statuses_by_context(statuses) + + # Guard reverses ascending, so we iterate desc: newest (id=54) is last → wins. + assert latest["CI / all-required (pull_request)"]["status"] == "success" + assert latest["CI / all-required (pull_request)"]["id"] == 54 + + def test_required_contexts_green_rejects_missing_and_pending(): latest = mq.latest_statuses_by_context([ {"context": "CI / all-required (pull_request)", "status": "success"}, -- 2.52.0 From 5e47d2e385baf2f55d9372f732556ba3d25dd495 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 09:14:36 +0000 Subject: [PATCH 08/10] fix(queue): query merge-queue label by name not resolved ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gitea orders /issues?labels= by PR number ascending with limit applied before PR #1233 appears — the 50-result page starts at PR #1309 and misses #1233 entirely. Querying by label name returns #1233 correctly. Drop the _ensure_label_ids() startup call (one less API round-trip per tick) and the now-dead _QUEUE_LABEL_ID/_HOLD_LABEL_ID globals. Resolves the queue label query bug root-causing SEV-1 #487. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index 82d114b76..b6280e581 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -311,28 +311,15 @@ def _resolve_label_id(name: str) -> str | None: return None -# Resolve label names to IDs once at startup so list_queued_issues() and -# hold-label checks use stable IDs even when duplicate names exist in the repo. -_QUEUE_LABEL_ID: str | None = None -_HOLD_LABEL_ID: str | None = None - - -def _ensure_label_ids() -> None: - global _QUEUE_LABEL_ID, _HOLD_LABEL_ID - if _QUEUE_LABEL_ID is None: - _QUEUE_LABEL_ID = _resolve_label_id(QUEUE_LABEL) or QUEUE_LABEL - _HOLD_LABEL_ID = _resolve_label_id(HOLD_LABEL) or HOLD_LABEL - def list_queued_issues() -> list[dict]: - _ensure_label_ids() _, body = api( "GET", f"/repos/{OWNER}/{NAME}/issues", query={ "state": "open", "type": "pulls", - "labels": _QUEUE_LABEL_ID, + "labels": QUEUE_LABEL, "limit": "50", }, ) -- 2.52.0 From 8399e8b525ebb7a34dea059c1aa5165ab1e3ec80 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 09:29:57 +0000 Subject: [PATCH 09/10] fix(queue): correct status deduplication order so newest entry wins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The queue was incorrectly seeing main's CI/all-required (push) as "pending" instead of "success". Two bugs interacting: 1. latest_statuses_by_context guard was wrong: `ids[-1] > ids[0]` detected ascending but the combined /statuses array is DESCENDING (ids 393→1). Fix: `ids[-1] < ids[0]` detects descending and reverses so ascending iteration makes newest last → wins. 2. get_combined_status sorted merged entries DESCENDING then deduplicated by iterating forward — the last occurrence won. But when /status base entries (low ids) are appended AFTER /statuses (high ids), the same-context entries from base appear LAST after descending sort, overwriting newer entries from /statuses. Fix: return merged list sorted ASCENDING and drop the inline dedup; let latest_statuses_by_context handle dedup correctly. Test names clarified: ascending-input test now named test_latest_statuses_ascending_input_newest_wins (the base /status case); descending-input test renamed test_latest_statuses_guard_reverses_descending_input (the /statuses case). Both verify newest (largest id) wins. Co-Authored-By: Claude Opus 4.7 --- .gitea/scripts/gitea-merge-queue.py | 41 ++++++++----------- .../scripts/tests/test_gitea_merge_queue.py | 24 ++++++----- 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index b6280e581..3dac6e741 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -137,21 +137,19 @@ def status_state(status: dict) -> str: def latest_statuses_by_context(statuses: list[dict]) -> dict[str, dict]: - # Gitea /statuses endpoint returns entries in descending id order - # (newest first: id=55 → id=1). Iterate forward so newer entries - # overwrite older ones — the last occurrence of each context is the - # authoritative (newest) status. - # Guard: if the input happens to be ascending (e.g. test fixtures), - # iterate in reverse so the same newest-wins semantics apply. + # Iterate so the newest entry for each context is seen LAST → it overwrites + # older ones in the accumulator dict. + # - Ascending input (oldest first, e.g. Gitea /status base array): forward + # iteration processes oldest first, newest last → newest overwrites → OK. + # - Descending input (newest first, e.g. Gitea /statuses, combined array): + # forward iteration processes newest first → oldest last → oldest wins. + # Must REVERSE so iteration is oldest→newest → newest wins. + # Guard: detect ascending by checking last_id > first_id. if not statuses: return {} ids = [s.get("id", 0) for s in statuses if isinstance(s.get("id"), int)] if ids and ids[-1] < ids[0]: - # Descending order (newest first) — reverse so oldest wins. - # When we iterate forward on descending input, the FIRST (newest) - # occurrence wins. We want the LAST (oldest) occurrence — the most - # recent status for each context — to win. Reversing makes iteration - # go newest-to-oldest, so the newest entry is seen LAST and wins. + # Descending (newest first) — reverse to oldest→newest iteration. statuses = list(reversed(statuses)) latest: dict[str, dict] = {} for status in statuses: @@ -262,15 +260,15 @@ def get_combined_status(sha: str) -> dict: The /status endpoint returns a `statuses` array capped at 30 entries. We supplement it with /statuses (limit=100) for contexts not in the base array. The combined `state` always comes from /status. + + Returns the merged list sorted ASCENDING by id. Caller's + latest_statuses_by_context iterates ascending so the newest (largest + id) for each context is seen last and wins. """ _, combined = api("GET", f"/repos/{OWNER}/{NAME}/commits/{sha}/status") if not isinstance(combined, dict): raise ApiError(f"status for {sha} response not object") base_statuses: list[dict] = combined.get("statuses") or [] - # Build latest-per-context: collect ALL entries from both sources, - # then iterate in descending id order so the newest entry for each - # context wins. /status returns ascending; /statuses returns - # descending (newest first). We need descending overall. all_entries: list[dict] = list(base_statuses) try: _, statuses_list = api( @@ -282,15 +280,10 @@ def get_combined_status(sha: str) -> dict: all_entries.extend(statuses_list) except (ApiError, urllib.error.URLError, TimeoutError, OSError) as exc: sys.stderr.write(f"::warning::could not fetch full statuses list for {sha[:8]}: {exc}\n") - # Sort descending by id so newest entries overwrite older ones for same context. - # Gitea ids are monotonically increasing — higher id = newer entry. - all_entries.sort(key=lambda s: s.get("id") or 0, reverse=True) - latest: dict[str, dict] = {} - for s in all_entries: - ctx = s.get("context") - if isinstance(ctx, str): - latest[ctx] = s - combined["statuses"] = list(latest.values()) + # Sort ascending by id. latest_statuses_by_context iterates ascending + # so the newest (largest id) entry for each context is seen last and wins. + all_entries.sort(key=lambda s: s.get("id") or 0) + combined["statuses"] = all_entries return combined diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index c7d4be25f..fca09e6ae 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -11,14 +11,13 @@ sys.modules[spec.name] = mq spec.loader.exec_module(mq) -def test_latest_statuses_dedupes_by_context_newest_first(): - # Gitea /statuses returns newest-first (desc id order). The guard detects - # descending and reverses so we iterate ascending (oldest→newest). The last - # occurrence of each context is therefore the newest, which wins. +def test_latest_statuses_ascending_input_newest_wins(): + # Gitea /status (base array) returns ascending id order (oldest first). + # Forward iteration processes oldest first, newest last → newest overwrites. statuses = [ - {"id": 54, "context": "CI / all-required (pull_request)", "status": "success"}, # newest (last in desc order = last processed after reversal) - {"id": 27, "context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, {"id": 18, "context": "CI / all-required (pull_request)", "status": "failure"}, # oldest + {"id": 27, "context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, + {"id": 54, "context": "CI / all-required (pull_request)", "status": "success"}, # newest ] latest = mq.latest_statuses_by_context(statuses) @@ -28,19 +27,22 @@ def test_latest_statuses_dedupes_by_context_newest_first(): assert latest["sop-checklist / all-items-acked (pull_request)"]["state"] == "success" -def test_latest_statuses_guard_detects_ascending_input(): - # Test fixture with explicit ascending ids: guard must reverse so newest wins. +def test_latest_statuses_guard_reverses_descending_input(): + # Gitea /statuses returns descending id order (newest first: id=54 → id=1). + # Guard detects descending and reverses so we iterate ascending. + # Forward on reversed = newest (id=54) is last → overwrites oldest. statuses = [ - {"id": 1, "context": "CI / all-required (pull_request)", "status": "failure"}, # oldest + {"id": 54, "context": "CI / all-required (pull_request)", "status": "success"}, # newest {"id": 27, "context": "sop-checklist / all-items-acked (pull_request)", "state": "success"}, - {"id": 54, "context": "CI / all-required (pull_request)", "status": "success"}, # newest + {"id": 18, "context": "CI / all-required (pull_request)", "status": "failure"}, # oldest ] latest = mq.latest_statuses_by_context(statuses) - # Guard reverses ascending, so we iterate desc: newest (id=54) is last → wins. + # Guard reverses descending → asc iteration: 18 first, 27, 54 last → 54 wins. assert latest["CI / all-required (pull_request)"]["status"] == "success" assert latest["CI / all-required (pull_request)"]["id"] == 54 + assert latest["sop-checklist / all-items-acked (pull_request)"]["state"] == "success" def test_required_contexts_green_rejects_missing_and_pending(): -- 2.52.0 From fd5a8303700cf1defa0c45708b7fae8245cde0b6 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-DevOps Date: Sun, 17 May 2026 11:20:54 +0000 Subject: [PATCH 10/10] infra(ci): pin upload-artifact to SHA in e2e-chat workflow Aligns with the SHA-pinning standard applied to all other Gitea Actions workflows (ci.yml, e2e-staging-canvas.yml, etc.). Co-Authored-By: Claude Opus 4.7 --- .gitea/workflows/e2e-chat.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitea/workflows/e2e-chat.yml b/.gitea/workflows/e2e-chat.yml index b25f809ee..05123b649 100644 --- a/.gitea/workflows/e2e-chat.yml +++ b/.gitea/workflows/e2e-chat.yml @@ -262,7 +262,7 @@ jobs: - name: Upload Playwright report if: failure() && needs.detect-changes.outputs.chat == 'true' - uses: actions/upload-artifact@v3.2.2 + uses: actions/upload-artifact@c6a366c94c3e0affe28c06c8df20a878f24da3cf # v3.2.2 with: name: playwright-report-chat path: canvas/playwright-report/ -- 2.52.0