Skip to content

Commit 937ab5e

Browse files
authored
Merge pull request #43 from AOSSIE-Org/fix/notify-chronological-order
fix: chronological Discord PR-open notification order
2 parents 3b09fa8 + 5e51d35 commit 937ab5e

2 files changed

Lines changed: 70 additions & 1 deletion

File tree

src/ghdcbot/engine/orchestrator.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,24 @@ def close(self) -> None:
270270
close()
271271

272272

273+
def _notification_event_sort_key(event: ContributionEvent) -> tuple:
274+
"""Sort key so Discord notifications go out in chronological open/activity order.
275+
276+
Ingestion often yields GitHub API order (newest-first within a repo). Sorting only
277+
affects the notification pass — storage/cursor still use the original list.
278+
"""
279+
payload = event.payload or {}
280+
return (
281+
event.created_at,
282+
event.event_type,
283+
event.repo,
284+
str(payload.get("pr_number") or payload.get("issue_number") or ""),
285+
event.github_user or "",
286+
# Final tie-breaker: equal-time pr_reviewed rows stay deterministic across ingest order.
287+
str(payload.get("review_id") or ""),
288+
)
289+
290+
273291
def _send_notifications_for_new_events(
274292
contributions: list[ContributionEvent],
275293
storage: Storage,
@@ -287,7 +305,9 @@ def _send_notifications_for_new_events(
287305
sent_count = 0
288306
pr_reviewed_count = 0
289307
channels = pr_open_channels or {}
290-
for event in contributions:
308+
# Copy + sort so batch syncs post older events first (e.g. PR #41 before #42).
309+
ordered_events = sorted(contributions, key=_notification_event_sort_key)
310+
for event in ordered_events:
291311
if event.event_type == "pr_opened":
292312
if send_pr_opened_channel_notification(
293313
event, storage, discord_writer, policy, config, channels, github_org

tests/test_notifications.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,3 +1324,52 @@ def test_link_comment_timeout_preserves_claim_prevents_duplicate(tmp_path) -> No
13241324
assert client.post.call_count == 1
13251325
assert storage.was_notification_sent("pr_opened_github_link:Gitcord-GithubDiscordBot:42")
13261326

1327+
1328+
def test_batch_pr_opened_notifications_sent_oldest_first() -> None:
1329+
"""Batch sync must post channel PR-open notices in created_at order (not API newest-first)."""
1330+
from ghdcbot.engine.orchestrator import _send_notifications_for_new_events
1331+
1332+
storage = MockStorage()
1333+
discord_writer = MockDiscordWriter()
1334+
config = NotificationConfig(enabled=True, pr_opened=True)
1335+
policy = MutationPolicy(
1336+
mode=RunMode.ACTIVE, github_write_allowed=True, discord_write_allowed=True
1337+
)
1338+
channels = {"Gitcord-GithubDiscordBot": "1465995983791063140"}
1339+
1340+
newer = ContributionEvent(
1341+
github_user="alice",
1342+
event_type="pr_opened",
1343+
repo="Gitcord-GithubDiscordBot",
1344+
created_at=datetime(2026, 8, 4, 18, 57, tzinfo=UTC),
1345+
payload={"pr_number": 42, "title": "Week 11 remote config"},
1346+
)
1347+
older = ContributionEvent(
1348+
github_user="alice",
1349+
event_type="pr_opened",
1350+
repo="Gitcord-GithubDiscordBot",
1351+
created_at=datetime(2026, 8, 4, 18, 13, tzinfo=UTC),
1352+
payload={"pr_number": 41, "title": "CI mock fix"},
1353+
)
1354+
1355+
# Newest-first (GitHub list order) — notifications should still post #41 then #42.
1356+
events = [newer, older]
1357+
_send_notifications_for_new_events(
1358+
events,
1359+
storage,
1360+
discord_writer,
1361+
policy,
1362+
config,
1363+
"AOSSIE-Org",
1364+
channels,
1365+
)
1366+
1367+
# Notification pass must not mutate the caller's ingestion list.
1368+
assert events[0] is newer
1369+
assert events[1] is older
1370+
assert events == [newer, older]
1371+
1372+
assert len(discord_writer.messages_sent) == 2
1373+
assert "#41" in discord_writer.messages_sent[0][1]
1374+
assert "#42" in discord_writer.messages_sent[1][1]
1375+

0 commit comments

Comments
 (0)