|
| 1 | +# GitHub Webhook Contract Hardening Implementation Plan |
| 2 | + |
| 3 | +Status: planned research artifact. Use as the starting point before changing |
| 4 | +GitHub webhook intake again. |
| 5 | + |
| 6 | +**Goal:** Make hosted CodeAlmanac's GitHub webhook intake match GitHub's event schemas for the webhook families we depend on, without adding reconciliation or a parallel sync path. |
| 7 | + |
| 8 | +**Architecture:** Route webhook parsing by `X-GitHub-Event`, normalize supported payloads into typed Pydantic messages, and audit unsupported payloads as ignored. Control-plane messages carry the parent provider facts they need; fanout subscribers keep owning their own tables. |
| 9 | + |
| 10 | +**Tech Stack:** FastAPI, SQLModel, Pydantic, GitHub App webhooks, Octokit generated webhook schemas for research. |
| 11 | + |
| 12 | +--- |
| 13 | + |
| 14 | +## Research Baseline |
| 15 | + |
| 16 | +Checked on 2026-07-03: |
| 17 | + |
| 18 | +- GitHub docs: `https://docs.github.com/en/webhooks/webhook-events-and-payloads` |
| 19 | +- GitHub Apps webhook docs: `https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/using-webhooks-with-github-apps` |
| 20 | +- `@octokit/webhooks-schemas@7.6.1`: 66 top-level event families, 224 action variants. |
| 21 | +- `@octokit/webhooks-examples@7.6.1`: current dotcom examples for 58 event families. |
| 22 | + |
| 23 | +The current backend supports only these event families: |
| 24 | + |
| 25 | +```text |
| 26 | +installation |
| 27 | +installation_repositories |
| 28 | +repository |
| 29 | +push |
| 30 | +pull_request |
| 31 | +``` |
| 32 | + |
| 33 | +That scope is correct for launch. The bug is not that we ignore most GitHub events; the bug is that the mapper currently guesses by payload shape instead of using `X-GitHub-Event`, and the `installation` action names do not match the schema. |
| 34 | + |
| 35 | +## Important Schema Findings |
| 36 | + |
| 37 | +`installation` actions are: |
| 38 | + |
| 39 | +```text |
| 40 | +created |
| 41 | +deleted |
| 42 | +new_permissions_accepted |
| 43 | +suspend |
| 44 | +unsuspend |
| 45 | +``` |
| 46 | + |
| 47 | +Current code checks `suspended` and `unsuspended`, which is wrong. |
| 48 | + |
| 49 | +`installation_repositories` actions are: |
| 50 | + |
| 51 | +```text |
| 52 | +added |
| 53 | +removed |
| 54 | +``` |
| 55 | + |
| 56 | +Each payload includes: |
| 57 | + |
| 58 | +```text |
| 59 | +installation |
| 60 | +repository_selection |
| 61 | +repositories_added |
| 62 | +repositories_removed |
| 63 | +requester |
| 64 | +sender |
| 65 | +``` |
| 66 | + |
| 67 | +The `installation.account` object is present there, so the mapper can produce `AccountSnapshot` and `InstallationSnapshot` for delta events without calling GitHub again. |
| 68 | + |
| 69 | +Most repository-scoped events include these optional or required top-level objects: |
| 70 | + |
| 71 | +```text |
| 72 | +installation |
| 73 | +repository |
| 74 | +organization |
| 75 | +sender |
| 76 | +``` |
| 77 | + |
| 78 | +That does not mean CodeAlmanac should persist every event. It means the webhook edge should route by event name, validate only supported families, and record unsupported families as ignored. |
| 79 | + |
| 80 | +## Out Of Scope |
| 81 | + |
| 82 | +- No login-time GitHub reconciler. |
| 83 | +- No background reconciler. |
| 84 | +- No broad "sync all installations" repair job. |
| 85 | +- No runtime dependency on Node or Octokit. |
| 86 | +- No modeling all 66 GitHub webhook families as Python messages. |
| 87 | +- No subscription expansion unless product work needs the event. |
| 88 | + |
| 89 | +## Target Shape |
| 90 | + |
| 91 | +```python |
| 92 | +message = github_webhooks.parse_github_message(event_name, payload) |
| 93 | + |
| 94 | +match event_name: |
| 95 | + case "installation": |
| 96 | + return parse_installation(payload) |
| 97 | + case "installation_repositories": |
| 98 | + return parse_installation_repositories(payload) |
| 99 | + case "repository": |
| 100 | + return parse_repository(payload) |
| 101 | + case "push": |
| 102 | + return parse_push(payload) |
| 103 | + case "pull_request": |
| 104 | + return parse_pull_request(payload) |
| 105 | + case _: |
| 106 | + return None |
| 107 | +``` |
| 108 | + |
| 109 | +Supported control-plane messages carry parent snapshots: |
| 110 | + |
| 111 | +```python |
| 112 | +InstallationRepositoriesAdded( |
| 113 | + account=AccountSnapshot(...), |
| 114 | + installation=InstallationSnapshot(...), |
| 115 | + repository_selection="selected", |
| 116 | + repositories=[...], |
| 117 | +) |
| 118 | +``` |
| 119 | + |
| 120 | +Identity fanout handles the parent rows: |
| 121 | + |
| 122 | +```python |
| 123 | +identity.on_installation_repositories_added(message): |
| 124 | + accounts.upsert(message.account) |
| 125 | + installations.upsert(message.installation) |
| 126 | +``` |
| 127 | + |
| 128 | +Repository fanout handles repository scope only: |
| 129 | + |
| 130 | +```python |
| 131 | +repositories.on_installation_repositories_added(message): |
| 132 | + repository_scope.sync_installation(message.installation.installation_id) |
| 133 | +``` |
| 134 | + |
| 135 | +## Implementation Tasks |
| 136 | + |
| 137 | +### Task 1: Route Parsing By GitHub Event Header |
| 138 | + |
| 139 | +**Files:** |
| 140 | + |
| 141 | +- Modify: `backend/src/almanac/services/github/service.py` |
| 142 | +- Modify: `backend/src/almanac/services/github/webhooks.py` |
| 143 | +- Test: `backend/tests/test_github_service_contract.py` |
| 144 | + |
| 145 | +Steps: |
| 146 | + |
| 147 | +1. Change `GitHubService.handle_webhook` to call `parse_github_message(event, payload)`. |
| 148 | +2. Replace shape-sniffing in `map_payload(payload)` with event-name dispatch. |
| 149 | +3. Keep unsupported event families returning `None`. |
| 150 | +4. Add tests showing unsupported `check_run` / `check_suite` remain ignored. |
| 151 | +5. Add a regression test showing a payload with both `repository` and `installation` is routed by event name, not shape. |
| 152 | + |
| 153 | +### Task 2: Correct Installation Action Contract |
| 154 | + |
| 155 | +**Files:** |
| 156 | + |
| 157 | +- Modify: `backend/src/almanac/services/github/webhooks.py` |
| 158 | +- Modify: `backend/src/almanac/services/github/webhook_messages.py` |
| 159 | +- Test: `backend/tests/test_github_service_contract.py` |
| 160 | + |
| 161 | +Steps: |
| 162 | + |
| 163 | +1. Accept `suspend` and `unsuspend`. |
| 164 | +2. Stop accepting non-schema `suspended` and `unsuspended`. |
| 165 | +3. Map `suspend` to `InstallationSuspended`. |
| 166 | +4. Map `unsuspend` to `InstallationUnsuspended`. |
| 167 | +5. Ignore `new_permissions_accepted` unless product behavior requires it later. |
| 168 | + |
| 169 | +### Task 3: Carry Parent Snapshots On Installation Repository Deltas |
| 170 | + |
| 171 | +**Files:** |
| 172 | + |
| 173 | +- Modify: `backend/src/almanac/messages/github.py` |
| 174 | +- Modify: `backend/src/almanac/services/github/webhook_messages.py` |
| 175 | +- Modify: `backend/src/almanac/wiring/fanout/identity.py` |
| 176 | +- Test: `backend/tests/test_installations_contract.py` |
| 177 | +- Test: `backend/tests/test_github_service_contract.py` |
| 178 | + |
| 179 | +Steps: |
| 180 | + |
| 181 | +1. Add `account: AccountSnapshot` and `installation: InstallationSnapshot` to `InstallationRepositoriesAdded`. |
| 182 | +2. Add the same fields to `InstallationRepositoriesRemoved`. |
| 183 | +3. Populate those snapshots from `installation.account` and `installation.id`. |
| 184 | +4. Subscribe identity fanout to both delta message types. |
| 185 | +5. Upsert account and installation in identity fanout before repository fanout syncs repositories. |
| 186 | +6. Keep repository fanout focused on `RepositoryScope`. |
| 187 | + |
| 188 | +### Task 4: Add Schema Guardrails For Supported Families |
| 189 | + |
| 190 | +**Files:** |
| 191 | + |
| 192 | +- Modify: `backend/tests/test_github_service_contract.py` |
| 193 | +- Optional create: `backend/tests/fixtures/github_webhooks/README.md` |
| 194 | + |
| 195 | +Steps: |
| 196 | + |
| 197 | +1. Add compact fixture payloads for the five supported event families. |
| 198 | +2. Include action coverage for: |
| 199 | + - `installation.created` |
| 200 | + - `installation.deleted` |
| 201 | + - `installation.suspend` |
| 202 | + - `installation.unsuspend` |
| 203 | + - `installation_repositories.added` |
| 204 | + - `installation_repositories.removed` |
| 205 | + - `repository.renamed` |
| 206 | + - `repository.transferred` |
| 207 | + - `repository.deleted` |
| 208 | + - `push` |
| 209 | + - supported `pull_request` actions |
| 210 | +3. Assert ignored actions are audited as ignored, not invalid. |
| 211 | +4. Assert malformed supported payloads are audited as invalid. |
| 212 | + |
| 213 | +### Task 5: Update Launch Docs |
| 214 | + |
| 215 | +**Files:** |
| 216 | + |
| 217 | +- Modify: `docs/codealmanac-launch/worklog.md` |
| 218 | +- Modify: `docs/codealmanac-launch/progress.md` |
| 219 | +- Modify: `docs/codealmanac-launch/verification-matrix.md` |
| 220 | +- Modify: `docs/codealmanac-launch/next-agent-brief.md` |
| 221 | +- Optional modify: `docs/codealmanac-launch/auth-api-contract.md` |
| 222 | + |
| 223 | +Steps: |
| 224 | + |
| 225 | +1. Record the schema inventory result. |
| 226 | +2. Record that reconciliation remains out of scope. |
| 227 | +3. Record that webhook parsing is event-header routed. |
| 228 | +4. Update percentages only after tests and deployment. |
| 229 | + |
| 230 | +## Verification |
| 231 | + |
| 232 | +Run in hosted repo: |
| 233 | + |
| 234 | +```bash |
| 235 | +cd /Users/rohan/.config/superpowers/worktrees/usealmanac/hosted-baseline-convergence |
| 236 | +uv run pytest backend/tests/test_github_service_contract.py backend/tests/test_installations_contract.py -q |
| 237 | +uv run pytest backend/tests/test_repositories_contract.py backend/tests/test_wiki_contract.py -q |
| 238 | +uv run ruff check backend/src backend/tests |
| 239 | +``` |
| 240 | + |
| 241 | +If backend changes pass, deploy hosted backend/frontend together only after the coherent slice is complete. |
| 242 | + |
| 243 | +## Decision Check |
| 244 | + |
| 245 | +This plan fixes real contract drift and the DB-wipe webhook edge without making DB wipe a supported product workflow. If we later want first-class repair, that should be a separate reconciler design with explicit product semantics. |
0 commit comments