feat(notifications): add opt-in Discord webhook via 'gac discord' subcommands#76
Merged
Conversation
…ications - Introduce discord_webhook module with embed-style notifications featuring repo name, branch, commit hash, and message in a GitHub-style card format - Add interactive Discord webhook configuration to gac init workflow with options to keep, replace, or remove existing webhook URLs - Integrate webhook notifications into both single and grouped commit workflows - Implement graceful error handling: network failures are logged but never block successful commits - Support configurable timeout (5 seconds) and environment-based URL storage via GAC_DISCORD_WEBHOOK_URL
- Add unit tests for webhook URL parsing, embed building, and message truncation logic - Test HTTP error handling to verify failures are swallowed gracefully - Test GitError handling to ensure webhook falls back to sensible defaults - Update existing init CLI tests to account for new Discord webhook prompt in the configuration flow - Verify embed payload structure matches Discord API requirements
- Add Discord Webhook Notifications section to USAGE.md with setup instructions and behavior details - Document GAC_DISCORD_WEBHOOK_URL environment variable configuration - Note that webhook failures are logged at WARNING level and never block commits - Update CHANGELOG with v3.36.0 release notes describing the new feature - Bump version from 3.35.2 to 3.36.0
📝 WalkthroughWalkthroughThis PR adds Discord webhook notifications to ChangesDiscord Webhook Notifications
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tests/test_discord_webhook.py (1)
57-67: ⚡ Quick winAssert the webhook timeout contract in this test.
On Line 62-Line 67, please also assert
timeout=5inhttpx.postkwargs, since timeout behavior is part of the feature contract and regressions here would silently weaken reliability.🤖 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 `@tests/test_discord_webhook.py` around lines 57 - 67, The test test_notify_commit_posts_embed_payload currently inspects the httpx.post call payload but doesn't assert the timeout contract; update the test to also check that the httpx.post call was invoked with timeout=5 by extracting the call kwargs from mpost.call_args (same as payload extraction) and asserting kwargs["timeout"] == 5 so the timeout behavior for notify_commit (and the underlying httpx.post call) is enforced; reference the test function test_notify_commit_posts_embed_payload and the mocked httpx.post (mpost) when adding this assertion.
🤖 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.
Inline comments:
In `@docs/en/USAGE.md`:
- Around line 652-654: Update the documented message format (the line stating
"Message format: `**repo** · `branch` · `short-sha`` followed by the commit
message in a code block" and the "Uses the gac avatar and the username `gac`"
note) to describe the actual embed payload: list which parts go into
embed.title, embed.description, embed.author.name and embed.footer.text, remove
the "code block" and 2000-char statement, and state the correct Discord embed
truncation limits (title 256, description 4096, author name 256, footer 2048) so
the docs match the implemented embed contract and truncation behavior.
- Line 653: The Markdown line with the example message format uses mismatched
inline code ticks causing MD038; fix it by properly pairing or escaping
backticks—e.g., wrap the whole format string as inline code or escape inner
backticks so the phrase **repo** · `branch` · `short-sha` and the following
commit message code block are valid; ensure each inline code span around
repo/branch/short-sha uses matching backticks or replace the inline spans with a
fenced code block to avoid nested backtick conflicts.
In `@src/gac/__version__.py`:
- Line 3: The __version__ module-level variable currently lacks a type
annotation; update the assignment for __version__ in __version__.py to include
an explicit type annotation (i.e., annotate __version__ as str) so it satisfies
the src/**/*.py typing rule and static type checks (modify the __version__
binding to use a type annotation rather than an untyped assignment).
In `@src/gac/discord_webhook.py`:
- Around line 24-37: Module-level variables/constants in this file lack explicit
type annotations; add them so the module complies with the repository typing
rules. Annotate logger as logger: logging.Logger = logging.getLogger(__name__),
make constants annotated (prefer using typing.Final) e.g. GAC_AVATAR_URL:
Final[str] = "...", GAC_USERNAME: Final[str] = "gac", WEBHOOK_TIMEOUT_SECONDS:
Final[float] = 5.0, ENV_KEY: Final[str] = "GAC_DISCORD_WEBHOOK_URL",
EMBED_COLOR: Final[int] = 0x2DA44E, _MAX_TITLE_LEN: Final[int] = 256, and
_MAX_DESCRIPTION_LEN: Final[int] = 4096 (and import typing.Final if not already
imported).
- Around line 63-71: _split_subject_body currently splits the commit message on
the first newline, which violates the blank-line rule; update it to split on the
first blank line instead (handle one or more blank lines possibly containing
spaces). Locate the function _split_subject_body and replace the split logic to
split on a blank-line separator (e.g., using a regex like r'\n\s*\n' with a
maxsplit of 1), then strip and return the two parts as (subject, body) as
before.
In `@src/gac/init_cli.py`:
- Around line 231-233: The current check only uses url.startswith and allows
malformed inputs; update the validation where the variable url is checked (the
block that currently does if not url.startswith(("http://", "https://")):) to
parse the URL (e.g., using urllib.parse.urlparse), ensure the scheme is exactly
"http" or "https" and that netloc/host is non-empty (and optionally reject
inputs that are just the scheme like "https://"); if the parsed URL is invalid,
echo a clear error and return instead of saving the value. Use the same variable
names (url) and replace the simple startswith check with this stricter
validation.
---
Nitpick comments:
In `@tests/test_discord_webhook.py`:
- Around line 57-67: The test test_notify_commit_posts_embed_payload currently
inspects the httpx.post call payload but doesn't assert the timeout contract;
update the test to also check that the httpx.post call was invoked with
timeout=5 by extracting the call kwargs from mpost.call_args (same as payload
extraction) and asserting kwargs["timeout"] == 5 so the timeout behavior for
notify_commit (and the underlying httpx.post call) is enforced; reference the
test function test_notify_commit_posts_embed_payload and the mocked httpx.post
(mpost) when adding this assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1044ef0d-091d-43cf-9b03-12c3831ad0af
📒 Files selected for processing (10)
CHANGELOG.mddocs/en/USAGE.mdsrc/gac/__version__.pysrc/gac/discord_webhook.pysrc/gac/grouped_commit_executor.pysrc/gac/init_cli.pysrc/gac/main.pytests/test_discord_webhook.pytests/test_init_cli.pytests/test_init_cli_extended.py
…discord subcommands Per review feedback, gac init is restored to upstream and Discord webhook configuration now lives in a dedicated 'gac discord' command group: - gac discord setup interactively configure the webhook URL - gac discord show display whether a webhook is configured (masked) - gac discord test send a test notification - gac discord remove delete the configured URL This keeps the init flow unchanged, gives Discord its own ergonomic home, and adds an explicit 'test' command for users to verify their setup without making a real commit.
mpfaffenberger
changed the title
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
2 participants
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.
🐶 Add Discord webhook notifications via a new
gac discordcommandAdds an opt-in Discord webhook integration so
gaccan announce each successful commit to a Discord channel of your choice — handy for team channels, side-project log feeds, or just vibes.✨ What this adds
gac discordcommand group for managing the integration —gac initis unchanged:uvx gac discord setup— interactively configure (or replace) the webhook URLuvx gac discord show— show whether a webhook is configured (URL masked)uvx gac discord test— fire a test notification to verify your setupuvx gac discord remove— delete the configured URLGAC_DISCORD_WEBHOOK_URLenvironment variable. When set,gacPOSTs an embed to your webhook after every successful commit.#2DA44E)<repo> · <branch>with the gac avatarcommit <short-sha>main.py) and the grouped-commit workflow (grouped_commit_executor.py— fires once per commit in the group).🛡️ Opt-in by design
The integration is strictly opt-in, gated multiple ways:
gac initnever mentions Discord — users must rungac discord setupexplicitly.~/.gac.envuntil the user passes a URL throughdiscord setup.notify_commit()short-circuits and returnsFalseifGAC_DISCORD_WEBHOOK_URLis unset, blank, or whitespace-only — no HTTP request is made.setupvalidates that the URL starts withhttp(s)://before writing anything.🪨 Robustness
WARNING, and never block a successful commit.--dry-runand--message-only(no commit happened, so no notification).📁 Files changed
New
src/gac/discord_webhook.py— the notifier (~135 lines, single responsibility)src/gac/discord_cli.py— thegac discordcommand group (~150 lines)tests/test_discord_webhook.py— 13 unit teststests/test_discord_cli.py— 13 unit tests covering all four subcommandsModified
src/gac/cli.py— registers the newdiscordcommand groupsrc/gac/main.py— fires notifier after a successful single commitsrc/gac/grouped_commit_executor.py— fires notifier per commit in grouped modesrc/gac/__version__.py—3.35.2→3.36.0CHANGELOG.md— newv3.36.0entrydocs/en/USAGE.md— new "Discord Webhook Notifications" sectionNotably untouched
src/gac/init_cli.py,tests/test_init_cli.py,tests/test_init_cli_extended.py— zero diff vsmain(confirmed viagit diff).✅ PR checklist
__version__.py(3.36.0)CHANGELOG.mdupdateduv run -- pytestpassing — 2951 passed, 0 faileduv run ruff checkclean on all touched filesuv run ruff format --checkclean on all touched filesuv run mypyclean on the new modulesdiscord_webhook.py≈ 135,discord_cli.py≈ 150)gac initbyte-for-byte unchanged vs upstream🖼️ What it looks like
Roughly the same shape as GitHub's commit notifications: bordered card, colored left stripe, repo/branch as the author row, commit subject as a bold title, body as the description, short SHA in the footer.
🔧 How to use it
To check status or disable later:
Or set it directly in
~/.gac.env:GAC_DISCORD_WEBHOOK_URL='https://discord.com/api/webhooks/XXXX/YYYY'🐾 Authored with the help of Biscuit, the most loyal digital puppy.