From ec667bbdd5f2ef5c5f4af1949c1fdfdfa37f7a25 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 28 Jun 2026 20:16:05 +0200 Subject: [PATCH 1/3] refactor(ci): switch Discord automation from webhooks to single bot 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. --- .github/scripts/discord-bot-api.mjs | 42 +++++ .github/scripts/discord-bot-api.test.mjs | 101 ++++++++++ .github/scripts/discord-pr-sync.mjs | 178 ++++++++---------- .../scripts/discord-weekly-leaderboard.mjs | 29 ++- .github/workflows/discord-pr-notify.yml | 8 +- .../workflows/discord-weekly-leaderboard.yml | 7 +- docs/github-actions-workflows.md | 6 +- 7 files changed, 240 insertions(+), 131 deletions(-) create mode 100644 .github/scripts/discord-bot-api.mjs create mode 100644 .github/scripts/discord-bot-api.test.mjs diff --git a/.github/scripts/discord-bot-api.mjs b/.github/scripts/discord-bot-api.mjs new file mode 100644 index 000000000..b26a838cd --- /dev/null +++ b/.github/scripts/discord-bot-api.mjs @@ -0,0 +1,42 @@ +import { warning } from "@actions/core"; + +const API_BASE = "https://discord.com/api/v10"; + +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, + }); + + if (res.status === 429) { + const txt = await res.text(); + warning(`Discord rate-limited (429) on ${method} ${path}: ${txt}`); + const err = new Error(`Discord rate-limited (429)`); + err.rateLimited = true; + throw err; + } + + if (!res.ok) { + const txt = await res.text(); + throw new Error(`Discord API ${method} ${path} failed ${res.status}: ${txt}`); + } + + if (res.status === 204) return null; + return res.json(); +} + +export async function createForumThread({ botToken, forumChannelId, payload }) { + return callDiscord(botToken, "POST", `/channels/${forumChannelId}/threads`, payload); +} + +export async function postChannelMessage({ botToken, channelId, payload }) { + return callDiscord(botToken, "POST", `/channels/${channelId}/messages`, payload); +} + +export async function patchChannel({ botToken, channelId, payload }) { + return callDiscord(botToken, "PATCH", `/channels/${channelId}`, payload); +} diff --git a/.github/scripts/discord-bot-api.test.mjs b/.github/scripts/discord-bot-api.test.mjs new file mode 100644 index 000000000..d4b5e6f6f --- /dev/null +++ b/.github/scripts/discord-bot-api.test.mjs @@ -0,0 +1,101 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createForumThread, patchChannel, postChannelMessage } from "./discord-bot-api.mjs"; + +beforeEach(() => { + vi.stubGlobal("fetch", vi.fn()); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("discord-bot-api", () => { + const botToken = "test-token"; + + function mockDiscordResponse({ status = 200, body } = {}) { + const json = body === undefined ? vi.fn() : vi.fn().mockResolvedValue(body); + vi.mocked(fetch).mockResolvedValue({ + ok: status >= 200 && status < 300, + status, + text: vi.fn().mockResolvedValue(JSON.stringify(body)), + json, + }); + } + + describe("createForumThread", () => { + it("POSTs to /channels/{forumChannelId}/threads with bot auth and payload", async () => { + mockDiscordResponse({ status: 201, body: { id: "999", name: "PR #1" } }); + + const result = await createForumThread({ + botToken, + forumChannelId: "forum-1", + payload: { name: "PR #1", message: { content: "hi" }, applied_tags: ["tag-a"] }, + }); + + expect(result.id).toBe("999"); + const [url, init] = vi.mocked(fetch).mock.calls[0]; + expect(url).toBe("https://discord.com/api/v10/channels/forum-1/threads"); + expect(init.method).toBe("POST"); + expect(init.headers.Authorization).toBe(`Bot ${botToken}`); + expect(JSON.parse(init.body)).toEqual({ + name: "PR #1", + message: { content: "hi" }, + applied_tags: ["tag-a"], + }); + }); + + it("throws with rateLimited flag on 429", async () => { + mockDiscordResponse({ status: 429, body: { retry_after: 1 } }); + + await expect( + createForumThread({ botToken, forumChannelId: "f", payload: { name: "x" } }), + ).rejects.toMatchObject({ rateLimited: true }); + }); + + it("throws on non-ok responses with status in message", async () => { + mockDiscordResponse({ status: 403, body: { message: "Missing Permissions" } }); + + await expect( + createForumThread({ botToken, forumChannelId: "f", payload: { name: "x" } }), + ).rejects.toThrow(/failed 403/); + }); + }); + + describe("postChannelMessage", () => { + it("POSTs to /channels/{channelId}/messages with bot auth", async () => { + mockDiscordResponse({ status: 200, body: { id: "msg-1" } }); + + const result = await postChannelMessage({ + botToken, + channelId: "thread-1", + payload: { content: "hello", embeds: [{ title: "t" }] }, + }); + + expect(result.id).toBe("msg-1"); + const [url, init] = vi.mocked(fetch).mock.calls[0]; + expect(url).toBe("https://discord.com/api/v10/channels/thread-1/messages"); + expect(init.method).toBe("POST"); + expect(init.headers.Authorization).toBe(`Bot ${botToken}`); + expect(JSON.parse(init.body)).toEqual({ content: "hello", embeds: [{ title: "t" }] }); + }); + }); + + describe("patchChannel", () => { + it("PATCHes /channels/{channelId} with bot auth and JSON body", async () => { + mockDiscordResponse({ status: 200, body: { id: "thread-1", archived: true } }); + + const result = await patchChannel({ + botToken, + channelId: "thread-1", + payload: { archived: true, applied_tags: ["merged"] }, + }); + + expect(result.archived).toBe(true); + const [url, init] = vi.mocked(fetch).mock.calls[0]; + expect(url).toBe("https://discord.com/api/v10/channels/thread-1"); + expect(init.method).toBe("PATCH"); + expect(init.headers.Authorization).toBe(`Bot ${botToken}`); + expect(JSON.parse(init.body)).toEqual({ archived: true, applied_tags: ["merged"] }); + }); + }); +}); diff --git a/.github/scripts/discord-pr-sync.mjs b/.github/scripts/discord-pr-sync.mjs index 3a62dc819..0f9123265 100644 --- a/.github/scripts/discord-pr-sync.mjs +++ b/.github/scripts/discord-pr-sync.mjs @@ -1,20 +1,12 @@ import { info, warning } from "@actions/core"; import { context, getOctokit } from "@actions/github"; +import { createForumThread, patchChannel, postChannelMessage } from "./discord-bot-api.mjs"; import { validateThreadChannel } from "./discord-thread-validator.mjs"; -const WEBHOOK_USERNAME = (process.env.DISCORD_WEBHOOK_USERNAME || "OpenScreen").trim(); -const WEBHOOK_AVATAR = (process.env.DISCORD_WEBHOOK_AVATAR_URL || "").trim(); - -const THREAD_MARKER_REGEX = //i; -const webhookUrl = ( - process.env.DISCORD_WEBHOOK_URL || - process.env.DISCORD_PR_FORUM_WEBHOOK || - "" -).trim(); const botToken = (process.env.DISCORD_BOT_TOKEN || "").trim(); const reviewerRoleId = (process.env.DISCORD_REVIEWER_ROLE_ID || "").trim(); -const alertWebhookUrl = (process.env.DISCORD_ALERT_WEBHOOK_URL || "").trim(); const forumChannelId = (process.env.DISCORD_PR_FORUM_CHANNEL_ID || "").trim(); +const alertChannelId = (process.env.DISCORD_ALERT_CHANNEL_ID || "").trim(); const TAGS = { open: "1493976692967080096", @@ -52,59 +44,13 @@ function extractThreadId(body) { return match ? match[1] : null; } +const THREAD_MARKER_REGEX = //i; + function upsertThreadMarker(body, threadId) { const cleaned = (body || "").replace(THREAD_MARKER_REGEX, "").trim(); return `${cleaned}\n\n`.trim(); } -async function discordPost(payload, options = {}) { - const endpoint = new URL(webhookUrl); - endpoint.searchParams.set("wait", "true"); - if (options.threadId) endpoint.searchParams.set("thread_id", String(options.threadId)); - - const response = await fetch(endpoint.toString(), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - username: WEBHOOK_USERNAME, - avatar_url: WEBHOOK_AVATAR, - allowed_mentions: { parse: [] }, - ...payload, - }), - }); - - const contentType = (response.headers.get("content-type") || "").toLowerCase(); - const text = await response.text(); - - if (!response.ok) { - throw new Error(`Discord API error ${response.status}: ${text}`); - } - - if (!text) return {}; - if (contentType.includes("application/json")) return JSON.parse(text); - - warning( - `Discord webhook returned non-JSON response (content-type: ${contentType || "unknown"}).`, - ); - return {}; -} - -async function patchDiscordThread(threadId, patchBody) { - if (!botToken || !threadId) return; - const response = await fetch(`https://discord.com/api/v10/channels/${threadId}`, { - method: "PATCH", - headers: { - Authorization: `Bot ${botToken}`, - "Content-Type": "application/json", - }, - body: JSON.stringify(patchBody), - }); - if (!response.ok) { - const text = await response.text(); - warning(`Discord thread patch failed (${response.status}): ${text}`); - } -} - function desiredStatusTag(prState) { if (prState.merged && TAGS.merged) return TAGS.merged; if (prState.closed && !prState.merged && TAGS.closed) return TAGS.closed; @@ -170,10 +116,10 @@ async function main() { return; } - if (!webhookUrl) { + if (!botToken || !forumChannelId) { warning( - `Discord sync skipped: webhook secret unavailable for event '${context.eventName}'. ` + - "Set either DISCORD_WEBHOOK_URL or DISCORD_PR_FORUM_WEBHOOK in repository secrets.", + `Discord sync skipped: bot token or forum channel id unavailable for event '${context.eventName}'. ` + + "Set DISCORD_BOT_TOKEN (secret) and DISCORD_PR_FORUM_CHANNEL_ID (variable).", ); return; } @@ -228,38 +174,50 @@ async function main() { const appliedTags = [...new Set([statusTag, ...mappedLabelTags].filter(Boolean))].slice(0, 5); const createPayload = { - content: - action === "ready_for_review" - ? "πŸ”” PR is now ready for review" - : "πŸ”” New pull request opened", - thread_name: trimThreadName(`PR #${number} - ${title}`), + name: trimThreadName(`PR #${number} - ${title}`), + auto_archive_duration: 4320, applied_tags: appliedTags, - embeds: [ - { - title: `PR #${number}: ${title}`, - url, - description: cleanDescription(body), - color: pr.draft ? 15105570 : 1998671, - author: { - name: author, - url: authorUrl || undefined, - icon_url: authorAvatar || undefined, + message: { + content: + action === "ready_for_review" + ? "πŸ”” PR is now ready for review" + : "πŸ”” New pull request opened", + embeds: [ + { + title: `PR #${number}: ${title}`, + url, + description: cleanDescription(body), + color: pr.draft ? 15105570 : 1998671, + author: { + name: author, + url: authorUrl || undefined, + icon_url: authorAvatar || undefined, + }, + fields, + footer: { text: repoFullName }, + timestamp: new Date().toISOString(), }, - fields, - footer: { text: repoFullName }, - timestamp: new Date().toISOString(), - }, - ], + ], + }, }; - const result = await discordPost(createPayload); - const createdThreadId = result.channel_id || null; + const thread = await createForumThread({ + botToken, + forumChannelId, + payload: createPayload, + }); + const createdThreadId = thread?.id || null; if (createdThreadId) { const updatedBody = upsertThreadMarker(body, createdThreadId); - await octokit.rest.pulls.update({ owner, repo, pull_number: number, body: updatedBody }); + await octokit.rest.pulls.update({ + owner, + repo, + pull_number: number, + body: updatedBody, + }); info(`Created Discord thread ${createdThreadId} and stored mapping.`); } else { - warning("Discord thread created but channel_id missing in response."); + warning("Discord thread created but id missing in response."); } return; } @@ -286,9 +244,13 @@ async function main() { }); const mappedLabelTags = tagIdsFromLabels(labels); const appliedTags = [...new Set([statusTag, ...mappedLabelTags].filter(Boolean))].slice(0, 5); - await patchDiscordThread(threadId, { - name: trimThreadName(`PR #${number} - ${title}`), - ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + await patchChannel({ + botToken, + channelId: threadId, + payload: { + name: trimThreadName(`PR #${number} - ${title}`), + ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + }, }); } @@ -338,9 +300,13 @@ async function main() { 0, 5, ); - await patchDiscordThread(threadId, { - ...(appliedTags.length ? { applied_tags: appliedTags } : {}), - ...(isMerged ? { archived: true, locked: true } : {}), + await patchChannel({ + botToken, + channelId: threadId, + payload: { + ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + ...(isMerged ? { archived: true, locked: true } : {}), + }, }); updateMessage = isMerged @@ -390,8 +356,12 @@ async function main() { 0, 5, ); - await patchDiscordThread(threadId, { - ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + await patchChannel({ + botToken, + channelId: threadId, + payload: { + ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + }, }); } } @@ -417,7 +387,7 @@ async function main() { const payload = { content: updateMessage || "" }; if (updateEmbed) payload.embeds = [updateEmbed]; - await discordPost(payload, { threadId }); + await postChannelMessage({ botToken, channelId: threadId, payload }); info(`Posted update to Discord thread ${threadId}.`); } catch (err) { const msg = err && err.message ? err.message : String(err); @@ -425,20 +395,20 @@ async function main() { `Discord sync failed, but this optional automation will not block PR validation: ${msg}`, ); - if (alertWebhookUrl) { + if (alertChannelId) { try { - await fetch(alertWebhookUrl, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - username: "OpenScreen", - avatar_url: WEBHOOK_AVATAR, + await postChannelMessage({ + botToken, + channelId: alertChannelId, + payload: { content: `⚠️ PR->Discord sync failed\n${msg}\nRun: ${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`, allowed_mentions: { parse: [] }, - }), + }, }); - } catch { - warning("Failed to send alert webhook."); + } catch (alertErr) { + warning( + `Failed to send alert message: ${alertErr && alertErr.message ? alertErr.message : alertErr}`, + ); } } } diff --git a/.github/scripts/discord-weekly-leaderboard.mjs b/.github/scripts/discord-weekly-leaderboard.mjs index bcf3192cf..0bfe180a4 100644 --- a/.github/scripts/discord-weekly-leaderboard.mjs +++ b/.github/scripts/discord-weekly-leaderboard.mjs @@ -1,13 +1,13 @@ import { info, warning } from "@actions/core"; import { context, getOctokit } from "@actions/github"; +import { postChannelMessage } from "./discord-bot-api.mjs"; -const spotlightWebhook = (process.env.DISCORD_SPOTLIGHT_WEBHOOK_URL || "").trim(); -const webhookUsername = (process.env.DISCORD_WEBHOOK_USERNAME || "OpenScreen").trim(); -const webhookAvatar = (process.env.DISCORD_WEBHOOK_AVATAR_URL || "").trim(); +const botToken = (process.env.DISCORD_BOT_TOKEN || "").trim(); +const spotlightChannelId = (process.env.DISCORD_SPOTLIGHT_CHANNEL_ID || "").trim(); async function main() { - if (!spotlightWebhook) { - info("DISCORD_SPOTLIGHT_WEBHOOK_URL missing. Skipping leaderboard post."); + if (!botToken || !spotlightChannelId) { + info("DISCORD_BOT_TOKEN or DISCORD_SPOTLIGHT_CHANNEL_ID missing. Skipping leaderboard post."); return; } @@ -45,8 +45,6 @@ async function main() { : "No merged PRs this week."; const payload = { - username: webhookUsername, - ...(webhookAvatar ? { avatar_url: webhookAvatar } : {}), embeds: [ { title: "🌟 Weekly Contributor Leaderboard", @@ -63,15 +61,14 @@ async function main() { allowed_mentions: { parse: [] }, }; - const res = await fetch(`${spotlightWebhook}?wait=true`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(payload), - }); - - if (!res.ok) { - const txt = await res.text(); - warning(`Leaderboard post failed ${res.status}: ${txt}`); + try { + await postChannelMessage({ + botToken, + channelId: spotlightChannelId, + payload, + }); + } catch (err) { + warning(`Leaderboard post failed: ${err && err.message ? err.message : err}`); } } diff --git a/.github/workflows/discord-pr-notify.yml b/.github/workflows/discord-pr-notify.yml index 8c97f78ec..7e5323757 100644 --- a/.github/workflows/discord-pr-notify.yml +++ b/.github/workflows/discord-pr-notify.yml @@ -32,13 +32,9 @@ jobs: - name: Sync PR activity to Discord forum thread continue-on-error: true env: - DISCORD_WEBHOOK_URL: ${{ secrets.DISCORD_WEBHOOK_URL }} - DISCORD_PR_FORUM_WEBHOOK: ${{ secrets.DISCORD_PR_FORUM_WEBHOOK }} - DISCORD_WEBHOOK_USERNAME: ${{ secrets.DISCORD_WEBHOOK_USERNAME }} - DISCORD_WEBHOOK_AVATAR_URL: ${{ secrets.DISCORD_WEBHOOK_AVATAR_URL }} DISCORD_BOT_TOKEN: ${{ secrets.DISCORD_BOT_TOKEN }} DISCORD_REVIEWER_ROLE_ID: ${{ secrets.DISCORD_REVIEWER_ROLE_ID }} - DISCORD_ALERT_WEBHOOK_URL: ${{ secrets.DISCORD_ALERT_WEBHOOK_URL }} DISCORD_PR_FORUM_CHANNEL_ID: ${{ vars.DISCORD_PR_FORUM_CHANNEL_ID }} + DISCORD_ALERT_CHANNEL_ID: ${{ vars.DISCORD_ALERT_CHANNEL_ID }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: node .github/scripts/discord-pr-sync.mjs + run: node .github/scripts/discord-pr-sync.mjs \ No newline at end of file diff --git a/.github/workflows/discord-weekly-leaderboard.yml b/.github/workflows/discord-weekly-leaderboard.yml index f2c93e453..770efc52f 100644 --- a/.github/workflows/discord-weekly-leaderboard.yml +++ b/.github/workflows/discord-weekly-leaderboard.yml @@ -21,8 +21,7 @@ jobs: - name: Post weekly leaderboard to Discord env: - DISCORD_SPOTLIGHT_WEBHOOK_URL: ${{ secrets.DISCORD_SPOTLIGHT_WEBHOOK_URL }} - DISCORD_WEBHOOK_USERNAME: ${{ secrets.DISCORD_WEBHOOK_USERNAME }} - DISCORD_WEBHOOK_AVATAR_URL: ${{ secrets.DISCORD_WEBHOOK_AVATAR_URL }} + DISCORD_BOT_TOKEN: ${{ secrets.DISCORD_BOT_TOKEN }} + DISCORD_SPOTLIGHT_CHANNEL_ID: ${{ vars.DISCORD_SPOTLIGHT_CHANNEL_ID }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: node .github/scripts/discord-weekly-leaderboard.mjs + run: node .github/scripts/discord-weekly-leaderboard.mjs \ No newline at end of file diff --git a/docs/github-actions-workflows.md b/docs/github-actions-workflows.md index 282fa20dd..3d1d6f80e 100644 --- a/docs/github-actions-workflows.md +++ b/docs/github-actions-workflows.md @@ -130,6 +130,8 @@ Triggered by `pull_request_target` (opened, reopened, synchronize, edited, label Runs `node .github/scripts/discord-pr-sync.mjs`, which creates or updates a Discord forum thread for each PR. Thread state is persisted via an HTML comment (``) in the PR body. Tag updates (draft, ready, changes requested, approved, merged, closed) are applied via the Discord API. The job is marked `continue-on-error: true` so that Discord failures never block the PR workflow. +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. + ### discord-roadmap-sync.yml Triggered on push to `main` and on merged PRs targeting `main`. Runs `node .github/scripts/discord-roadmap-sync.mjs`, which: @@ -143,7 +145,9 @@ Requires `DISCORD_BOT_TOKEN` (secret) and `DISCORD_ROADMAP_CHANNEL_ID` (variable ### discord-weekly-leaderboard.yml -Triggered by schedule (Mondays at 12:00 UTC) and `workflow_dispatch`. Runs `node .github/scripts/discord-weekly-leaderboard.mjs`, which queries the GitHub Search API for merged PRs in the last 7 days, ranks contributors by PR count, and posts a top-10 leaderboard to a Discord webhook. +Triggered by schedule (Mondays at 12:00 UTC) and `workflow_dispatch`. Runs `node .github/scripts/discord-weekly-leaderboard.mjs`, which queries the GitHub Search API for merged PRs in the last 7 days, ranks contributors by PR count, and posts a top-10 leaderboard to the `#πŸŒŸγƒ»contributor-spotlight` channel via the same bot (`DISCORD_BOT_TOKEN`). + +Requires `DISCORD_SPOTLIGHT_CHANNEL_ID` (variable) and the bot to have View Channel + Send Messages + Embed Links on that channel. ### merged-pr-bookkeeping.yml From eb4266120684bf5198f2d1559ba6ef2c65a66a62 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 28 Jun 2026 20:37:07 +0200 Subject: [PATCH 2/3] fix(ci): restore mention suppression and patch resilience after webhook migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/scripts/discord-bot-api.mjs | 4 +- .github/scripts/discord-bot-api.test.mjs | 4 +- .github/scripts/discord-pr-sync.mjs | 70 ++++++++++++------- .github/workflows/discord-pr-notify.yml | 2 +- .../workflows/discord-weekly-leaderboard.yml | 2 +- 5 files changed, 51 insertions(+), 31 deletions(-) diff --git a/.github/scripts/discord-bot-api.mjs b/.github/scripts/discord-bot-api.mjs index b26a838cd..531a98f7e 100644 --- a/.github/scripts/discord-bot-api.mjs +++ b/.github/scripts/discord-bot-api.mjs @@ -15,9 +15,7 @@ async function callDiscord(botToken, method, path, body) { if (res.status === 429) { const txt = await res.text(); warning(`Discord rate-limited (429) on ${method} ${path}: ${txt}`); - const err = new Error(`Discord rate-limited (429)`); - err.rateLimited = true; - throw err; + throw new Error(`Discord rate-limited (429) on ${method} ${path}`); } if (!res.ok) { diff --git a/.github/scripts/discord-bot-api.test.mjs b/.github/scripts/discord-bot-api.test.mjs index d4b5e6f6f..f0190892e 100644 --- a/.github/scripts/discord-bot-api.test.mjs +++ b/.github/scripts/discord-bot-api.test.mjs @@ -44,12 +44,12 @@ describe("discord-bot-api", () => { }); }); - it("throws with rateLimited flag on 429", async () => { + it("throws a rate-limited error on 429", async () => { mockDiscordResponse({ status: 429, body: { retry_after: 1 } }); await expect( createForumThread({ botToken, forumChannelId: "f", payload: { name: "x" } }), - ).rejects.toMatchObject({ rateLimited: true }); + ).rejects.toThrow(/rate-limited \(429\)/); }); it("throws on non-ok responses with status in message", async () => { diff --git a/.github/scripts/discord-pr-sync.mjs b/.github/scripts/discord-pr-sync.mjs index 0f9123265..a40e34f4b 100644 --- a/.github/scripts/discord-pr-sync.mjs +++ b/.github/scripts/discord-pr-sync.mjs @@ -8,6 +8,8 @@ const reviewerRoleId = (process.env.DISCORD_REVIEWER_ROLE_ID || "").trim(); const forumChannelId = (process.env.DISCORD_PR_FORUM_CHANNEL_ID || "").trim(); const alertChannelId = (process.env.DISCORD_ALERT_CHANNEL_ID || "").trim(); +const THREAD_MARKER_REGEX = //i; + const TAGS = { open: "1493976692967080096", draft: "1493976782028935279", @@ -44,13 +46,23 @@ function extractThreadId(body) { return match ? match[1] : null; } -const THREAD_MARKER_REGEX = //i; - function upsertThreadMarker(body, threadId) { const cleaned = (body || "").replace(THREAD_MARKER_REGEX, "").trim(); return `${cleaned}\n\n`.trim(); } +const NO_MENTIONS = { allowed_mentions: { parse: [] } }; + +async function safePatchChannel(args, contextLabel) { + try { + await patchChannel(args); + } catch (err) { + warning( + `Discord thread patch failed for ${contextLabel} (continuing): ${err && err.message ? err.message : err}`, + ); + } +} + function desiredStatusTag(prState) { if (prState.merged && TAGS.merged) return TAGS.merged; if (prState.closed && !prState.merged && TAGS.closed) return TAGS.closed; @@ -198,6 +210,7 @@ async function main() { timestamp: new Date().toISOString(), }, ], + allowed_mentions: { parse: [] }, }, }; @@ -244,14 +257,17 @@ async function main() { }); const mappedLabelTags = tagIdsFromLabels(labels); const appliedTags = [...new Set([statusTag, ...mappedLabelTags].filter(Boolean))].slice(0, 5); - await patchChannel({ - botToken, - channelId: threadId, - payload: { - name: trimThreadName(`PR #${number} - ${title}`), - ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + await safePatchChannel( + { + botToken, + channelId: threadId, + payload: { + name: trimThreadName(`PR #${number} - ${title}`), + ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + }, }, - }); + `tag refresh on ${action}`, + ); } let updateMessage = null; @@ -300,14 +316,17 @@ async function main() { 0, 5, ); - await patchChannel({ - botToken, - channelId: threadId, - payload: { - ...(appliedTags.length ? { applied_tags: appliedTags } : {}), - ...(isMerged ? { archived: true, locked: true } : {}), + await safePatchChannel( + { + botToken, + channelId: threadId, + payload: { + ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + ...(isMerged ? { archived: true, locked: true } : {}), + }, }, - }); + `close (${isMerged ? "merged" : "closed without merge"})`, + ); updateMessage = isMerged ? `βœ… PR #${number} was merged` @@ -356,13 +375,16 @@ async function main() { 0, 5, ); - await patchChannel({ - botToken, - channelId: threadId, - payload: { - ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + await safePatchChannel( + { + botToken, + channelId: threadId, + payload: { + ...(appliedTags.length ? { applied_tags: appliedTags } : {}), + }, }, - }); + `review ${state}`, + ); } } } else if (context.eventName === "issue_comment") { @@ -385,7 +407,7 @@ async function main() { return; } - const payload = { content: updateMessage || "" }; + const payload = { content: updateMessage || "", ...NO_MENTIONS }; if (updateEmbed) payload.embeds = [updateEmbed]; await postChannelMessage({ botToken, channelId: threadId, payload }); info(`Posted update to Discord thread ${threadId}.`); @@ -402,7 +424,7 @@ async function main() { channelId: alertChannelId, payload: { content: `⚠️ PR->Discord sync failed\n${msg}\nRun: ${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`, - allowed_mentions: { parse: [] }, + ...NO_MENTIONS, }, }); } catch (alertErr) { diff --git a/.github/workflows/discord-pr-notify.yml b/.github/workflows/discord-pr-notify.yml index 7e5323757..20ed1c47c 100644 --- a/.github/workflows/discord-pr-notify.yml +++ b/.github/workflows/discord-pr-notify.yml @@ -37,4 +37,4 @@ jobs: DISCORD_PR_FORUM_CHANNEL_ID: ${{ vars.DISCORD_PR_FORUM_CHANNEL_ID }} DISCORD_ALERT_CHANNEL_ID: ${{ vars.DISCORD_ALERT_CHANNEL_ID }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: node .github/scripts/discord-pr-sync.mjs \ No newline at end of file + run: node .github/scripts/discord-pr-sync.mjs diff --git a/.github/workflows/discord-weekly-leaderboard.yml b/.github/workflows/discord-weekly-leaderboard.yml index 770efc52f..9724e36de 100644 --- a/.github/workflows/discord-weekly-leaderboard.yml +++ b/.github/workflows/discord-weekly-leaderboard.yml @@ -24,4 +24,4 @@ jobs: DISCORD_BOT_TOKEN: ${{ secrets.DISCORD_BOT_TOKEN }} DISCORD_SPOTLIGHT_CHANNEL_ID: ${{ vars.DISCORD_SPOTLIGHT_CHANNEL_ID }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: node .github/scripts/discord-weekly-leaderboard.mjs \ No newline at end of file + run: node .github/scripts/discord-weekly-leaderboard.mjs From 0c5c1b873d703afd669a7faf0e2457751d07c455 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Sun, 28 Jun 2026 20:41:25 +0200 Subject: [PATCH 3/3] test(ci): collapse discord-bot-api tests via describe.each MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/scripts/discord-bot-api.test.mjs | 139 +++++++++-------------- 1 file changed, 55 insertions(+), 84 deletions(-) diff --git a/.github/scripts/discord-bot-api.test.mjs b/.github/scripts/discord-bot-api.test.mjs index f0190892e..a0a586ac2 100644 --- a/.github/scripts/discord-bot-api.test.mjs +++ b/.github/scripts/discord-bot-api.test.mjs @@ -1,6 +1,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createForumThread, patchChannel, postChannelMessage } from "./discord-bot-api.mjs"; +const botToken = "test-token"; + beforeEach(() => { vi.stubGlobal("fetch", vi.fn()); }); @@ -9,93 +11,62 @@ afterEach(() => { vi.unstubAllGlobals(); }); -describe("discord-bot-api", () => { - const botToken = "test-token"; - - function mockDiscordResponse({ status = 200, body } = {}) { - const json = body === undefined ? vi.fn() : vi.fn().mockResolvedValue(body); - vi.mocked(fetch).mockResolvedValue({ - ok: status >= 200 && status < 300, - status, - text: vi.fn().mockResolvedValue(JSON.stringify(body)), - json, - }); - } - - describe("createForumThread", () => { - it("POSTs to /channels/{forumChannelId}/threads with bot auth and payload", async () => { - mockDiscordResponse({ status: 201, body: { id: "999", name: "PR #1" } }); - - const result = await createForumThread({ - botToken, - forumChannelId: "forum-1", - payload: { name: "PR #1", message: { content: "hi" }, applied_tags: ["tag-a"] }, - }); - - expect(result.id).toBe("999"); - const [url, init] = vi.mocked(fetch).mock.calls[0]; - expect(url).toBe("https://discord.com/api/v10/channels/forum-1/threads"); - expect(init.method).toBe("POST"); - expect(init.headers.Authorization).toBe(`Bot ${botToken}`); - expect(JSON.parse(init.body)).toEqual({ - name: "PR #1", - message: { content: "hi" }, - applied_tags: ["tag-a"], - }); - }); - - it("throws a rate-limited error on 429", async () => { - mockDiscordResponse({ status: 429, body: { retry_after: 1 } }); - - await expect( - createForumThread({ botToken, forumChannelId: "f", payload: { name: "x" } }), - ).rejects.toThrow(/rate-limited \(429\)/); - }); - - it("throws on non-ok responses with status in message", async () => { - mockDiscordResponse({ status: 403, body: { message: "Missing Permissions" } }); - - await expect( - createForumThread({ botToken, forumChannelId: "f", payload: { name: "x" } }), - ).rejects.toThrow(/failed 403/); - }); +function mockResponse({ status = 200, body = { id: "x" } } = {}) { + vi.mocked(fetch).mockResolvedValue({ + ok: status >= 200 && status < 300, + status, + text: vi.fn().mockResolvedValue(JSON.stringify(body)), + json: vi.fn().mockResolvedValue(body), }); - - describe("postChannelMessage", () => { - it("POSTs to /channels/{channelId}/messages with bot auth", async () => { - mockDiscordResponse({ status: 200, body: { id: "msg-1" } }); - - const result = await postChannelMessage({ - botToken, - channelId: "thread-1", - payload: { content: "hello", embeds: [{ title: "t" }] }, - }); - - expect(result.id).toBe("msg-1"); - const [url, init] = vi.mocked(fetch).mock.calls[0]; - expect(url).toBe("https://discord.com/api/v10/channels/thread-1/messages"); - expect(init.method).toBe("POST"); - expect(init.headers.Authorization).toBe(`Bot ${botToken}`); - expect(JSON.parse(init.body)).toEqual({ content: "hello", embeds: [{ title: "t" }] }); - }); +} + +const happyCases = [ + { + name: "createForumThread", + call: (args) => createForumThread(args), + args: { forumChannelId: "forum-1", payload: { name: "PR #1" } }, + expectUrl: "https://discord.com/api/v10/channels/forum-1/threads", + expectMethod: "POST", + expectBody: { name: "PR #1" }, + }, + { + name: "postChannelMessage", + call: (args) => postChannelMessage(args), + args: { channelId: "thread-1", payload: { content: "hello" } }, + expectUrl: "https://discord.com/api/v10/channels/thread-1/messages", + expectMethod: "POST", + expectBody: { content: "hello" }, + }, + { + name: "patchChannel", + call: (args) => patchChannel(args), + args: { channelId: "thread-1", payload: { archived: true } }, + expectUrl: "https://discord.com/api/v10/channels/thread-1", + expectMethod: "PATCH", + expectBody: { archived: true }, + }, +]; + +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(JSON.parse(init.body)).toEqual(expectBody); }); - describe("patchChannel", () => { - it("PATCHes /channels/{channelId} with bot auth and JSON body", async () => { - mockDiscordResponse({ status: 200, body: { id: "thread-1", archived: true } }); - - const result = await patchChannel({ - botToken, - channelId: "thread-1", - payload: { archived: true, applied_tags: ["merged"] }, - }); + 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\)/); + }); - expect(result.archived).toBe(true); - const [url, init] = vi.mocked(fetch).mock.calls[0]; - expect(url).toBe("https://discord.com/api/v10/channels/thread-1"); - expect(init.method).toBe("PATCH"); - expect(init.headers.Authorization).toBe(`Bot ${botToken}`); - expect(JSON.parse(init.body)).toEqual({ archived: true, applied_tags: ["merged"] }); - }); + it("throws on non-ok responses with status in message", async () => { + mockResponse({ status: 403, body: { message: "Missing Permissions" } }); + await expect(call({ ...args, botToken })).rejects.toThrow(/failed 403/); }); });