Skip to content

feat(github): deliver issue and PR alerts to linked topics (4/5) - #15

Merged
TOMOKI977 merged 3 commits into
mainfrom
feat/github-alerts-delivery
Sep 25, 2026
Merged

TOMOKI977 merged 3 commits into
mainfrom
feat/github-alerts-delivery

Conversation

@TOMOKI977

Copy link
Copy Markdown
Contributor

Summary

Fourth PR of the github-alerts chain. This is where alerts actually reach Telegram. /github/webhook now maps the verified event, routes it through the org claim, the team and the repo link, and sends it to the linked forum topic.

  • Mapper mapGithubEvent: reads only allowlisted fields (repo, action, GitHub login, number, title, link, reviewer). Commit author emails and names never leave the payload. Supported events:
    • pull_request: opened, closed (or merged when merged === true), review_requested (reviewer login or team slug)
    • issues: opened, closed
  • createTelegramAlertSender: sendMessage with message_thread_id, in plain text with no parse_mode, so PR titles cannot inject formatting.
  • buildGithubRouter in the composition root: wires the D1 claim, link and team repos and the sender.

Status policy (after a valid signature)

Case Response
Unsupported event or action, incomplete payload 200, logged ignored:unsupported-event
Unclaimed org / unlinked repo 200, logged with the reason, nothing sent
Delivered 200
Telegram send failure 200, logged AlertSendFailed with a fixed class: rate-limited (429), telegram-unavailable (5xx or network), rejected (other 4xx). No retry in the request.
D1 or other unexpected error 500, so GitHub shows the delivery as failed and it can be redelivered

Size exception: about 1,120 lines. Production code is about 260 of them; the rest is tests and apply-progress notes.

Review

Full review with four lenses: payload handling, PII, cross-team delivery. The first run was cut short by a rate limit and was re-run in full. No blocker or critical findings; the risk lens found nothing. Six warnings were fixed in one correction:

  • Send failures were indistinguishable. A transient 429 looked the same as a permanent 400. The status stays 2xx per the spec, but the log now says which class it was, so lost alerts are visible. Retry or throttling is deferred until volume makes it matter.
  • The send-failure test now asserts exactly one sendMessage call ("no retry within the same request").
  • Mapper null-guard tests were added, and the unclaimed-org test now checks the logged reason.
  • Stale "not yet wired" comments were fixed.
  • The Telegram fetch stub was extracted to test/support/telegram-stub.ts. It was copied in three files.

A scoped fix-delta validator approved the correction.

Test plan

  • npx vitest run: 320/320 pass
  • npx tsc --noEmit: no errors

After merge

  • Operator step 6.1: set GITHUB_WEBHOOK_SECRET. It is still pending: the route answers 500 until it is set.
  • Operator step 6.2: configure the org webhook with content type application/json, the same secret, and the Pull requests and Issues events. Check that the ping returns 200.
  • Alerts start flowing once a repo is linked, which comes in PR 5 (/linkrepo).

Maps pull_request (opened, closed/merged, review_requested) and issues (opened, closed) payloads to allowlisted GithubEvent fields and routes them through org claim, team and repo link to the linked forum topic via sendMessage with message_thread_id. Unsupported events, unclaimed orgs and unlinked repos are acknowledged and logged; Telegram send failures return 2xx with a fixed failure class (rate-limited, telegram-unavailable, rejected); D1 failures return 500.
@TOMOKI977 TOMOKI977 added the size:exception PR over the 400-line review budget, accepted explicitly label Sep 25, 2026
@TOMOKI977
TOMOKI977 merged commit 5387578 into main Sep 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:exception PR over the 400-line review budget, accepted explicitly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant