refactor(ci): switch Discord automation from webhooks to single bot#42
Open
EtienneLescot wants to merge 3 commits into
Open
refactor(ci): switch Discord automation from webhooks to single bot#42EtienneLescot wants to merge 3 commits into
EtienneLescot wants to merge 3 commits into
Conversation
All four PR-review automations (intro post, thread updates, status
mutations, weekly spotlight) now flow through one Discord bot identity
(DISCORD_BOT_TOKEN) instead of three separate webhook URLs. Removes
the per-channel webhook secret rotation surface and consolidates rate
limits, audit logs, and permissions under one app.
Behavioural changes:
- Intro post + forum thread creation: was POST to webhook with
thread_name, now POST /channels/{forumChannelId}/threads via bot.
- Per-event message in thread: was POST to webhook with thread_id, now
POST /channels/{threadId}/messages via bot.
- Thread status (tags, archive, lock): unchanged, already used bot.
- Weekly leaderboard: was POST to spotlight webhook, now POST to
spotlight channel via bot.
- Failure alerts: optional, was alert webhook, now alert channel via
bot (DISCORD_ALERT_CHANNEL_ID); unset to silence.
Display name and avatar for bot messages are now the bot's configured
profile in the Discord developer portal rather than per-webhook env
vars. Remove the old secrets after migration:
DISCORD_WEBHOOK_URL, DISCORD_PR_FORUM_WEBHOOK, DISCORD_SPOTLIGHT_WEBHOOK_URL,
DISCORD_ALERT_WEBHOOK_URL, DISCORD_WEBHOOK_USERNAME, DISCORD_WEBHOOK_AVATAR_URL.
Add as variables (channel ids, not secrets):
DISCORD_PR_FORUM_CHANNEL_ID, DISCORD_SPOTLIGHT_CHANNEL_ID,
DISCORD_ALERT_CHANNEL_ID (optional).
Bot permissions required on each channel:
- PR forum: View, Send Messages, Embed Links, Create Public Threads,
Manage Threads.
- Spotlight channel: View, Send Messages, Embed Links.
- Alert channel (optional): View, Send Messages.
📝 WalkthroughWalkthroughAdds a Discord bot REST helper module, switches PR sync and weekly leaderboard posting to bot token plus channel ID configuration, and updates the related workflows and docs to match the new Discord delivery path. ChangesDiscord Bot API Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🤖 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 @.github/scripts/discord-bot-api.mjs:
- Around line 5-13: The callDiscord helper can hang indefinitely because it
makes a fetch without any request timeout, which can block PR sync and
leaderboard workflows. Update callDiscord in discord-bot-api.mjs to use an
AbortController-based timeout, matching the pattern used by the other Discord
helper, and ensure the controller is wired into fetch and cleared/handled after
the request completes or fails.
In `@docs/github-actions-workflows.md`:
- Around line 133-134: The Discord permissions documentation for the forum
workflow is incorrect: the bot needs thread-scoped posting permission rather
than listing Create Public Threads. Update the permissions text in the Discord
workflow docs to mention Send Messages in Threads for posting into existing
forum threads, and keep the other required permissions aligned with the forum
channel behavior described by the thread creation and message-posting flow.
🪄 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 Plus
Run ID: 8305e2ba-ed4d-4418-8092-a2fa146e981d
📒 Files selected for processing (7)
.github/scripts/discord-bot-api.mjs.github/scripts/discord-bot-api.test.mjs.github/scripts/discord-pr-sync.mjs.github/scripts/discord-weekly-leaderboard.mjs.github/workflows/discord-pr-notify.yml.github/workflows/discord-weekly-leaderboard.ymldocs/github-actions-workflows.md
Comment on lines
+5
to
+13
| async function callDiscord(botToken, method, path, body) { | ||
| const res = await fetch(`${API_BASE}${path}`, { | ||
| method, | ||
| headers: { | ||
| Authorization: `Bot ${botToken}`, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: body !== undefined ? JSON.stringify(body) : undefined, | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files:\n'
git ls-files '.github/scripts/*' | sed -n '1,120p'
printf '\nOutline discord-bot-api.mjs:\n'
ast-grep outline .github/scripts/discord-bot-api.mjs --view expanded || true
printf '\nRelevant source:\n'
cat -n .github/scripts/discord-bot-api.mjs | sed -n '1,220p'
printf '\nSearch for AbortController/timeout usage in scripts:\n'
rg -n --hidden --glob '.github/scripts/*' 'AbortController|setTimeout|timeout|signal:' .github/scripts || true
printf '\nCallers:\n'
rg -n --hidden --glob '.github/scripts/*' 'callDiscord\(' .github/scripts || trueRepository: getopenscreen/openscreen
Length of output: 3478
Add a timeout to callDiscord()
Every Discord request can hang indefinitely here, which can stall the PR sync and leaderboard workflows until the job times out. Add an AbortController-based timeout like the other Discord helper uses.
🤖 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 @.github/scripts/discord-bot-api.mjs around lines 5 - 13, The callDiscord
helper can hang indefinitely because it makes a fetch without any request
timeout, which can block PR sync and leaderboard workflows. Update callDiscord
in discord-bot-api.mjs to use an AbortController-based timeout, matching the
pattern used by the other Discord helper, and ensure the controller is wired
into fetch and cleared/handled after the request completes or fails.
Comment on lines
+133
to
+134
| All Discord traffic goes through a single bot (`DISCORD_BOT_TOKEN`, secret). The script creates the forum thread via `POST /channels/{forumChannelId}/threads` and posts review/comment updates into the existing thread via `POST /channels/{threadId}/messages`. Required bot permissions on the PR forum channel: View Channel, Send Messages, Embed Links, Create Public Threads, Manage Threads (for tag/archive/lock). Optional failure alerts can be sent to a separate channel via `DISCORD_ALERT_CHANNEL_ID` (variable); unset to silence. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the forum permissions Discord checks.
Forum posts and thread replies need Send Messages in Threads; Create Public Threads is not the right permission to list here. Without that, the bot can still hit 403s when it posts PR updates.
✏️ Proposed doc fix
-All Discord traffic goes through a single bot (`DISCORD_BOT_TOKEN`, secret). The script creates the forum thread via `POST /channels/{forumChannelId}/threads` and posts review/comment updates into the existing thread via `POST /channels/{threadId}/messages`. Required bot permissions on the PR forum channel: View Channel, Send Messages, Embed Links, Create Public Threads, Manage Threads (for tag/archive/lock). Optional failure alerts can be sent to a separate channel via `DISCORD_ALERT_CHANNEL_ID` (variable); unset to silence.
+All Discord traffic goes through a single bot (`DISCORD_BOT_TOKEN`, secret). The script creates the forum thread via `POST /channels/{forumChannelId}/threads` and posts review/comment updates into the existing thread via `POST /channels/{threadId}/messages`. Required bot permissions on the PR forum channel: View Channel, Send Messages, Send Messages in Threads, Embed Links, and Manage Threads (for tag/archive/lock). Optional failure alerts can be sent to a separate channel via `DISCORD_ALERT_CHANNEL_ID` (variable); unset to silence.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| All Discord traffic goes through a single bot (`DISCORD_BOT_TOKEN`, secret). The script creates the forum thread via `POST /channels/{forumChannelId}/threads` and posts review/comment updates into the existing thread via `POST /channels/{threadId}/messages`. Required bot permissions on the PR forum channel: View Channel, Send Messages, Embed Links, Create Public Threads, Manage Threads (for tag/archive/lock). Optional failure alerts can be sent to a separate channel via `DISCORD_ALERT_CHANNEL_ID` (variable); unset to silence. | |
| All Discord traffic goes through a single bot (`DISCORD_BOT_TOKEN`, secret). The script creates the forum thread via `POST /channels/{forumChannelId}/threads` and posts review/comment updates into the existing thread via `POST /channels/{threadId}/messages`. Required bot permissions on the PR forum channel: View Channel, Send Messages, Send Messages in Threads, Embed Links, and Manage Threads (for tag/archive/lock). Optional failure alerts can be sent to a separate channel via `DISCORD_ALERT_CHANNEL_ID` (variable); unset to silence. |
🤖 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 `@docs/github-actions-workflows.md` around lines 133 - 134, The Discord
permissions documentation for the forum workflow is incorrect: the bot needs
thread-scoped posting permission rather than listing Create Public Threads.
Update the permissions text in the Discord workflow docs to mention Send
Messages in Threads for posting into existing forum threads, and keep the other
required permissions aligned with the forum channel behavior described by the
thread creation and message-posting flow.
…ok migration
Three regressions from the bot-only migration, caught on self-review:
1. Webhook POSTs defaulted to allowed_mentions: { parse: [] }; bot POSTs
default to allowing all mentions. Without restoring it, the
<@&{reviewerRoleId}> embed in ready_for_review and changes_requested
review posts would start pinging the reviewer role, spamming
maintainers.
2. patchChannel now throws on non-ok (was: warn-and-return in the
inline patchDiscordThread). That made the user-facing message after
each tag/archive patch (PR merged, review state, etc.) skipped when
the patch failed, because the throw bubbled to the outer catch.
Added safePatchChannel that logs and continues so the message still
posts even if tagging fails.
3. Move THREAD_MARKER_REGEX back to the top of the file (the rewrite
dropped it below the function that uses it — TDZ-safe but ugly).
Also: drop the unused rateLimited flag from the bot-api error (no
consumer read it; the message is enough), and restore trailing
newlines on the two yml files.
5 near-duplicate tests (3 happy-path + 2 error, only on createForumThread) became 9 parameterized tests (3 cases × 3 paths) covering all 3 helpers uniformly, in 39 fewer lines. Side benefit: postChannelMessage and patchChannel now also have explicit rate-limit and error-path coverage.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/discord-bot-api.test.mjs (1)
50-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the remaining
callDiscordcontract.This table still leaves two important behaviors untested: it never asserts the
"Content-Type": "application/json"header, and it never exercises the204branch that should resolve tonullin.github/scripts/discord-bot-api.mjs. Adding both checks here would make the wrapper tests match the helper’s full request/response contract.Suggested test additions
describe.each(happyCases)("$name", ({ call, args, expectUrl, expectMethod, expectBody }) => { it("calls Discord with the right URL, method, bot auth, and payload", async () => { mockResponse(); await call({ ...args, botToken }); const [url, init] = vi.mocked(fetch).mock.calls[0]; expect(url).toBe(expectUrl); expect(init.method).toBe(expectMethod); expect(init.headers.Authorization).toBe(`Bot ${botToken}`); + expect(init.headers["Content-Type"]).toBe("application/json"); expect(JSON.parse(init.body)).toEqual(expectBody); }); + + it("returns null on 204", async () => { + mockResponse({ status: 204, body: {} }); + await expect(call({ ...args, botToken })).resolves.toBeNull(); + }); it("throws on 429 with rate-limit message", async () => { mockResponse({ status: 429, body: { retry_after: 1 } }); await expect(call({ ...args, botToken })).rejects.toThrow(/rate-limited \(429\)/); });🤖 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 @.github/scripts/discord-bot-api.test.mjs around lines 50 - 70, Update the `describe.each(happyCases)` tests in `discord-bot-api.test.mjs` to cover the full `callDiscord` contract: assert that the request sent by `call` includes the `"Content-Type": "application/json"` header alongside the bot auth header, and add a test case that exercises the `204` response branch so `callDiscord` resolves to `null` as implemented in `.github/scripts/discord-bot-api.mjs`. Use the existing `call`, `mockResponse`, and `happyCases` setup to keep the checks consistent with the current wrapper tests.
🤖 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 @.github/scripts/discord-bot-api.test.mjs:
- Around line 50-70: Update the `describe.each(happyCases)` tests in
`discord-bot-api.test.mjs` to cover the full `callDiscord` contract: assert that
the request sent by `call` includes the `"Content-Type": "application/json"`
header alongside the bot auth header, and add a test case that exercises the
`204` response branch so `callDiscord` resolves to `null` as implemented in
`.github/scripts/discord-bot-api.mjs`. Use the existing `call`, `mockResponse`,
and `happyCases` setup to keep the checks consistent with the current wrapper
tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e892549c-d592-4dc9-b17e-892607ef45b7
📒 Files selected for processing (1)
.github/scripts/discord-bot-api.test.mjs
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
1 participant
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.
Consolidates all Discord PR-review automations (intro post, thread updates, status mutations, weekly spotlight) under one bot identity (\DISCORD_BOT_TOKEN) instead of three separate webhook URLs. Removes the per-channel webhook secret-rotation surface and unifies rate limits, audit logs, and permissions under one app.
What changes
Bot display name + avatar are now the bot's configured Discord profile (set in the developer portal), no longer overridable per-message via \DISCORD_WEBHOOK_USERNAME\ / \DISCORD_WEBHOOK_AVATAR_URL.
Migration steps for the maintainer
Files
Testing
px vitest --run .github/scripts/\ → 12/12 pass (5 new + 7 existing).
px biome check .\ → clean.
Summary by CodeRabbit