feat(domain): github alert routing and repo-topic links (1/5) - #12
Merged
Merged
Conversation
Adds migration 0002 (github_org_claims, repo_topic_links), the GithubEvent model and alert formatter, ports for org claims, repo links and alert sending, and the linkRepoToTopic, unlinkRepo, listRepoLinks and routeGithubEvent use cases. Linking is admin-only and limited to the team's claimed org; unlinked repos produce no alerts; infrastructure failures propagate so the route can answer 500.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First PR of the
github-alertschain (5 PRs). It adds only the domain and the migration. Nothing is wired to HTTP or Telegram yet, so there is no user-visible change.0002_github_alerts.sql: addsgithub_org_claims(one org is bound to one team) andrepo_topic_links(one topic per repo per team). Both are additiveCREATE TABLEs, applied automatically on deploy.GithubEvent+formatGithubAlert: alerts carry only allowlisted fields (repo, action, GitHub login, number, title, link, reviewer) and are capped at Telegram's 4096 characters.linkRepoToTopic: admin-only and limited to the team's claimed org. Re-linking moves the repo and reports the previous topic.unlinkRepo: admin-only.listRepoLinks: open to any registered team member.routeGithubEvent: org claim, then team, then link, then send. Unlinked repos and unclaimed orgs are ignored. A Telegram send failure is reported as an outcome, and infrastructure failures propagate so the route can answer 500.Size exception: about 1,120 lines. Production code is about 350 of them; the rest is tests, fakes and the apply-progress notes.
Review
Full review with four lenses, since the diff is over 400 lines and includes the authorization and claim gates. There were no blocker findings. Two critical findings were confirmed and fixed. Both were untested behavior:
throwsoption, and tests now assert that D1-style errors reject instead of being swallowed as "ignored".mergedandreview_requestedalerts had no tests.The warnings were also fixed:
link-repo.tsrenamed tolink-repo-to-topic.tsso the file matches its export.A scoped fix-delta validator approved the correction.
Intended and left as is:
org_loginis the primary key ofgithub_org_claims, so one org belongs to exactly one team.Disclosed process gap
Part of this slice was not written test-first: the reviewer line and two action variants were implemented without a failing test. The review caught it, and every added test passed on the existing code with no bug found.
Test plan
npx vitest run: 229/229 pass, including migration 0002 applied in the D1 test poolnpx tsc --noEmit: no errorsAfter merge
Operator step: claim the org in D1 (
tasks.md6.3), before/linkrepoships in PR 5.