From 114ee8c71535530229e29124c6b172156f45bff3 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Sun, 7 Jun 2026 13:32:15 +0000 Subject: [PATCH] fix(merge-control): harden ci-required-drift.py + gitea-merge-queue.py ci-required-drift.py: - Add fail-closed validation of REQUIRED_CHECKS_JSON array items: reject non-string, empty/whitespace, and duplicate contexts. - Prevents malformed audit manifests from being silently normalized (hiding config errors). - Add regression tests for non-string, empty, and duplicate cases. gitea-merge-queue.py: - Add enumerate_readiness() to evaluate ALL candidates and return their readiness states without stopping at the first actionable one. - Add print_post_batch_summary() for structured operator visibility (counts + per-PR state/action/reason). - Add --enumerate CLI flag for report-only batch evaluation. - Fail-closed: API errors per candidate are recorded as unverifiable rather than skipped silently. - Add tests for full-candidate evaluation, ineligible detection, and fail-closed API-error handling. Refs dropped dispatch 3b16f607. --- .gitea/scripts/ci-required-drift.py | 28 +++- .gitea/scripts/gitea-merge-queue.py | 110 ++++++++++++++++ .../scripts/tests/test_ci_required_drift.py | 30 +++++ .../scripts/tests/test_gitea_merge_queue.py | 123 ++++++++++++++++++ 4 files changed, 290 insertions(+), 1 deletion(-) diff --git a/.gitea/scripts/ci-required-drift.py b/.gitea/scripts/ci-required-drift.py index d9739129b..eed421f83 100755 --- a/.gitea/scripts/ci-required-drift.py +++ b/.gitea/scripts/ci-required-drift.py @@ -317,7 +317,33 @@ def required_checks_env(audit_doc: dict, branch: str) -> set[str]: f"::error::REQUIRED_CHECKS_JSON['{branch}'] is {type(branch_checks).__name__}, expected list\n" ) sys.exit(3) - return {str(item).strip() for item in branch_checks if str(item).strip()} + # Fail-closed validation: every entry must be a non-empty string. + # Reject null, int, dict, or empty/whitespace strings silently — + # they indicate a malformed manifest that drift-detect must not + # normalize away (that would hide config errors). + validated: set[str] = set() + for idx, item in enumerate(branch_checks): + if not isinstance(item, str): + sys.stderr.write( + f"::error::REQUIRED_CHECKS_JSON['{branch}'][{idx}] is " + f"{type(item).__name__} (value={item!r}), expected str\n" + ) + sys.exit(3) + stripped = item.strip() + if not stripped: + sys.stderr.write( + f"::error::REQUIRED_CHECKS_JSON['{branch}'][{idx}] is " + f"empty/whitespace string\n" + ) + sys.exit(3) + if stripped in validated: + sys.stderr.write( + f"::error::REQUIRED_CHECKS_JSON['{branch}'] contains " + f"duplicate context '{stripped}' at index {idx}\n" + ) + sys.exit(3) + validated.add(stripped) + return validated # Legacy variant fallback. if found_legacy: diff --git a/.gitea/scripts/gitea-merge-queue.py b/.gitea/scripts/gitea-merge-queue.py index ceea9855a..8c8ff3ec2 100644 --- a/.gitea/scripts/gitea-merge-queue.py +++ b/.gitea/scripts/gitea-merge-queue.py @@ -1166,12 +1166,122 @@ def _evaluate_candidate( return decision, ctx +@dataclasses.dataclass(frozen=True) +class ReadinessEntry: + """One candidate's readiness state.""" + + pr_number: int + decision: MergeDecision | None + reason: str + + +def enumerate_readiness(*, dry_run: bool = False) -> list[ReadinessEntry]: + """Evaluate ALL candidates and return their readiness states. + + Fail-closed: if branch protection cannot be fetched, raise + BranchProtectionUnavailable (caller must handle). Unlike + process_once, this does NOT stop at the first actionable candidate; + it evaluates every eligible PR and returns the full list so a + post-batch summary can be printed. + """ + bp = get_branch_protection(WATCH_BRANCH) + contexts = bp.required_contexts + required_approvals = bp.required_approvals + + main_sha = get_branch_head(WATCH_BRANCH) + main_status = get_combined_status(main_sha) + main_latest = latest_statuses_by_context(main_status.get("statuses") or []) + main_ok, main_bad = required_contexts_green(main_latest, push_required_contexts()) + + candidates = choose_candidate_issues( + list_candidate_issues(auto_discover=AUTO_DISCOVER), + queue_label=QUEUE_LABEL, + opt_out_labels=OPT_OUT_LABELS, + auto_discover=AUTO_DISCOVER, + ) + + entries: list[ReadinessEntry] = [] + for issue in candidates: + pr_number = int(issue["number"]) + try: + decision, ctx = _evaluate_candidate( + issue, + main_sha=main_sha, + main_status=main_status, + required_contexts=contexts, + required_approvals=required_approvals, + dry_run=dry_run, + ) + except ApiError as exc: + # Fail-closed per candidate: an unreadable PR is recorded as + # unverifiable, not skipped silently. + entries.append( + ReadinessEntry( + pr_number=pr_number, + decision=None, + reason=f"unverifiable (API error: {exc})", + ) + ) + continue + if decision is None: + entries.append( + ReadinessEntry( + pr_number=pr_number, + decision=None, + reason="not merge-eligible (opt-out/draft/fork/wrong-base)", + ) + ) + continue + entries.append( + ReadinessEntry( + pr_number=pr_number, + decision=decision, + reason=decision.reason, + ) + ) + return entries + + +def print_post_batch_summary(entries: list[ReadinessEntry]) -> None: + """Print a structured summary of all candidates' readiness. + + Emits ::notice:: lines for machine parsing and a human-readable + block for operator visibility. + """ + ready = [e for e in entries if e.decision and e.decision.ready] + waiting = [e for e in entries if e.decision and not e.decision.ready] + ineligible = [e for e in entries if e.decision is None] + + print("::group::merge-queue readiness summary") + print(f"total_candidates={len(entries)}") + print(f"ready={len(ready)}") + print(f"waiting={len(waiting)}") + print(f"ineligible/unverifiable={len(ineligible)}") + print("") + for e in entries: + state = "ready" if e.decision and e.decision.ready else ( + "waiting" if e.decision else "ineligible" + ) + action = e.decision.action if e.decision else "n/a" + print(f"PR #{e.pr_number}: state={state} action={action} reason={e.reason}") + print("::endgroup::") + + def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--dry-run", action="store_true") + parser.add_argument( + "--enumerate", + action="store_true", + help="Evaluate all candidates and print a readiness summary without merging.", + ) args = parser.parse_args() _require_runtime_env() try: + if args.enumerate: + entries = enumerate_readiness(dry_run=args.dry_run) + print_post_batch_summary(entries) + return 0 return process_once(dry_run=args.dry_run) except ApiError as exc: # FAIL-CLOSED: API errors are not "transient success" — they mean diff --git a/.gitea/scripts/tests/test_ci_required_drift.py b/.gitea/scripts/tests/test_ci_required_drift.py index 99a50da9b..b74f91961 100644 --- a/.gitea/scripts/tests/test_ci_required_drift.py +++ b/.gitea/scripts/tests/test_ci_required_drift.py @@ -107,6 +107,36 @@ def test_required_checks_env_json_malformed_fails(): raise AssertionError("expected SystemExit(3)") +def test_required_checks_env_json_non_string_item_fails(): + doc = _make_audit_doc_json({"main": ["ctx-a", 123, "ctx-b"]}) + try: + drift.required_checks_env(doc, "main") + except SystemExit as exc: + assert exc.code == 3 + else: + raise AssertionError("expected SystemExit(3)") + + +def test_required_checks_env_json_empty_string_item_fails(): + doc = _make_audit_doc_json({"main": ["ctx-a", " ", "ctx-b"]}) + try: + drift.required_checks_env(doc, "main") + except SystemExit as exc: + assert exc.code == 3 + else: + raise AssertionError("expected SystemExit(3)") + + +def test_required_checks_env_json_duplicate_context_fails(): + doc = _make_audit_doc_json({"main": ["ctx-a", "ctx-b", "ctx-a"]}) + try: + drift.required_checks_env(doc, "main") + except SystemExit as exc: + assert exc.code == 3 + else: + raise AssertionError("expected SystemExit(3)") + + # --------------------------------------------------------------------------- # sentinel_needs # --------------------------------------------------------------------------- diff --git a/.gitea/scripts/tests/test_gitea_merge_queue.py b/.gitea/scripts/tests/test_gitea_merge_queue.py index 6b8dd9e8e..a47980725 100644 --- a/.gitea/scripts/tests/test_gitea_merge_queue.py +++ b/.gitea/scripts/tests/test_gitea_merge_queue.py @@ -1563,3 +1563,126 @@ def test_process_once_defensive_skip_when_pull_payload_opted_out(monkeypatch): assert rc == 0 assert calls["merged"] is None + + +# --------------------------------------------------------------------------- +# readiness-enumeration + post-batch summary +# --------------------------------------------------------------------------- + +def test_enumerate_readiness_evaluates_all_candidates(monkeypatch): + """enumerate_readiness returns every candidate's state, not stopping at + the first actionable one.""" + old_head, new_head = "a" * 40, "c" * 40 + _wire_multi_candidate_process_once( + monkeypatch, + issues=[ + _issue(500, labels=[], created="2026-06-01T01:00:00Z"), + _issue(501, labels=[], created="2026-06-01T02:00:00Z"), + ], + pulls={ + 500: {"state": "open", "mergeable": False, "draft": False, + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": old_head, "repo_id": 1}, "labels": []}, + 501: {"state": "open", "mergeable": True, "draft": False, + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": new_head, "repo_id": 1}, "labels": []}, + }, + reviews={500: _two_approvals(old_head), 501: _two_approvals(new_head)}, + calls={}, + ) + + entries = mq.enumerate_readiness(dry_run=False) + + assert len(entries) == 2 + by_num = {e.pr_number: e for e in entries} + assert by_num[500].decision is not None + assert by_num[500].decision.ready is False + assert by_num[501].decision is not None + assert by_num[501].decision.ready is True + + +def test_enumerate_readiness_includes_ineligible_pr(monkeypatch): + """enumerate_readiness marks fork / wrong-base PRs as ineligible + (decision=None) while still evaluating the rest.""" + head = "a" * 40 + _wire_multi_candidate_process_once( + monkeypatch, + issues=[ + _issue(600, labels=[], created="2026-06-01T01:00:00Z"), + _issue(601, labels=[], created="2026-06-01T02:00:00Z"), + ], + pulls={ + 600: {"state": "open", "mergeable": True, "draft": False, + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": head, "repo_id": 2}, "labels": []}, # fork + 601: {"state": "open", "mergeable": True, "draft": False, + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": head, "repo_id": 1}, "labels": []}, + }, + reviews={600: _two_approvals(head), 601: _two_approvals(head)}, + calls={}, + ) + + entries = mq.enumerate_readiness(dry_run=False) + + by_num = {e.pr_number: e for e in entries} + assert by_num[600].decision is None + assert "not merge-eligible" in by_num[600].reason + assert by_num[601].decision is not None + assert by_num[601].decision.ready is True + + +def test_enumerate_readiness_fail_closed_on_api_error(monkeypatch): + """If get_pull raises for one candidate, that candidate is recorded as + unverifiable; other candidates are still evaluated.""" + head = "a" * 40 + _wire_multi_candidate_process_once( + monkeypatch, + issues=[ + _issue(700, labels=[], created="2026-06-01T01:00:00Z"), + _issue(701, labels=[], created="2026-06-01T02:00:00Z"), + ], + pulls={ + 700: {"state": "open", "mergeable": True, "draft": False, + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": head, "repo_id": 1}, "labels": []}, + 701: {"state": "open", "mergeable": True, "draft": False, + "base": {"ref": "main", "repo_id": 1}, + "head": {"sha": head, "repo_id": 1}, "labels": []}, + }, + reviews={700: _two_approvals(head), 701: _two_approvals(head)}, + calls={}, + ) + + original_get_pull = mq.get_pull + def failing_get_pull(n): + if n == 700: + raise mq.ApiError("simulated API failure") + return original_get_pull(n) + monkeypatch.setattr(mq, "get_pull", failing_get_pull) + + entries = mq.enumerate_readiness(dry_run=False) + + by_num = {e.pr_number: e for e in entries} + assert by_num[700].decision is None + assert "unverifiable" in by_num[700].reason + assert by_num[701].decision is not None + assert by_num[701].decision.ready is True + + +def test_print_post_batch_summary_counts_correctly(capsys): + entries = [ + mq.ReadinessEntry(pr_number=1, decision=mq.MergeDecision(True, "merge", "ready"), reason="ready"), + mq.ReadinessEntry(pr_number=2, decision=mq.MergeDecision(False, "wait", "CI red"), reason="CI red"), + mq.ReadinessEntry(pr_number=3, decision=None, reason="draft"), + ] + mq.print_post_batch_summary(entries) + captured = capsys.readouterr() + out = captured.out + assert "total_candidates=3" in out + assert "ready=1" in out + assert "waiting=1" in out + assert "ineligible/unverifiable=1" in out + assert "PR #1: state=ready" in out + assert "PR #2: state=waiting" in out + assert "PR #3: state=ineligible" in out -- 2.52.0