Skip to content

Migrate slack notifications to composite action#89

Merged
kernelsam merged 3 commits intomainfrom
skern-slack-composite-action
Apr 10, 2026
Merged

Migrate slack notifications to composite action#89
kernelsam merged 3 commits intomainfrom
skern-slack-composite-action

Conversation

@kernelsam
Copy link
Copy Markdown
Contributor

Summary

  • Replace standalone slack-notification jobs with inline composite action step from senzing-factory/build-resources/slack-failure-notification@v4
  • Pass SLACK_BOT_TOKEN and SLACK_CHANNEL secrets to reusable workflows (add-labels-to-issue, add-to-project, add-to-project-dependabot) that now handle notifications internally
  • Add concurrency groups where missing (skip tag-only and project management workflows)
  • Remove unused outputs: status from jobs that only existed for the old notification pattern
  • Use sdk-versions composite action instead of hardcoded version lists (code-snippets-v4 only)

Test plan

  • Verify add-labels and add-to-project workflows still function on issue creation
  • Verify build/test workflows notify on failure
  • Verify concurrency cancels in-progress runs on new pushes

- Replace standalone slack-notification jobs with inline composite action step
- Pass SLACK_BOT_TOKEN and SLACK_CHANNEL to reusable workflows that now handle notifications internally
- Add concurrency groups to workflows missing them (skip tag-only and project management workflows)
- Remove unused job outputs (status) that were only needed for the old notification pattern
- Use sdk-versions composite action instead of hardcoded version lists (code-snippets-v4)
@kernelsam kernelsam requested a review from a team as a code owner April 10, 2026 19:35
@kernelsam kernelsam self-assigned this Apr 10, 2026
@kernelsam kernelsam enabled auto-merge (squash) April 10, 2026 19:35
@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review


PR Code Review

PR Summary: Migrates Slack failure notifications from standalone downstream jobs to a composite action step (for pylint.yaml) or delegates them into upstream reusable workflows (for the other three workflows). Also adds a secrets-outside-env zizmor suppression for pylint.yaml.


Code Quality

Style Guide
Style guide was inaccessible from this environment, but the YAML follows consistent formatting and naming conventions aligned with existing files.

No commented-out code
No commented-out code present.

Meaningful variable names
All identifiers are clear and descriptive.

DRY principle
This PR actively improves DRY: removes four repeated slack-notification job blocks by either inlining the notification as a composite action step or delegating to the upstream workflow.

Defects / Logic errors ⚠️

One behavioral difference in pylint.yaml to be aware of:

  • Original: The slack-notification was a separate job that ran always() and filtered by failure or cancelled. As a job-level construct, it would fire even if a matrix shard failed.

  • New (line 48–54): The step is added once at the end of the job template. With a matrix strategy across 4 Python versions, each matrix instance will independently attempt to post to Slack on failure. This could result in duplicate Slack notifications (up to 4) for a single failure event.

    If this is intentional (each matrix failure is independently reported), it's fine — but it should be confirmed. The original single downstream job would have fired once regardless of how many matrix shards failed.

CLAUDE.md
No issues. The project CLAUDE.md is Python-focused and general enough for any developer.


Testing

Unit/integration tests ✅ N/A
No application logic changed — CI/CD workflow files only.


Documentation

CHANGELOG.md
CHANGELOG.md exists and is actively maintained in this repo. The [Unreleased] section contains placeholder entries (Thing 4, Thing 5) but does not mention this CI change. For a workflow-visible behavioral change (Slack notification migration), a changelog entry under [Unreleased] is expected.

README / API docs ✅ N/A — no user-facing changes.

Inline comments for complex logic ✅ No complex logic requiring comments.

Markdown formatting ✅ YAML files, not Markdown.


Security

No hardcoded credentials

zizmor.yaml suppression ⚠️ Acceptable but worth noting:
The secrets-outside-env suppression at .github/linters/zizmor.yaml:6–8 is added specifically to allow pylint.yaml to pass secrets as with: inputs to the composite action rather than through env:. This is a legitimate pattern for composite actions (they receive inputs, not env vars), so the suppression is justified. However, the comment explaining why this suppression is needed is absent — a brief inline comment in zizmor.yaml would help future maintainers understand the intent rather than assuming it's a blanket bypass.

No sensitive data in logs

No license files (.lic / AQAAAD)

Upstream workflow trust ⚠️ Low-risk note:
For add-labels-standardized.yaml, add-to-project-senzing-dependabot.yaml, and add-to-project-senzing.yaml, the SLACK_BOT_TOKEN and SLACK_CHANNEL secrets are now forwarded to the upstream build-resources reusable workflows. This implicitly trusts that those upstream workflows have been updated to accept and use these secrets for failure notifications. If the upstream workflows are pinned at @v4 but the secret-handling was added in a later version, these secrets would be silently ignored and notifications would be lost. Confirm that the @v4 tag of each upstream workflow accepts these secrets.


Summary

Area Status
Code style
No commented code
DRY / naming
Logic / defects ⚠️ Potential duplicate Slack notifications in matrix builds
Tests ✅ N/A
CHANGELOG.md ❌ Not updated
Security ⚠️ Zizmor suppression lacks explanatory comment; upstream @v4 secret support should be verified

Blocking issues: None that are strictly blocking, but the CHANGELOG omission and the matrix/duplicate-notification concern should be addressed before merge.

Automated code review analyzing defects and coding standards

@github-actions
Copy link
Copy Markdown

Super-linter summary

Language Validation result
CHECKOV Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

@kernelsam kernelsam merged commit 3371c51 into main Apr 10, 2026
25 checks passed
@kernelsam kernelsam deleted the skern-slack-composite-action branch April 10, 2026 20:33
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