Summary

Add a base class and registry for GitHub App webhook handlers.

Review Request #15334 — Created Sept. 24, 2026 and updated Oct. 9, 2026, 1:15 p.m.

Information

Owner

maubin

Repository

Review Board

Branch

release-9.x

Bugs

Depends On

Blocks

15335

Reviewers

Groups

reviewboard

People

Description

A GitHub App has a single, app-wide webhook URL that receives events from
every account the app is installed on. We have the GitHubAppWebhookView for
this, which currently only handles app installation events. We should have an
easy way for us and extension authors to handle GitHub webhook events too.

This change introduces the BaseGitHubAppWebhookHandler, which can be
subclassed to list the events the handler supports and the required GitHub App
permissions. Events will be passed to handlers as a GitHubAppWebhookEvent,
containing the event's raw payload, delivery ID, and the account(s) and
repository it may apply to. The actual dispatching to handlers will come in
a later change.

This also adds a registry for the handlers. The registry has a helper for
merging all the events and required pemissions of the registered handlers,
so that they can all be shown and requested when creating a GitHub App. The
hook for registering handlers will come in a later change.

Testing Done


Ran unit tests.
Used in the Pull Request integration work.

Commits

Summary	ID
Add a base class and registry for GitHub App webhook handlers. A GitHub App has a single, app-wide webhook URL that receives events from every account the app is installed on. We have the `GitHubAppWebhookView` for this, which currently only handles app installation events. We should have an easy way for us and extension authors to handle GitHub webhook events too. This change introduces the `BaseGitHubAppWebhookHandler`, which can be subclassed to list the events the handler supports and the required GitHub App permissions. Events will be passed to handlers as a `GitHubAppWebhookEvent`, containing the event's raw payload, delivery ID, and the account(s) and repository it may apply to. The actual dispatching to handlers will come in a later change. This also adds a registry for the handlers. The registry has a helper for merging all the events and required pemissions of the registered handlers, so that they can all be shown and requested when creating a GitHub App. The hook for registering handlers will come in a later change.	d3317ad8b4747b74e8e46902a7689955d9aa16b0

Summary

Add a base class and registry for GitHub App webhook handlers.

A GitHub App has a single, app-wide webhook URL that receives events from every account the app is installed on. We have the `GitHubAppWebhookView` for this, which currently only handles app installation events. We should have an easy way for us and extension authors to handle GitHub webhook events too. This change introduces the `BaseGitHubAppWebhookHandler`, which can be subclassed to list the events the handler supports and the required GitHub App permissions. Events will be passed to handlers as a `GitHubAppWebhookEvent`, containing the event's raw payload, delivery ID, and the account(s) and repository it may apply to. The actual dispatching to handlers will come in a later change. This also adds a registry for the handlers. The registry has a helper for merging all the events and required pemissions of the registered handlers, so that they can all be shown and requested when creating a GitHub App. The hook for registering handlers will come in a later change.

d3317ad8b4747b74e8e46902a7689955d9aa16b0

Issues

Description	From	Last Updated
Needs "Version Added"	david	Oct. 8, 2026, 2:15 p.m.
Can we call this name or event_name? So we avoid having code that says event.event?	david	Sept. 29, 2026, 3:22 p.m.
typo: this is value -> this is the value	david	Sept. 29, 2026, 3:23 p.m.
These don't have defaults, and nothing checks them at registration. They'll only fail with an AttributeError later, inside get_events() or …	david	Sept. 30, 2026, 9:42 a.m.
This line is missing a :	david	Sept. 29, 2026, 3:27 p.m.
Parens aren't necessary here.	david	Oct. 8, 2026, 2:16 p.m.
OrderedRegistry.__init__ doesn't take any args. I don't think we need args or *kwargs passthrough.	david	Sept. 29, 2026, 3:32 p.m.
This isn't validating that events is a set. It seems like probably an easy mistake to write: events = 'pull_requests' …	david	Oct. 8, 2026, 3:12 p.m.
It seems like this exception will get caught by the extension hook, which will also log. Can we check and …	david	Oct. 8, 2026, 3:15 p.m.
This should probably return a Sequence	david	Sept. 29, 2026, 4:27 p.m.
Do we want to return this as a frozenset so that callers can't potentially mutate the cache?	david	Sept. 30, 2026, 9:08 a.m.
Should this return Mapping?	david	Sept. 30, 2026, 9:08 a.m.
Mapping	david	Sept. 30, 2026, 9:08 a.m.
self._events is still typed as set[str], and we're returning that internal cache directly. Can we type that as frozenset, and …	david	Oct. 8, 2026, 3:28 p.m.
Mapping	david	Sept. 30, 2026, 9:08 a.m.
This doesn't use self at all. Can we make it module-level like merge_github_app_permissions?	david	Oct. 8, 2026, 3:29 p.m.
Should be Mapping[str, GitHubAppPermissionLevel]	david	Sept. 30, 2026, 9:19 a.m.
Missing a "with" in here, and the closing """ should go on its own line.	david	Sept. 30, 2026, 9:23 a.m.
Might want to pin what logs we capture here so unrelated things don't cause the test to break: with self.assertLogs('reviewboard.hostingsvcs.github.webhooks', …	david	Sept. 30, 2026, 9:42 a.m.
no newline at end of file Column: 15 Error code: W292	reviewbot	Sept. 24, 2026, 3:46 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/hostingsvcs/tests/github/test_webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
no newline at end of file

Column: 15
Error code: W292
```

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we call this name or event_name? So we avoid having code that says event.event?
```
reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
typo: this is value -> this is the value
```

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)

The issue has been resolved. Show all issues

These don't have defaults, and nothing checks them at registration. They'll only fail with an AttributeError later, inside get_events() or get_handlers_for_event(). That would cause every webhook delivery to fail, not just the broken one.

Same for an invalid level set in required_permissions.

We should probably override on_item_registering and fail loudly if the handler is broken. The SCMTools registry does something like this for reference.

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This line is missing a :
```

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)

The issue has been resolved. Show all issues

OrderedRegistry.__init__ doesn't take any args. I don't think we need *args or **kwargs passthrough.

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should probably return a Sequence
```

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)

The issue has been resolved. Show all issues

Do we want to return this as a frozenset so that callers can't potentially mutate the cache?

reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should this return Mapping?
```
reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Mapping
```
reviewboard/hostingsvcs/github/webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Mapping
```
reviewboard/hostingsvcs/tests/github/test_webhooks.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Should be Mapping[str, GitHubAppPermissionLevel]
```
reviewboard/hostingsvcs/tests/github/test_webhooks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing a "with" in here, and the closing """ should go on its own line.
```

reviewboard/hostingsvcs/tests/github/test_webhooks.py (Diff revision 1)

The issue has been resolved. Show all issues

Might want to pin what logs we capture here so unrelated things don't cause the test to break:

with self.assertLogs('reviewboard.hostingsvcs.github.webhooks',
                     level='WARNING') as logs:

Change Summary:


Update GitHubAppWebhookEvent.event to event_name.
Validate webhook handlers before registering them.

Commits:

	Summary	ID
	Add a base class and registry for GitHub App webhook handlers. A GitHub App has a single, app-wide webhook URL that receives events from every account the app is installed on. We have the `GitHubAppWebhookView` for this, which currently only handles app installation events. We should have an easy way for us and extension authors to handle GitHub webhook events too. This change introduces the `BaseGitHubAppWebhookHandler`, which can be subclassed to list the events the handler supports and the required GitHub App permissions. Events will be passed to handlers as a `GitHubAppWebhookEvent`, containing the event's raw payload, delivery ID, and the account(s) and repository it may apply to. The actual dispatching to handlers will come in a later change. This also adds a registry for the handlers. The registry has a helper for merging all the events and required pemissions of the registered handlers, so that they can all be shown and requested when creating a GitHub App. The hook for registering handlers will come in a later change.	89b3df9fa22bd8a85990587f90633bc976faa54e
	Add a base class and registry for GitHub App webhook handlers. A GitHub App has a single, app-wide webhook URL that receives events from every account the app is installed on. We have the `GitHubAppWebhookView` for this, which currently only handles app installation events. We should have an easy way for us and extension authors to handle GitHub webhook events too. This change introduces the `BaseGitHubAppWebhookHandler`, which can be subclassed to list the events the handler supports and the required GitHub App permissions. Events will be passed to handlers as a `GitHubAppWebhookEvent`, containing the event's raw payload, delivery ID, and the account(s) and repository it may apply to. The actual dispatching to handlers will come in a later change. This also adds a registry for the handlers. The registry has a helper for merging all the events and required pemissions of the registered handlers, so that they can all be shown and requested when creating a GitHub App. The hook for registering handlers will come in a later change.	d80a8f7e357a8cc5ba2cb2abe6b6784a680efc12

Diff:

Revision 2 (+1762)

Show changes

	docs/manual/extending/coderef/index.rst
	reviewboard/hostingsvcs/github/webhooks.py
	reviewboard/hostingsvcs/tests/github/test_webhooks.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/hostingsvcs/github/webhooks.py (Diff revisions 1 - 2)
The issue has been resolved. Show all issues
```
Needs "Version Added"
```
reviewboard/hostingsvcs/github/webhooks.py (Diff revisions 1 - 2)
The issue has been resolved. Show all issues
```
Parens aren't necessary here.
```

reviewboard/hostingsvcs/github/webhooks.py (Diff revisions 1 - 2)

The issue has been resolved. Show all issues

This isn't validating that events is a set. It seems like probably an easy mistake to write:

events = 'pull_requests'

Which will end up iterating over single characters, sticking 'p', 'u', 'l', ... into the github app manifest.

reviewboard/hostingsvcs/github/webhooks.py (Diff revisions 1 - 2)

The issue has been resolved. Show all issues

It seems like this exception will get caught by the extension hook, which will also log. Can we check and make sure we're not adding extra noise by logging here?

reviewboard/hostingsvcs/github/webhooks.py (Diff revisions 1 - 2)

The issue has been resolved. Show all issues

self._events is still typed as set[str], and we're returning that internal cache directly. Can we type that as frozenset, and then assemble it here:

events = frozenset(
    event
    for handler in self
    for event in handler.events
)

reviewboard/hostingsvcs/github/webhooks.py (Diff revisions 1 - 2)

The issue has been resolved. Show all issues

This doesn't use self at all. Can we make it module-level like merge_github_app_permissions?

Commits:

	Summary	ID
	Add a base class and registry for GitHub App webhook handlers. A GitHub App has a single, app-wide webhook URL that receives events from every account the app is installed on. We have the `GitHubAppWebhookView` for this, which currently only handles app installation events. We should have an easy way for us and extension authors to handle GitHub webhook events too. This change introduces the `BaseGitHubAppWebhookHandler`, which can be subclassed to list the events the handler supports and the required GitHub App permissions. Events will be passed to handlers as a `GitHubAppWebhookEvent`, containing the event's raw payload, delivery ID, and the account(s) and repository it may apply to. The actual dispatching to handlers will come in a later change. This also adds a registry for the handlers. The registry has a helper for merging all the events and required pemissions of the registered handlers, so that they can all be shown and requested when creating a GitHub App. The hook for registering handlers will come in a later change.	d80a8f7e357a8cc5ba2cb2abe6b6784a680efc12
	Add a base class and registry for GitHub App webhook handlers. A GitHub App has a single, app-wide webhook URL that receives events from every account the app is installed on. We have the `GitHubAppWebhookView` for this, which currently only handles app installation events. We should have an easy way for us and extension authors to handle GitHub webhook events too. This change introduces the `BaseGitHubAppWebhookHandler`, which can be subclassed to list the events the handler supports and the required GitHub App permissions. Events will be passed to handlers as a `GitHubAppWebhookEvent`, containing the event's raw payload, delivery ID, and the account(s) and repository it may apply to. The actual dispatching to handlers will come in a later change. This also adds a registry for the handlers. The registry has a helper for merging all the events and required pemissions of the registered handlers, so that they can all be shown and requested when creating a GitHub App. The hook for registering handlers will come in a later change.	d3317ad8b4747b74e8e46902a7689955d9aa16b0

Diff:

Revision 3 (+1776)

Show changes

	docs/manual/extending/coderef/index.rst
	reviewboard/hostingsvcs/github/webhooks.py
	reviewboard/hostingsvcs/tests/github/test_webhooks.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```