Conversation
There was a problem hiding this comment.
Pull request overview
Reintroduces Slack notifications for CI results by adding a reusable workflow that posts to Slack and wiring it into both the push and PR workflows.
Changes:
- Added a reusable
workflow_callworkflow to post success/failure messages to Slack. - Added a
slack-notificationjob topush.ymlandpr.ymlthat runsalways()and reports the combined outcome ofstaticcheck+test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/slack-notifications.yml |
New reusable workflow that builds a Slack message payload via jq and posts to chat.postMessage. |
.github/workflows/push.yml |
Calls the reusable Slack workflow after staticcheck and the matrix test job. |
.github/workflows/pr.yml |
Calls the reusable Slack workflow after staticcheck and test on PRs. |
| { | ||
| "type": "section", | ||
| "text": { | ||
| "type": "mrkdwn", | ||
| "text": ":large_green_circle: *All checks have passed:* *\($branch)* :white_check_mark:" | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The jq filter string contains trailing commas after the first block object (after the closing brace of the section block). jq's JSON/object syntax does not allow trailing commas, so payload construction will fail and the step may silently proceed with an empty/invalid payload.
| { | ||
| "type": "section", | ||
| "text": { | ||
| "type": "mrkdwn", | ||
| "text": ":red_circle: *Failed run:* *\($branch)*" | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Same issue in the failure payload: there's a trailing comma after the section block object. This makes the jq filter invalid and will prevent building the Slack message payload.
| run: | | ||
| if [ "${{ inputs.status }}" == "success" ]; then | ||
| payload=$(jq -n --arg repository "${{ inputs.repository }}" --arg branch "${{ inputs.branch }}" --arg actor "${{ inputs.actor }}" --arg run_id "${{ inputs.run_id }}" '{ | ||
| "channel": "team-gnark-build", | ||
| "text": "GitHub Action build result: success", | ||
| "blocks": [ | ||
| { | ||
| "type": "section", | ||
| "text": { | ||
| "type": "mrkdwn", | ||
| "text": ":large_green_circle: *All checks have passed:* *\($branch)* :white_check_mark:" | ||
| }, | ||
| }, | ||
| { | ||
| "type": "context", | ||
| "elements": [ | ||
| { | ||
| "type": "mrkdwn", | ||
| "text": "\($repository) -- \($actor) -- <https://github.com/\($repository)/actions/runs/\($run_id)|View details>" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }') | ||
| else | ||
| payload=$(jq -n --arg repository "${{ inputs.repository }}" --arg branch "${{ inputs.branch }}" --arg actor "${{ inputs.actor }}" --arg run_id "${{ inputs.run_id }}" '{ | ||
| "channel": "team-gnark-build", | ||
| "text": "GitHub Action build result: failure", | ||
| "blocks": [ | ||
| { | ||
| "type": "section", | ||
| "text": { | ||
| "type": "mrkdwn", | ||
| "text": ":red_circle: *Failed run:* *\($branch)*" | ||
| }, | ||
| }, | ||
| { | ||
| "type": "context", | ||
| "elements": [ | ||
| { | ||
| "type": "mrkdwn", | ||
| "text": "\($repository) -- \($actor) -- <https://github.com/\($repository)/actions/runs/\($run_id)|View details>" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }') | ||
| fi | ||
| response=$(curl -s -X POST -H 'Content-type: application/json; charset=utf-8' --data "$payload" https://slack.com/api/chat.postMessage -H "Authorization: Bearer ${{ secrets.SLACK_BOT_TOKEN }}" ) | ||
| shell: bash |
There was a problem hiding this comment.
This script doesn't enable set -e/-o pipefail or validate the Slack API response. As written, jq/curl failures (or Slack returning { "ok": false }) can be ignored and the workflow will still report success, making notifications flaky and hard to debug. Consider failing fast on jq/curl errors and at least checking/logging response.ok/response.error from the Slack API (or explicitly mark this job continue-on-error if notification failures shouldn't fail CI).
| secrets: | ||
| SLACK_BOT_TOKEN: | ||
| required: true |
There was a problem hiding this comment.
Declaring SLACK_BOT_TOKEN as required: true for this reusable workflow will cause pull_request workflows from forks to fail at workflow-call time (fork PRs don't get repository secrets). Either make this secret optional and no-op when it's missing, or ensure callers add an if: guard to skip notifications for forked PRs.
| slack-notification: | ||
| needs: [staticcheck, test] | ||
| if: always() | ||
| uses: ./.github/workflows/slack-notifications.yml | ||
| with: | ||
| status: ${{ (needs.staticcheck.result == 'success' && needs.test.result == 'success') && 'success' || 'failure' }} | ||
| actor: ${{ github.actor }} | ||
| repository: ${{ github.repository }} | ||
| branch: ${{ github.head_ref || github.ref_name }} | ||
| run_id: ${{ github.run_id }} | ||
| secrets: | ||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} |
There was a problem hiding this comment.
This job always runs on pull_request, but SLACK_BOT_TOKEN won't be available for PRs from forks, and the reusable workflow currently requires it. That will make forked PR checks fail. Add an if: guard to skip on forked PRs (e.g., when github.event.pull_request.head.repo.full_name != github.repository) and/or adjust the reusable workflow secret requirement to be optional with a graceful no-op.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "type": "mrkdwn", | ||
| "text": ":large_green_circle: *All checks have passed:* *\($branch)* :white_check_mark:" | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Trailing commas in jq expressions break Slack notifications
High Severity
The jq -n object construction has a trailing comma after the "text" inner object closes (before the section object closes). jq does not allow trailing commas and will fail with a syntax error. This affects both the success and failure payload branches, meaning Slack notifications will never be sent. Since GitHub Actions bash steps default to set -eo pipefail, the jq failure will cause the entire step to error out.
Note
Low Risk
CI-only changes that add outbound Slack messaging; risk is mainly misconfiguration/noise or leaking metadata to Slack, not code/runtime behavior.
Overview
Reintroduces Slack notifications for CI by adding a reusable workflow,
.github/workflows/slack-notifications.yml, that posts a success/failure message (with repo/branch/actor/run link) to theteam-gnark-buildchannel usingSLACK_BOT_TOKEN.Updates the
prandpushworkflows to run a newslack-notificationjob always afterstaticcheckandtest, computing a combined status from those jobs. Minor whitespace-only cleanup inpr.ymltest commands.Written by Cursor Bugbot for commit b19a2e6. This will update automatically on new commits. Configure here.