discord: retry the webhook instead of dropping the message

.github/workflows/discord_pr.yml

@@ -35,53 +35,66 @@ jobs:
   notify_pr:
     runs-on: ubuntu-latest
     steps:
-      - name: Send PR notification to Discord
+      - name: Compose the message
+        id: compose
         env:
           PR_TITLE: ${{ inputs.pr_title }}
           PR_NUMBER: ${{ inputs.pr_number }}
           PR_URL: ${{ inputs.pr_url }}
           REPO_FULL_NAME: ${{ inputs.repo_full_name }}
           PR_DRAFT: ${{ inputs.pr_draft }}
-          DISCORD_WEBHOOK: ${{ secrets.discord_webhook }}
+          # Through env, not interpolated into the body: a PR title is
+          # attacker-controlled text and must not be able to become shell.
+          EVENT_ACTION: ${{ github.event.action }}
+          PR_MERGED: ${{ github.event.pull_request.merged }}
         run: |
           # Determine the message based on the event type and draft status
-          if [[ "${{ github.event.action }}" == "opened" ]]; then
+          if [[ "$EVENT_ACTION" == "opened" ]]; then
             if [[ "$PR_DRAFT" == "true" ]]; then
               ACTION_ICON="📝"
               ACTION_TEXT="draft has been opened."
             else
               ACTION_ICON="<:devpropened:1275356590316322858>"
               ACTION_TEXT="has been opened. Please review!"
             fi
-          elif [[ "${{ github.event.action }}" == "ready_for_review" ]]; then
+          elif [[ "$EVENT_ACTION" == "ready_for_review" ]]; then
             ACTION_ICON="<:devpropened:1275356590316322858>"
             ACTION_TEXT="is ready for review!"
-          elif [[ "${{ github.event.action }}" == "converted_to_draft" ]]; then
+          elif [[ "$EVENT_ACTION" == "converted_to_draft" ]]; then
             ACTION_ICON="📝"
             ACTION_TEXT="has been converted to draft."
-          elif [[ "${{ github.event.pull_request.merged }}" == "true" ]]; then
+          elif [[ "$PR_MERGED" == "true" ]]; then
             ACTION_ICON="<:devprmerged:1275356589284393021>"
             ACTION_TEXT="has been merged!"
-          elif [[ "${{ github.event.action }}" == "closed" ]]; then
+          elif [[ "$EVENT_ACTION" == "closed" ]]; then
             ACTION_ICON="<:devprclosed:1275356586218356776>"
             ACTION_TEXT="has been closed!"
           else
             echo "Unhandled event type. Exiting."
+            echo "send=false" >> "$GITHUB_OUTPUT"
             exit 0
           fi
 
-          REPO_NAME=$(echo $REPO_FULL_NAME | cut -d'/' -f2)
-          MESSAGE=$(jq -n \
-            --arg repo "$REPO_NAME" \
-            --arg number "$PR_NUMBER" \
-            --arg title "$PR_TITLE" \
-            --arg url "$PR_URL" \
-            --arg icon "$ACTION_ICON" \
-            --arg action "$ACTION_TEXT" \
-            '{content: "\($icon) \($repo) > [PR #\($number): \($title)](<\($url)>) \($action)"}')
+          REPO_NAME=${REPO_FULL_NAME#*/}
+          CONTENT="$ACTION_ICON $REPO_NAME > [PR #$PR_NUMBER: $PR_TITLE](<$PR_URL>) $ACTION_TEXT"
+
+          # Random delimiter: a title cannot be allowed to close the heredoc
+          # and inject further outputs.
+          delim="EOF_${RANDOM}${RANDOM}"
+          {
+            printf 'content<<%s\n' "$delim"
+            printf '%s\n' "$CONTENT"
+            printf '%s\n' "$delim"
+            printf 'send=true\n'
+          } >> "$GITHUB_OUTPUT"
 
-          # Send the notification to Discord
-          curl -H "Content-Type: application/json" \
-            -X POST \
-            -d "$MESSAGE" \
-            "$DISCORD_WEBHOOK"
+      # The action retries a rate-limited webhook and exits non-zero if the
+      # message never lands. Sending it is this workflow's whole purpose, so
+      # that failure is allowed to redden the run — nothing else would record
+      # that the notification vanished.
+      - name: Send PR notification to Discord
+        if: steps.compose.outputs.send == 'true'
+        uses: novem-code/github-actions/discord-notify@d9a94f342c5cdacb85f91a2f30d0000004f6ca73
+        with:
+          webhook-url: ${{ secrets.discord_webhook }}
+          content: ${{ steps.compose.outputs.content }}

.github/workflows/discord_push.yml

@@ -45,27 +45,33 @@ jobs:
             echo "is_direct_commit=false" >> $GITHUB_OUTPUT
           fi
 
-      - name: Send push notification to Discord
+      - name: Compose the message
+        id: compose
         if: steps.input_check.outputs.is_direct_commit == 'true'
         env:
           BRANCH_NAME: ${{ steps.input_check.outputs.branch_name }}
           COMMIT_MESSAGE: ${{ inputs.commit_message }}
           COMMIT_URL: ${{ inputs.commit_url }}
           REPO_FULL_NAME: ${{ inputs.repo_full_name }}
-          DISCORD_WEBHOOK: ${{ secrets.discord_webhook }}
         run: |
-          REPO_NAME=$(echo $REPO_FULL_NAME | cut -d'/' -f2)
-          COMMIT_TITLE="$(echo "$COMMIT_MESSAGE" | head -1)"
-          MESSAGE=$(jq -n \
-            --arg repo "$REPO_NAME" \
-            --arg branch_name "$BRANCH_NAME" \
-            --arg commit_title "$COMMIT_TITLE" \
-            --arg commit_url "$COMMIT_URL" \
-            --arg icon "<:devcommit:1275358039884435517>" \
-            '{content: "\($icon) \($repo) > Direct push to \($branch_name): [\($commit_title)](<\($commit_url)>)"}')
+          REPO_NAME=${REPO_FULL_NAME#*/}
+          COMMIT_TITLE="$(printf '%s\n' "$COMMIT_MESSAGE" | head -1)"
+          CONTENT="<:devcommit:1275358039884435517> $REPO_NAME > Direct push to $BRANCH_NAME: [$COMMIT_TITLE](<$COMMIT_URL>)"
+
+          # Random delimiter: a commit title cannot be allowed to close the
+          # heredoc and inject further outputs.
+          delim="EOF_${RANDOM}${RANDOM}"
+          {
+            printf 'content<<%s\n' "$delim"
+            printf '%s\n' "$CONTENT"
+            printf '%s\n' "$delim"
+          } >> "$GITHUB_OUTPUT"
 
-          # Send the notification to Discord
-          curl -H "Content-Type: application/json" \
-            -X POST \
-            -d "$MESSAGE" \
-            "$DISCORD_WEBHOOK"
+      # Sending the message is this workflow's whole purpose, so an
+      # undeliverable one is allowed to redden the run.
+      - name: Send push notification to Discord
+        if: steps.input_check.outputs.is_direct_commit == 'true'
+        uses: novem-code/github-actions/discord-notify@d9a94f342c5cdacb85f91a2f30d0000004f6ca73
+        with:
+          webhook-url: ${{ secrets.discord_webhook }}
+          content: ${{ steps.compose.outputs.content }}

README.md

@@ -1,2 +1,43 @@
 # github-actions
 A collection of reusable GitHub Actions for Novem
+
+## `discord-notify`
+
+Post one message to a Discord webhook, honouring the rate limit. A plain
+`curl` exits 0 whatever Discord answers, so a 429 — what several workflows
+firing at once earns you — printed the rate-limit body into the log and left
+the step green with the message thrown away.
+
+```yaml
+- uses: novem-code/github-actions/discord-notify@<full-sha>
+  with:
+    webhook-url: ${{ secrets.DISCORD_WEBHOOK_URL }}
+    content: "🚀 deploy finished"
+```
+
+Pin the **full commit SHA**, never a tag or branch: refs here are mutable.
+
+| Input | Required | Default | |
+|---|---|---|---|
+| `webhook-url` | yes | | The webhook. A composite action cannot declare `secrets:`, so pass the secret as this input; masking is by value, so it stays redacted. |
+| `content` | one of | `''` | Message text. JSON-encoded for you — quotes and newlines are safe without reaching for `jq`. |
+| `payload` | one of | `''` | A complete Discord JSON object, for embeds. Give exactly one of `content` / `payload`. |
+| `fail-on-undeliverable` | no | `'true'` | Whether an undeliverable message fails the step. |
+
+Output `delivered` is `"true"`/`"false"`.
+
+### Which way to set `fail-on-undeliverable`
+
+A notifier that **is** the deliverable may fail the run; a notifier reporting
+**someone else's** failure may not. Leave it `true` when sending the message is
+the whole job — nothing else records that it vanished. Set it `false` in an
+`if: failure()` step, where a red notifier is read as the failure it was
+reporting. Either way an `::error::` annotation lands on the run summary, so
+the loss is never silent.
+
+Retries honour `retry_after` and `X-RateLimit-Reset-After`, bounded at 5
+attempts / 30s per wait / 60s total, with `--max-time` so one hung connection
+cannot stall a job. A 4xx that is not a 429 is not retried — a malformed
+payload or a dead webhook does not improve by repetition.
+
+Needs `bash`, `curl` and `jq` on the runner.

discord-notify/action.yml

@@ -0,0 +1,58 @@
+name: 'Discord notify'
+description: >-
+  POST one message to a Discord webhook, honouring the rate limit and failing
+  loudly when the message never lands.
+
+inputs:
+  webhook-url:
+    description: >-
+      The Discord webhook URL. A composite action cannot declare `secrets:`, so
+      the caller passes the secret in as this input, reading it from
+      secrets.DISCORD_WEBHOOK_URL at its own call site. Masking is by value, so
+      it stays redacted crossing the boundary. This text names that context
+      without the expression wrapper on purpose: the runner evaluates input
+      descriptions too, so writing it out here fails the manifest before the
+      action ever runs.
+    required: true
+  content:
+    description: >-
+      Message text. The action JSON-encodes it, so quotes, backslashes and
+      newlines are safe without the caller reaching for jq. Give exactly one
+      of `content` or `payload`.
+    required: false
+    default: ''
+  payload:
+    description: >-
+      A complete Discord webhook JSON object, for embeds and anything
+      `content` cannot express. Give exactly one of `content` or `payload`.
+    required: false
+    default: ''
+  fail-on-undeliverable:
+    description: >-
+      Whether a message that never lands fails the step. Leave `true` when
+      sending the message IS the job — nothing else records that it vanished.
+      Set `false` in an `if: failure()` notifier, where a red step would be
+      read as the failure it was reporting. The `::error::` annotation is
+      emitted either way, so the loss is on the run summary regardless.
+    required: false
+    default: 'true'
+
+outputs:
+  delivered:
+    description: '"true" when Discord accepted the message, "false" when it never landed.'
+    value: ${{ steps.send.outputs.delivered }}
+
+runs:
+  using: composite
+  steps:
+    # Everything crosses into the script through env, never through `${{ }}`
+    # interpolated into the shell body — a PR title is attacker-controlled text
+    # and must not be able to become code.
+    - id: send
+      shell: bash
+      env:
+        DISCORD_WEBHOOK_URL: ${{ inputs.webhook-url }}
+        DISCORD_CONTENT: ${{ inputs.content }}
+        DISCORD_PAYLOAD: ${{ inputs.payload }}
+        DISCORD_FAIL_ON_UNDELIVERABLE: ${{ inputs.fail-on-undeliverable }}
+      run: '"$GITHUB_ACTION_PATH/discord-notify.sh"'

discord-notify/discord-notify.sh

@@ -0,0 +1,151 @@
+#!/usr/bin/env bash
+#
+# Post one message to a Discord webhook, honouring the rate limit.
+#
+# A plain `curl` exits 0 whatever Discord answers, so a 429 — which is what
+# several workflows firing at once earns you — threw the notification away
+# without a trace. Discord's 429 names the wait it wants:
+#
+#   {"message": "Service resource is being rate limited.", "retry_after": 3, ...}
+#
+# Wait that long and try again, bounded so no job can hang on a notification,
+# and exit non-zero when the message never lands — unless the caller set
+# fail-on-undeliverable: false, which is for a notifier reporting someone
+# else's failure, whose own red would be mistaken for that failure.
+#
+# Everything arrives via env, set by action.yml from the action's inputs.
+# Nothing is interpolated into this file, so caller-controlled text (a PR
+# title, a commit message) can never become shell.
+
+set -euo pipefail
+
+: "${DISCORD_WEBHOOK_URL:?webhook-url is required and was empty}"
+
+content=${DISCORD_CONTENT:-}
+payload=${DISCORD_PAYLOAD:-}
+fail_on_undeliverable=${DISCORD_FAIL_ON_UNDELIVERABLE:-true}
+
+if [ -n "$content" ] && [ -n "$payload" ]; then
+  echo "::error::discord-notify: give content or payload, not both"
+  exit 1
+fi
+
+# Encode here rather than making every caller reach for jq: getting a title
+# with a quote in it into valid JSON by hand is the bug this avoids.
+if [ -z "$payload" ]; then
+  if [ -z "$content" ]; then
+    echo "::error::discord-notify: one of content or payload is required"
+    exit 1
+  fi
+  payload=$(jq -n --arg content "$content" '{content: $content}')
+elif ! jq -e . >/dev/null 2>&1 <<<"$payload"; then
+  echo "::error::discord-notify: payload is not valid JSON"
+  exit 1
+fi
+
+MAX_ATTEMPTS=5
+MAX_TOTAL_WAIT=60 # across all retries — the ceiling on delaying a job
+MAX_SINGLE_WAIT=30
+DEFAULT_WAIT=2 # a 429 that names no wait at all
+
+body=$(mktemp)
+hdrs=$(mktemp)
+trap 'rm -f "$body" "$hdrs"' EXIT
+
+# Report the verdict once, in one place, so the output and the exit code
+# cannot disagree. `delivered` lets a caller that suppressed the failure still
+# branch on what happened.
+finish() {
+  local delivered=$1
+  [ -z "${GITHUB_OUTPUT:-}" ] || echo "delivered=$delivered" >>"$GITHUB_OUTPUT"
+  [ "$delivered" = false ] || exit 0
+  [ "$fail_on_undeliverable" != false ] || exit 0
+  exit 1
+}
+
+# Whole seconds, rounded UP. retry_after is fractional and the shell has no
+# float arithmetic; oversleeping costs nothing, undersleeping earns another 429.
+ceil_seconds() {
+  local value=${1:-} whole=${1%%.*}
+  case $whole in '' | *[!0-9]*) echo 0 ;; *)
+    if [ "$whole" = "$value" ]; then echo "$whole"; else echo $((whole + 1)); fi
+    ;;
+  esac
+}
+
+# A response header by name (give it lowercase), empty when absent. Bash
+# builtins only: the self-hosted runners' nix shells promise curl and jq on
+# PATH, not awk.
+header() {
+  local want=$1 line key value=""
+  while IFS= read -r line; do
+    line=${line%$'\r'}
+    key=${line%%:*}
+    if [ "${key,,}" = "$want" ]; then value=${line#*: }; fi
+  done <"$hdrs"
+  printf '%s' "$value"
+}
+
+# What the response actually asked for, in descending authority. The body is
+# Discord's own answer; the headers cover a 429 raised in front of the API,
+# which arrives with no JSON body at all.
+rate_limit_wait() {
+  local secs
+  secs=$(ceil_seconds "$(jq -r '.retry_after // empty' "$body" 2>/dev/null || true)")
+  [ "$secs" -gt 0 ] || secs=$(ceil_seconds "$(header x-ratelimit-reset-after)")
+  [ "$secs" -gt 0 ] || secs=$(ceil_seconds "$(header retry-after)")
+  [ "$secs" -gt 0 ] || secs=$DEFAULT_WAIT
+  echo "$secs"
+}
+
+waited=0
+attempt=1
+while :; do
+  # --max-time bounds a hung connection: without it "bounded retry" still
+  # leaves one attempt able to stall the job indefinitely.
+  status=$(curl -sS --connect-timeout 5 --max-time 15 \
+    -o "$body" -D "$hdrs" -w '%{http_code}' \
+    -H 'Content-Type: application/json' \
+    -X POST --data-binary "$payload" \
+    "$DISCORD_WEBHOOK_URL") || status=000
+
+  case $status in
+    2*)
+      finish true
+      ;;
+    429)
+      delay=$(rate_limit_wait)
+      scope=$(jq -r 'if .global == true then "global" else "route" end' "$body" 2>/dev/null || echo route)
+      reason="rate limited, $scope"
+      ;;
+    5* | 000)
+      # Discord is down, or the request never landed. Neither says how long to
+      # wait, so back off on the attempt count.
+      delay=$((attempt * 2))
+      reason="transient"
+      ;;
+    *)
+      # 400/401/404: a malformed payload or a dead webhook does not improve by
+      # being repeated. Fail now and name it, rather than burning the budget.
+      echo "::error::discord notification refused with HTTP $status — payload or webhook, not a rate limit"
+      detail=$(<"$body")
+      printf 'discord: HTTP %s: %s\n' "$status" "${detail:0:500}" >&2
+      finish false
+      ;;
+  esac
+
+  [ "$delay" -le "$MAX_SINGLE_WAIT" ] || delay=$MAX_SINGLE_WAIT
+
+  if [ "$attempt" -ge "$MAX_ATTEMPTS" ] || [ $((waited + delay)) -gt "$MAX_TOTAL_WAIT" ]; then
+    break
+  fi
+
+  printf 'discord: HTTP %s (%s), retrying in %ss (attempt %s/%s)\n' \
+    "$status" "$reason" "$delay" "$attempt" "$MAX_ATTEMPTS" >&2
+  sleep "$delay"
+  waited=$((waited + delay))
+  attempt=$((attempt + 1))
+done
+
+echo "::error::discord notification never landed: $attempt attempts over ${waited}s, last status $status"
+finish false