From 9f12bf50ca2bffd04fc01f4b1d28f07476f16997 Mon Sep 17 00:00:00 2001 From: aglichandrap Date: Wed, 27 May 2026 16:10:05 +0700 Subject: [PATCH 1/3] fix: remove check_suite/push/label from webhook event routing Events 'check_suite', 'push', and 'label' were being routed to _handle_accepted_issue_label(), which expects 'issues' or 'pull_request' payload structures. These event types don't have 'issue' or 'pull_request' top-level fields, so the handler always returned 'missing_issue' status. - 'check_suite' events have: action, check_suite, repository, sender - 'push' events have: ref, repository, pusher, commits - 'label' events are for label CRUD (create/edit/delete), not for labeling issues/PRs (those fire as 'issues' events with action='labeled') Fix: Only route 'issues' and 'pull_request' events to the label handler. Other event types are now properly ignored. --- app/webhooks/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/webhooks/github.py b/app/webhooks/github.py index 145085d9..9dd26d01 100644 --- a/app/webhooks/github.py +++ b/app/webhooks/github.py @@ -280,7 +280,7 @@ def handle_github_webhook( if not isinstance(payload, dict): _record_event(database_url, delivery_id, event_type, hashed, "invalid_payload") return {"status": "invalid_payload"} - if event_type in {"issues", "pull_request", "label", "check_suite", "push"}: + if event_type in {"issues", "pull_request"}: return _handle_accepted_issue_label( database_url, payload, event_type, delivery_id, hashed, accepted_labelers ) From 50d1916060c9500c86734cfd8098ff1dff738667 Mon Sep 17 00:00:00 2001 From: aglichandrap Date: Wed, 27 May 2026 18:06:00 +0700 Subject: [PATCH 2/3] test: add regression test for webhook event routing (PR #510) Verify that check_suite and push events are recorded as 'ignored' (not routed to the label handler), while issues and pull_request events continue to route correctly. --- tests/test_webhook_event_routing.py | 150 ++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 tests/test_webhook_event_routing.py diff --git a/tests/test_webhook_event_routing.py b/tests/test_webhook_event_routing.py new file mode 100644 index 00000000..9ad5a7ba --- /dev/null +++ b/tests/test_webhook_event_routing.py @@ -0,0 +1,150 @@ +"""Regression test for PR #510: only 'issues' and 'pull_request' events route +to the label handler. Previously 'check_suite', 'push', and 'label' were also +routed, which caused unhandled-key errors in the label handler. + +This test ensures those events are recorded as 'ignored' while the two +supported event types continue to work. +""" +from __future__ import annotations + +import hashlib +import hmac +import json + +from app.db import create_schema, session_scope +from app.ledger.service import create_bounty, ensure_genesis, get_balance +from app.models import WebhookEvent +from app.webhooks.github import handle_github_webhook + + +SECRET = "test-secret" + + +def _sig(body: bytes) -> str: + return f"sha256={hmac.new(SECRET.encode(), body, hashlib.sha256).hexdigest()}" + + +def _wrap(event_type: str, payload: dict, delivery_id: str) -> tuple[dict, bytes]: + body = json.dumps(payload, separators=(",", ":")).encode() + headers = { + "X-GitHub-Delivery": delivery_id, + "X-GitHub-Event": event_type, + "X-Hub-Signature-256": _sig(body), + } + return headers, body + + +# -- events that must be ignored (PR #510 regression) ----------------------- + + +def test_check_suite_event_is_ignored(sqlite_url: str) -> None: + """check_suite must NOT be routed to the label handler.""" + create_schema(sqlite_url) + payload = { + "action": "completed", + "check_suite": {"id": 1}, + "repository": {"full_name": "ramimbo/mergework"}, + "sender": {"login": "bot"}, + } + headers, body = _wrap("check_suite", payload, "delivery-check-suite") + + result = handle_github_webhook(sqlite_url, headers, body, SECRET) + + assert result == {"status": "ignored"} + with session_scope(sqlite_url) as session: + event = session.get(WebhookEvent, "delivery-check-suite") + assert event is not None + assert event.processed_status == "ignored" + + +def test_push_event_is_ignored(sqlite_url: str) -> None: + """push must NOT be routed to the label handler.""" + create_schema(sqlite_url) + payload = { + "ref": "refs/heads/main", + "repository": {"full_name": "ramimbo/mergework"}, + "sender": {"login": "dev"}, + } + headers, body = _wrap("push", payload, "delivery-push") + + result = handle_github_webhook(sqlite_url, headers, body, SECRET) + + assert result == {"status": "ignored"} + with session_scope(sqlite_url) as session: + event = session.get(WebhookEvent, "delivery-push") + assert event is not None + assert event.processed_status == "ignored" + + +# -- events that must still route to the label handler ---------------------- + + +def test_issues_event_routes_to_label_handler(sqlite_url: str) -> None: + """'issues' events must still be processed by the label handler.""" + create_schema(sqlite_url) + payload = { + "action": "labeled", + "label": {"name": "mrwk:accepted"}, + "issue": { + "number": 100, + "html_url": "https://github.com/ramimbo/mergework/issues/100", + "user": {"login": "contributor"}, + }, + "repository": {"full_name": "ramimbo/mergework"}, + "sender": {"login": "maintainer"}, + } + headers, body = _wrap("issues", payload, "delivery-issues-route") + + with session_scope(sqlite_url) as session: + ensure_genesis(session) + create_bounty( + session, + repo="ramimbo/mergework", + issue_number=100, + issue_url="https://github.com/ramimbo/mergework/issues/100", + title="Routing test bounty", + reward_mrwk="50", + acceptance="Maintainer applies mrwk:accepted", + ) + + result = handle_github_webhook(sqlite_url, headers, body, SECRET) + + assert result["status"] == "paid" + with session_scope(sqlite_url) as session: + assert get_balance(session, "github:contributor") == 50_000_000 + + +def test_pull_request_event_routes_to_label_handler(sqlite_url: str) -> None: + """'pull_request' events must still be processed by the label handler.""" + create_schema(sqlite_url) + payload = { + "action": "labeled", + "label": {"name": "mrwk:accepted"}, + "pull_request": { + "number": 10, + "html_url": "https://github.com/ramimbo/mergework/pull/10", + "body": "Closes #101", + "user": {"login": "contributor"}, + }, + "repository": {"full_name": "ramimbo/mergework"}, + "sender": {"login": "maintainer"}, + } + headers, body = _wrap("pull_request", payload, "delivery-pr-route") + + with session_scope(sqlite_url) as session: + ensure_genesis(session) + create_bounty( + session, + repo="ramimbo/mergework", + issue_number=101, + issue_url="https://github.com/ramimbo/mergework/issues/101", + title="PR routing test bounty", + reward_mrwk="75", + acceptance="Accepted PR closes the bounty.", + ) + + result = handle_github_webhook(sqlite_url, headers, body, SECRET) + + assert result["status"] == "paid" + with session_scope(sqlite_url) as session: + assert get_balance(session, "github:contributor") == 75_000_000 From e437a7fcb64847f9d7390f5f5ceb421ddd7ce2bd Mon Sep 17 00:00:00 2001 From: Agli Chandra Pratama Date: Sat, 30 May 2026 07:12:49 +0700 Subject: [PATCH 3/3] fix: add label event test and fix ruff formatting - Add test_label_event_is_ignored to cover the 'label' event type regression path (previously unrouted, now recorded as 'ignored') - Fix import sorting per ruff I001 - Fix whitespace/formatting per ruff format Addresses review feedback from Zejia and eliasx45. --- tests/test_webhook_event_routing.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/test_webhook_event_routing.py b/tests/test_webhook_event_routing.py index 9ad5a7ba..bb621a01 100644 --- a/tests/test_webhook_event_routing.py +++ b/tests/test_webhook_event_routing.py @@ -5,6 +5,7 @@ This test ensures those events are recorded as 'ignored' while the two supported event types continue to work. """ + from __future__ import annotations import hashlib @@ -16,7 +17,6 @@ from app.models import WebhookEvent from app.webhooks.github import handle_github_webhook - SECRET = "test-secret" @@ -76,6 +76,26 @@ def test_push_event_is_ignored(sqlite_url: str) -> None: assert event.processed_status == "ignored" +def test_label_event_is_ignored(sqlite_url: str) -> None: + """label events must NOT be routed to the label handler.""" + create_schema(sqlite_url) + payload = { + "action": "created", + "label": {"name": "bug"}, + "repository": {"full_name": "ramimbo/mergework"}, + "sender": {"login": "maintainer"}, + } + headers, body = _wrap("label", payload, "delivery-label") + + result = handle_github_webhook(sqlite_url, headers, body, SECRET) + + assert result == {"status": "ignored"} + with session_scope(sqlite_url) as session: + event = session.get(WebhookEvent, "delivery-label") + assert event is not None + assert event.processed_status == "ignored" + + # -- events that must still route to the label handler ----------------------