diff --git a/.github/scripts/discord-release-announce.mjs b/.github/scripts/discord-release-announce.mjs index c4bdc5813..47d48fb15 100644 --- a/.github/scripts/discord-release-announce.mjs +++ b/.github/scripts/discord-release-announce.mjs @@ -1,4 +1,4 @@ -import { info, warning } from "@actions/core"; +import { info, setFailed, warning } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { createForumThread, postChannelMessage } from "./discord-bot-api.mjs"; @@ -14,13 +14,29 @@ const stableTag = (process.env.STABLE_TAG || "").trim(); const rcTag = (process.env.RC_TAG || "").trim(); const extra = (process.env.EXTRA || "").trim(); -if (!stableTag) { - warning("STABLE_TAG missing; skipping."); +// Set only by announce-release.yml. Every path below that ends without posting +// used to exit 0, which is right when prerelease.yml and promote.yml call this: +// the release is already out, the announcement is bookkeeping, and failing the +// job would misreport a successful release. A manual dispatch has the opposite +// contract — it exists *because* an announcement was missed, so a green run +// that posted nothing recreates the exact failure it was invoked to repair. +const strict = (process.env.STRICT || "").trim() !== ""; + +/** Ends the run without announcing: fatal under STRICT, a skip otherwise. */ +function bail(message, note = warning) { + if (strict) { + setFailed(message); + process.exit(1); + } + note(message); process.exit(0); } + +if (!stableTag) { + bail("STABLE_TAG missing; skipping."); +} if (!botToken || !channelId) { - info("Discord announce skipped: set DISCORD_BOT_TOKEN and a channel id variable."); - process.exit(0); + bail("Discord announce skipped: set DISCORD_BOT_TOKEN and a channel id variable.", info); } const owner = context.repo.owner; @@ -91,10 +107,11 @@ async function fetchChannelType() { }); if (!res.ok) { const txt = await res.text(); - warning(`Discord channel fetch failed ${res.status}: ${txt}`); - return null; + // Returned rather than reported here, so the single exit policy in bail() + // decides whether a failed lookup is fatal. + return { error: `Discord channel fetch failed ${res.status}: ${txt}` }; } - return res.json(); + return { channel: await res.json() }; } async function announceToForum() { @@ -125,9 +142,9 @@ async function announceToText() { info(`📣 ${kind} announcement posted to text channel (id=${result.id}).`); } -const channel = await fetchChannelType(); -if (!channel) { - process.exit(0); +const { channel, error } = await fetchChannelType(); +if (error) { + bail(error); } try { @@ -137,5 +154,13 @@ try { await announceToText(); } } catch (err) { - warning(`Discord announce failed: ${err?.message ?? err}`); + // Not bail(): this is the last statement, so there is nothing left to skip + // and the non-strict path must fall through rather than exit — "handles 4xx + // gracefully without throwing" pins that the module finishes on its own. + const message = `Discord announce failed: ${err?.message ?? err}`; + if (strict) { + setFailed(message); + process.exit(1); + } + warning(message); } diff --git a/.github/scripts/discord-release-announce.test.mjs b/.github/scripts/discord-release-announce.test.mjs index a5f23ddfb..1381c480b 100644 --- a/.github/scripts/discord-release-announce.test.mjs +++ b/.github/scripts/discord-release-announce.test.mjs @@ -1,3 +1,4 @@ +import { setFailed } from "@actions/core"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const mockCreateForumThread = vi.fn(); @@ -8,6 +9,7 @@ const mockListForRepo = vi.fn(); vi.mock("@actions/core", () => ({ info: vi.fn(), warning: vi.fn(), + setFailed: vi.fn(), })); vi.mock("@actions/github", () => ({ context: { @@ -52,6 +54,7 @@ const BASE_ENV = { beforeEach(() => { vi.stubGlobal("fetch", vi.fn()); + vi.mocked(setFailed).mockReset(); mockCreateForumThread.mockReset(); mockPostChannelMessage.mockReset(); mockListMilestones.mockReset(); @@ -65,7 +68,8 @@ beforeEach(() => { k === "STABLE_TAG" || k === "RC_TAG" || k === "KIND" || - k === "EXTRA" + k === "EXTRA" || + k === "STRICT" ) { delete process.env[k]; } @@ -185,3 +189,58 @@ describe("discord-release-announce", () => { expect(mockPostChannelMessage).toHaveBeenCalledTimes(1); }); }); + +// Every path below exits 0 without STRICT, which is correct when prerelease.yml +// and promote.yml call this: the release is already published and a failed +// announcement must not report it as broken. announce-release.yml is dispatched +// *because* an announcement was missed, so the same silence there recreates the +// failure it was invoked to repair — v1.9.0 shipped unannounced exactly that way. +describe("strict mode", () => { + const STRICT_ENV = { ...BASE_ENV, STRICT: "1", DISCORD_RELEASE_CHANNEL_ID: "123" }; + const exits1 = /process\.exit unexpectedly called with "1"/; + + it("fails when the Discord configuration is missing", async () => { + await expect(loadScript({ STABLE_TAG: "v1.5.0", STRICT: "1" })).rejects.toThrow(exits1); + expect(vi.mocked(setFailed)).toHaveBeenCalledWith(expect.stringContaining("DISCORD_BOT_TOKEN")); + }); + + it("fails when the channel lookup fails", async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: false, + status: 403, + text: async () => "Missing Access", + }); + + await expect(loadScript(STRICT_ENV)).rejects.toThrow(exits1); + expect(vi.mocked(setFailed)).toHaveBeenCalledWith(expect.stringContaining("403")); + }); + + it("fails when the post itself fails", async () => { + vi.mocked(fetch).mockResolvedValue({ ok: true, status: 200, json: async () => ({ type: 0 }) }); + mockPostChannelMessage.mockRejectedValue(new Error("failed 403: Missing Permissions")); + + await expect(loadScript(STRICT_ENV)).rejects.toThrow(exits1); + expect(vi.mocked(setFailed)).toHaveBeenCalledWith( + expect.stringContaining("Missing Permissions"), + ); + }); + + it("announces normally when everything works", async () => { + vi.mocked(fetch).mockResolvedValue({ ok: true, status: 200, json: async () => ({ type: 0 }) }); + mockPostChannelMessage.mockResolvedValue({ id: "42" }); + + await loadScript(STRICT_ENV); + + expect(mockPostChannelMessage).toHaveBeenCalledTimes(1); + expect(vi.mocked(setFailed)).not.toHaveBeenCalled(); + }); + + it("leaves the release workflows non-fatal without STRICT", async () => { + vi.mocked(fetch).mockResolvedValue({ ok: true, status: 200, json: async () => ({ type: 0 }) }); + mockPostChannelMessage.mockRejectedValue(new Error("failed 403: Missing Permissions")); + + await loadScript({ ...BASE_ENV, DISCORD_RELEASE_CHANNEL_ID: "123" }); + + expect(vi.mocked(setFailed)).not.toHaveBeenCalled(); + }); +}); diff --git a/.github/workflows/announce-release.yml b/.github/workflows/announce-release.yml new file mode 100644 index 000000000..563b1a3b7 --- /dev/null +++ b/.github/workflows/announce-release.yml @@ -0,0 +1,67 @@ +# Announcing a release lived only inside prerelease.yml and promote.yml, as the +# last step of each. Both promotions so far failed before reaching it — v1.8.0 on +# a merge conflict, v1.9.0 on a non-rebasable sync branch — and each time the +# announcement was silently skipped along with everything downstream of the +# failure, with no way to send it afterwards short of re-running the whole +# promotion over an already-tagged release. +# +# This exposes the same script on its own, so a missed announcement is a dispatch +# rather than a recovery operation. It reads nothing but the tag and the matching +# milestone, so it produces the identical message whenever it runs. +name: Announce a release on Discord + +on: + workflow_dispatch: + inputs: + tag: + description: "Release tag to announce (e.g. v1.9.0)" + required: true + type: string + kind: + description: "stable posts to the release channel, rc to the testing channel" + required: true + type: choice + options: [stable, rc] + default: stable + release_notes_extra: + description: "Optional message prepended to the announcement" + required: false + type: string + default: "" + +permissions: + contents: read + +jobs: + announce: + name: Announce ${{ inputs.tag }} + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: ./.github/actions/setup + + - name: Announce on Discord + env: + DISCORD_BOT_TOKEN: ${{ secrets.DISCORD_BOT_TOKEN }} + # The script prefers the RC channel whenever that variable is non-empty, + # so exactly one of these may be set — blanking the other is what selects + # the destination, not the KIND value. + DISCORD_RELEASE_CHANNEL_ID: ${{ inputs.kind == 'stable' && vars.DISCORD_RELEASE_CHANNEL_ID || '' }} + DISCORD_RC_TESTING_CHANNEL_ID: ${{ inputs.kind == 'rc' && vars.DISCORD_RC_TESTING_CHANNEL_ID || '' }} + # Only used to list the closed issues of the matching milestone; the + # announcement still posts without it, just without that section. + GITHUB_TOKEN: ${{ secrets.OPENSCREEN_RELEASE_TOKEN }} + STABLE_TAG: ${{ inputs.tag }} + EXTRA: ${{ inputs.release_notes_extra }} + KIND: ${{ inputs.kind }} + # Every path that ends without posting exits 0 by default, which is what + # prerelease.yml and promote.yml need — the release is already out and a + # failed announcement must not report it as broken. Here the contract is + # inverted: this workflow is dispatched *because* an announcement was + # missed, so a green run that posted nothing would recreate exactly the + # failure it was invoked to repair. That is how v1.9.0 shipped silent. + STRICT: "1" + run: node .github/scripts/discord-release-announce.mjs