Skip to content

fix(ci): use jq for Slack notification payload#15

Merged
justin-layerv merged 1 commit into
mainfrom
fix/slack-notification-json
Apr 10, 2026
Merged

fix(ci): use jq for Slack notification payload#15
justin-layerv merged 1 commit into
mainfrom
fix/slack-notification-json

Conversation

@justin-layerv
Copy link
Copy Markdown
Contributor

Summary

Slack notification returned HTTP 400 on run #24266878474 because the JSON payload was built via shell string concatenation with escaped quotes. The commit message chore(simplify): simplify recent changes in qurl-python (#11) contained characters that broke the hand-crafted JSON.

The || echo "Slack notification failed (non-critical)" swallowed the error, so the step showed as success despite the 400.

Fix

Replace shell string concatenation with jq -n --arg which handles all JSON escaping automatically. The payload is now built as a single jq expression — no manual quote escaping, no string concatenation, no special character issues.

Test plan

  • Payload structure validated with jq locally
  • Merge and verify next push-to-main triggers a successful Slack notification

🤖 Generated with Claude Code

…ial chars

The Slack notification returned HTTP 400 because the JSON payload was
built via shell string concatenation with escaped quotes. Commit messages
containing markdown, backticks, or parentheses produced malformed JSON.

Replace with jq --arg which handles all JSON escaping automatically.
Tested against the exact commit message that triggered the 400:
"chore(simplify): simplify recent changes in qurl-python (#11)"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@justin-layerv justin-layerv self-assigned this Apr 10, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 10, 2026

PR Review — fix(ci): use jq for Slack notification payload

Verdict: Approve — this is a clean, well-motivated fix. A couple of minor observations below.

What's good

  1. Root cause is correctly identified and fixed. Shell string concatenation with escaped quotes is fragile by nature — any commit message containing ", `, \, or $ can break the JSON. jq -n --arg is the right tool for this job since it handles all JSON escaping automatically.

  2. Net reduction in complexity. −60/+37 lines, and the resulting code is significantly more readable. The payload structure is visible at a glance as a single jq expression rather than scattered across multiple string concatenation blocks.

  3. Security posture is preserved. The existing defense of passing all github.* context through env: (rather than inline ${{ }} interpolation in the run: block) is correctly kept, which prevents script injection via crafted branch/actor names.

  4. No new dependencies. jq is pre-installed on all ubuntu-latest GitHub Actions runners, so this adds no setup overhead.

Minor observations

  1. Commit section is now unconditional. The old code guarded the commit block with if [[ -n "$COMMIT_MESSAGE" ]]. The new code always includes it. This is fine for push events (which always have a commit message), and this job already gates on github.event_name != 'pull_request'. Not a problem — just noting the behavior change.

  2. Backticks in commit messages. The old code ran tr '' "'"to replace backticks with single quotes before embedding in the Slack mrkdwn. That's no longer done.jq` handles the JSON escaping, but a commit message containing literal backticks could still produce odd Slack mrkdwn rendering (unmatched inline code spans). This is very low risk — most commit messages don't contain backticks, and the worst case is slightly garbled formatting, not a failure. Worth knowing but not worth adding back.

  3. Style nit (non-blocking): echo "$COMMIT_MESSAGE" | head -n1 spawns a subshell + pipe. A here-string head -n1 <<< "$COMMIT_MESSAGE" would be marginally cleaner, but this is purely cosmetic in a CI context.

Summary

The fix is correct, the approach is idiomatic, and the PR description does a great job explaining the problem and linking to the failing run. Ship it.


Reviewed by Claude

@justin-layerv justin-layerv merged commit fa97b05 into main Apr 10, 2026
5 checks passed
@justin-layerv justin-layerv deleted the fix/slack-notification-json branch April 10, 2026 22:28
justin-layerv added a commit to layervai/qurl-typescript that referenced this pull request Apr 10, 2026
## Summary

Adds Slack build notifications to the CI workflow, matching the pattern
established in qurl-python.

- Notifies on push-to-main only (not PRs)
- Uses `jq` for JSON payload construction (avoids shell escaping bugs —
see layervai/qurl-python#15)
- Reports test status, branch, trigger, commit message
- Skips gracefully when `SLACK_WEBHOOK_URL` secret is not configured
- `continue-on-error: true` + `timeout-minutes: 5` — notification
failure never blocks CI

## Prerequisites

The `SLACK_WEBHOOK_URL` secret must be configured in the repo settings.
Until then, the job logs "not configured, skipping" and exits cleanly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant