-
-
Notifications
You must be signed in to change notification settings - Fork 0
Fixed GitHub notifications fail on unfinished status checks. #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.env (1)
170-171: Fix typos in commentsMinor documentation polish.
Apply this diff:
-# Notificaions are sent accross multiple channels before and after deployment. +# Notifications are sent across multiple channels before and after deployment.
| # | ||
| # A combination of comma-separated values: email,newrelic,github,jira | ||
| VORTEX_NOTIFY_CHANNELS=email | ||
| VORTEX_NOTIFY_CHANNELS=email,github |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Enabling GitHub notifications by default — verify tokens and failure modes
Switching the default to include the GitHub channel can cause CI/local runs to attempt GitHub API calls. Please verify:
- The notifier gracefully no-ops when no GitHub token/repo context is present (does not fail the build).
- CI secrets (e.g., GITHUB_TOKEN/GH_TOKEN) are configured where this runs.
- The change is mirrored (or intentionally not) in .env.local.example and docs.
If graceful degradation isn’t guaranteed, consider enabling the GitHub channel only in CI (via CI env vars) to avoid breaking local developer flows.
You can run the following script to verify usage, token checks, and payload changes:
I can help add a guard in notify-github.sh to short-circuit when tokens are missing or update docs/templates to scope GitHub notifications to CI only.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find where VORTEX_NOTIFY_CHANNELS is consumed:"
rg -n --pretty 'VORTEX_NOTIFY_CHANNELS' -A 3 -B 3 || true
echo
echo "2) Inspect the GitHub notify script for token checks and error handling:"
fd -a 'notify-github\.sh' | while read -r f; do
echo "----- ${f} -----"
rg -n 'set -e|GITHUB_TOKEN|GH_TOKEN|GITHUB_API_TOKEN|Authorization|repo|owner|environment|deployment' -A 2 -B 2 "$f" || true
done
echo
echo "3) Confirm that required_contexts: [] was added to deployments payload:"
rg -n --pretty 'required_contexts' -A 3 -B 3 || echo "No occurrences of required_contexts found."
echo
echo "4) Check examples/templates for consistency (keep defaults developer-friendly):"
fd -a '.env.local.example' | while read -r f; do
echo "----- ${f} -----"
rg -n 'VORTEX_NOTIFY_CHANNELS' -A 2 -B 2 "$f" || true
doneLength of output: 7157
Ensure GitHub notifications gracefully skip when tokens are missing
The new default (VORTEX_NOTIFY_CHANNELS=email,github) will break local or any runs without a valid GitHub token/repo context, since notify-github.sh currently fails hard if VORTEX_NOTIFY_GITHUB_TOKEN, VORTEX_NOTIFY_REPOSITORY or VORTEX_NOTIFY_BRANCH are unset. Please:
• In scripts/vortex/notify-github.sh (around lines 57–59), replace the “fail” on missing token/repo/branch with a guard that no-ops and exits 0 when running outside CI. For example:
if [ -z "$VORTEX_NOTIFY_GITHUB_TOKEN" ] || [ -z "$VORTEX_NOTIFY_REPOSITORY" ] || [ -z "$VORTEX_NOTIFY_BRANCH" ]; then
note "Skipping GitHub notification: missing token/repo/branch"
exit 0
fi• Update your environment examples and docs:
– Mirror the new default in your sample files (e.g. .env.local.example or .env.example) and call out that the GitHub channel only works when those three vars are set.
– Add a snippet in README or docs reminding developers that they must supply VORTEX_NOTIFY_GITHUB_TOKEN, VORTEX_NOTIFY_REPOSITORY and VORTEX_NOTIFY_BRANCH (typically via CI secrets) for GitHub notifications.
• Verify your CI workflows (e.g. .github/workflows/*.yml) export or map GITHUB_TOKEN (or your org’s PAT) into VORTEX_NOTIFY_GITHUB_TOKEN, and set VORTEX_NOTIFY_REPOSITORY & VORTEX_NOTIFY_BRANCH before invoking scripts/vortex/notify.sh.
These changes will prevent build failures for anyone without a configured GitHub context while keeping the new notification channel enabled in CI.
🤖 Prompt for AI Agents
In scripts/vortex/notify-github.sh around lines 57–59, the script currently
fails when VORTEX_NOTIFY_GITHUB_TOKEN, VORTEX_NOTIFY_REPOSITORY or
VORTEX_NOTIFY_BRANCH are unset; change that check to gracefully no-op and exit 0
when running outside CI (log a note that GitHub notifications are skipped due to
missing vars) instead of hard failing; additionally, update environment sample
files (.env.example or .env.local.example) to show the new default
VORTEX_NOTIFY_CHANNELS=email,github and document that GitHub notifications
require VORTEX_NOTIFY_GITHUB_TOKEN, VORTEX_NOTIFY_REPOSITORY and
VORTEX_NOTIFY_BRANCH (typically provided via CI secrets), and verify CI
workflows (.github/workflows/*.yml) map/export a token into
VORTEX_NOTIFY_GITHUB_TOKEN and set VORTEX_NOTIFY_REPOSITORY and
VORTEX_NOTIFY_BRANCH before calling scripts/vortex/notify.sh.
Summary by CodeRabbit