Skip to content

feat: AI-generated deploy Slack notifications - #101

Open
olerichter00 wants to merge 1 commit into
mainfrom
olerichter00/deploy-slack-notifications
Open

feat: AI-generated deploy Slack notifications#101
olerichter00 wants to merge 1 commit into
mainfrom
olerichter00/deploy-slack-notifications

Conversation

@olerichter00

@olerichter00 olerichter00 commented Jul 16, 2026

Copy link
Copy Markdown

Resolves “New changes just went live! 🥳” Deploy Slack notifications

Description

Adds deploy Slack notifications: when a Deploy PR (merged into the release branch) ships in a subscribed repo, a GitHub Action collects the shipped PRs, asks Claude to write a plain-language digest, and posts it to the subscribed Slack channels — new features in the top-level message, fixes and maintenance in a thread reply. Channels subscribe to repos via a central deploy-notifications.yml config in duchamp.

Example Slack Notification

Bildschirmfoto 2026-07-16 um 13 22 16

Changes

  • deploy-notifications.yml — central subscription config (repo → Slack channels + optional context hints for Claude), seeded with artsy/volt#product-amber
  • .github/workflows/deploy-notification.yml — reusable workflow, runs only for merged PRs into the release branch (default release)
  • scripts/deploy-notification.ts — collects shipped PRs via the GitHub API, generates the digest via the Claude API, posts via Slack chat.postMessage (no new dependencies)
  • templates/run-deploy-notification.yml — copy-paste caller workflow for consumer repos
  • scripts/test-deploy-notification.ts — local dry-run that prints the Claude prompt for a real deploy PR, no API keys needed (uses gh auth)
  • docs/deploy-notifications.md — setup docs incl. remaining org setup (Slack app with chat:write, SLACK_BOT_TOKEN org secret)
  • Unit tests for PR-number extraction, response parsing, and prompt building

Test Plan

  • yarn jest scripts/deploy-notification.test.ts — 9 tests pass
  • npx ts-node scripts/test-deploy-notification.ts artsy/volt 11796 — verified against a real Volt deploy PR: shipped PRs detected, prompt rendered with repo context

Assisted-by: Claude:Fable-5 [claude-code]

🤖 Generated with Claude Code

Adds a reusable workflow that posts a Claude-written digest to
subscribed Slack channels when a Deploy PR (merged into the release
branch) ships. Channels subscribe to repos via a central
deploy-notifications.yml config in duchamp.

Assisted-by: Claude:Fable-5 [claude-code]
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines +298 to +300
for (const channel of subscription.channels) {
await postToSlack(channel, digest, slackToken)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 One failing channel aborts the rest. postToSlack throws when data.ok is false (line 265), so if the first channel fails, this loop stops and no later channel gets the digest — and the whole job fails. The deploy-notifications.yml docs explicitly warn the bot must be invited to each channel (/invite @<bot-name>), so a not-invited channel is a realistic failure that would silently block notifications to correctly-configured channels.

Consider catching per channel so one bad subscription doesn't take down the others:

Suggested change
for (const channel of subscription.channels) {
await postToSlack(channel, digest, slackToken)
}
let anyFailed = false
for (const channel of subscription.channels) {
try {
await postToSlack(channel, digest, slackToken)
} catch (error) {
anyFailed = true
console.error(`Failed to post to ${channel}: ${error}`)
}
}
if (anyFailed) {
process.exitCode = 1
}

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code Review

Summary

Adds an AI-generated deploy Slack notification system: a reusable GitHub Actions workflow (deploy-notification.yml) that fires when a Deploy PR merges into a repo's release branch, a TypeScript script that collects the shipped PRs, asks Claude for a two-part Slack digest (features up top, fixes/maintenance in a thread), and posts it via chat.postMessage. Channel subscriptions live centrally in deploy-notifications.yml, with docs, a template consumer workflow, unit tests, and a local dry-run script.

Nicely scoped PR — it reuses the established .tooling checkout + yarn install + npx ts-node pattern from claude-review.yml, the pure helpers (extractPrNumbers, parseDigest, buildPrompt) are well factored and unit-tested, and the docs are thorough. A few robustness/edge-case notes below.

Issues Found

🟡 Important

  • One failing Slack channel aborts the restscripts/deploy-notification.ts:298-300. The posting loop await postToSlack(...) with no try/catch, and postToSlack throws when Slack returns ok: false (line 265). If the first channel fails (e.g. bot not invited — a case the docs explicitly call out as a required manual step), the remaining subscribed channels never receive the digest and the job fails. Inline suggestion left to catch per-channel.

🟢 Suggestions

  • Partial post on thread failurescripts/deploy-notification.ts:248-250. If the top-level message posts but the thread reply then fails, the top message is already live and the job errors out, leaving a message that says "check the thread 🧵" with no thread. Low likelihood; not worth blocking, just flagging.
  • Prompt injection from PR bodiesbuildPrompt interpolates each shipped PR's title/body (truncated to 1500 chars) directly into the Claude prompt. For internal Artsy repos this is low risk, but a crafted PR body could nudge the digest content. Acceptable given the trust boundary; noting for awareness.

Areas Reviewed

  • Architecture & Design: Consistent with existing reusable-workflow conventions; centralized subscription config resolved against duchamp main at runtime is a nice touch (subscription changes don't require consumer redeploys, as documented).
  • Security: Secrets passed through workflow inputs correctly; permissions scoped to contents: read / pull-requests: read. Job if guards to merged PRs into the release branch, so only trusted deploy merges trigger it. No secret leakage observed.
  • Performance: Commit pagination capped at 3 pages (300 commits, above GitHub's 250-commit PR-commits API cap anyway) and shipped PRs capped at 50 with a warning — both reasonable and documented.
  • Bugs & Edge Cases: extractPrNumbers correctly handles squash (#n) vs merge commits and ignores mid-subject/body references; parseDigest tolerates code fences and validates top_message. Good coverage.
  • Testing: Pure helpers are unit-tested; network functions (fetchShippedPrs, askClaude, postToSlack) are untested but exercised via the test-deploy-notification.ts dry-run. Reasonable trade-off.

Questions for Author

  • For pull_request events, GitHub uses the workflow file from the PR's base branch. Since Deploy PRs target release, does the consumer's .github/workflows/deploy-notification.yml need to be present on the release branch (not just main) for the closed trigger to fire? Might be worth a one-line note in docs/deploy-notifications.md if so.

@github-actions

Copy link
Copy Markdown

Detected new or modified files in .github/workflows/. Please ensure you update the relevant documentation alongside these workflow changes.

@chr-tatu chr-tatu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a non-blocking questions. Let's test it out and we can polish it later!

type: string
model:
description: "Claude model used to write the digest"
default: "claude-opus-4-8"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need opus for this? Or would sonnet be able to do the work, too?

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.

2 participants