docs: clarify GitHub webhook setup (org vs repo) and warn about duplicate notifications#9042
Open
ewwollesen wants to merge 2 commits into
Open
docs: clarify GitHub webhook setup (org vs repo) and warn about duplicate notifications#9042ewwollesen wants to merge 2 commits into
ewwollesen wants to merge 2 commits into
Conversation
…cate notifications The GitHub plugin integration guide said to "create a webhook in GitHub for each GitHub organization you want to set up," which implied org-level webhooks were the only path and gave no warning about overlapping webhooks. Both organization-level and repository-level webhooks are valid and fully supported by the plugin (the webhook handler routes purely on the repository in the event payload and doesn't care about the delivering webhook's scope). However, the plugin posts once per delivery with no cross-webhook de-dup, so a repository covered by multiple webhooks (e.g. an org-level hook plus a repo-level hook) produces duplicate notifications. This updates the "Manual configuration" tab to: - state that org-level and repo-level webhooks are both valid, - add an .. important:: warning that exactly one webhook must deliver a given repository's events, since overlapping webhooks cause duplicate posts, - add a .. note:: about the misleading "No webhook was found for this repository or organization" message shown when subscribing to a specific repo while relying on an org-level webhook. Documentation-only. Motivated by support ticket 51590 (multi-node HA customer seeing triple-posted GitHub events from overlapping org + repo webhooks). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe "Create a webhook in GitHub" section in the GitHub integration guide is updated to support both organization-level and repository-level webhook creation. An "important" warning is added about avoiding duplicate webhooks, and a note is added explaining the plugin's false-negative "No webhook was found" message when using organization-level webhooks. ChangesGitHub Webhook Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
source/integrations-guide/github.rst (1)
80-82: ⚡ Quick winClarify the note's advice to align with the "only one webhook" requirement.
The final sentence—"To avoid the message, add a repository-level webhook for that repository"—could be misunderstood as adding a repository-level webhook in addition to the organization-level webhook you already have. Since the preceding
.. important::block explicitly requires only one webhook per repository, this phrasing should clarify that users should replace the organization-level webhook with a repository-level one, not add both.Suggested clarification
Replace the final sentence with one of the following:
Option 1 (direct):
- To avoid the message, add a repository-level webhook for that repository. + To avoid the message while maintaining only one webhook per repository, switch from using an organization-level webhook to a repository-level webhook for that repository instead.Option 2 (concise):
- To avoid the message, add a repository-level webhook for that repository. + To avoid the message, use a repository-level webhook instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@source/integrations-guide/github.rst` around lines 80 - 82, The final sentence of the note starting with "To avoid the message, add a repository-level webhook for that repository" is ambiguous and could be misinterpreted as instructing users to add a repository-level webhook in addition to the existing organization-level webhook. Since the preceding important block requires only one webhook per repository, reword this sentence to explicitly clarify that users should replace the organization-level webhook with a repository-level webhook for that specific repository, not add both.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@source/integrations-guide/github.rst`:
- Around line 80-82: The final sentence of the note starting with "To avoid the
message, add a repository-level webhook for that repository" is ambiguous and
could be misinterpreted as instructing users to add a repository-level webhook
in addition to the existing organization-level webhook. Since the preceding
important block requires only one webhook per repository, reword this sentence
to explicitly clarify that users should replace the organization-level webhook
with a repository-level webhook for that specific repository, not add both.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8833175a-3237-40a2-a595-65124aae29ec
📒 Files selected for processing (1)
source/integrations-guide/github.rst
Contributor
|
Newest code from mattermost has been published to preview environment for Git SHA 221f3f7 |
…plicates) Address review feedback: the note's final sentence could be read as telling users to add a repository-level webhook in addition to the organization-level one, which would cause the duplicate notifications warned about in the preceding .. important:: block. Reword to state the message is safe to ignore, and that avoiding it means delivering that repository's events through a repository-level webhook instead of the organization-level webhook -- not in addition to it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Newest code from mattermost has been published to preview environment for Git SHA 0286a1a |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the GitHub integration guide to clarify webhook setup options (organization-level vs repository-level) and to warn readers that overlapping webhooks can cause duplicate Mattermost notifications.
Changes:
- Replaces org-only webhook guidance with explicit org-level and repo-level webhook options.
- Adds an
.. important::admonition warning that each repository’s events must be delivered by exactly one webhook to avoid duplicate notifications. - Adds an
.. note::explaining a potentially misleading “No webhook was found…” message when relying on org-level webhooks.
|
|
||
| .. note:: | ||
|
|
||
| When you subscribe a channel to a specific repository using ``/github subscribe owner/repository`` while relying on an **organization-level** webhook, the plugin may report ``No webhook was found for this repository or organization`` even though the organization-level webhook is delivering events correctly. This check currently only looks for repository-level webhooks; the message is informational and doesn't mean events are missing, so it's safe to ignore. If you prefer to avoid it, deliver that repository's events through a repository-level webhook *instead of* the organization-level webhook. Don't add a repository-level webhook in addition to the organization-level one, or that repository will receive duplicate notifications, as described above. |
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
Documentation-only update to the GitHub plugin integration guide (
source/integrations-guide/github.rst), in the Manual configuration tab under Create a webhook in GitHub.The guide previously said to "create a webhook in GitHub for each GitHub organization you want to set up", which (a) implied org-level webhooks were the only/expected approach, (b) didn't mention that repository-level webhooks work equally well, and (c) gave no warning that overlapping webhooks cause duplicated notifications.
This change replaces that single sentence with guidance that:
.. important::admonition). There is no cross-webhook de-duplication — each delivery is a distinct event and the plugin posts once per delivery — so a repository covered by both an org-level and a repo-level webhook (or by duplicate webhooks) produces duplicate notifications... note::documents the misleadingNo webhook was found for this repository or organizationmessage shown when subscribing to a specific repo (/github subscribe owner/repository) while relying on an org-level webhook. The plugin's existence check only lists repository-level hooks, so the message is informational and doesn't mean events are missing.Step 1 of the numbered setup steps was lightly tweaked to clarify the Settings page can be your organization or a specific repository. The remaining numbered steps (Payload URL, Content Type, Secret, event selection) are unchanged.
Motivation
Support ticket 51590: a multi-node HA customer saw the same GitHub event posted multiple times (triple-posted) in Mattermost. Root cause was configuration — several repository-level webhooks plus an organization-level webhook all firing for the same repo. GitHub sends one delivery per webhook and the plugin posts once per delivery, so overlapping webhooks produce duplicate notifications. The fix is configuration (one webhook per repository's events); this PR documents that so customers can self-diagnose and avoid it.
Verified the documented behavior against the plugin source (
mattermost/mattermost-plugin-github, tagv2.7.0):handleWebhook/GetSubscribedChannelsForRepository(scope-agnostic routing),flows.go(Organizations.CreateHookfor org-level setup), andcheckIfConfiguredWebhookExistsincommand.go(repo-level-only existence check).Type
Documentation-only. No plugin source is modified.
Related follow-up (separate, not in this PR)
The plugin's
checkIfConfiguredWebhookExists(server/plugin/command.go) only lists repository-level hooks when a user subscribes to a specific repo, so it never detects an org-level webhook that is correctly delivering events — which is why the misleading "No webhook was found…" note appears. A code fix to also check org-level hooks on a repo subscription is being tracked separately (to be filed as a Jira/GitHub issue), not in this documentation PR.Testing
Validated the edited RST structurally with
docutils(the.. important::and.. note::admonitions and bullet lists parse with zero system messages; only the Sphinx-specific.. tab::directive is non-standard). The full Sphinx build environment (Python 3.12 / pipenv) was not bootstrapped locally; CI will run the fullgmake htmlbuild.🤖 Generated with Claude Code