fix(ci): repair reusable notify-slack workflow to fix post-merge startup_failure - #11452
fix(ci): repair reusable notify-slack workflow to fix post-merge startup_failure#11452nv-tusharma wants to merge 2 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Slack notification step fails when the webhook secret is absent, despite being declared optional
The notification step runs with an empty webhook URL (webhook: ${{ secrets.SLACK_NOTIFY_NIGHTLY_WEBHOOK_URL }} at .github/workflows/notify-slack.yml:98) when the secret is missing, so the Slack action errors out instead of being silently skipped.
Impact: In contexts where the webhook secret is not configured, any run with failures will crash the notifier job, turning a graceful skip into a visible CI failure.
The if-condition on the Notify Slack step doesn't guard against an absent secret
The PR changes the secret declaration to required: false (line 37) and the comment on lines 28-35 explicitly promises to "tolerate absence at runtime in the Notify Slack step below." However, the step's if condition (line 95) only checks steps.failed-jobs.outputs.has_failures == 'true' — it does not verify the webhook secret is non-empty.
When secrets.SLACK_NOTIFY_NIGHTLY_WEBHOOK_URL is absent, it resolves to an empty string. The slackapi/slack-github-action v2.x requires a valid webhook URL and will error when given an empty one. The step needs an additional guard:
if: steps.failed-jobs.outputs.has_failures == 'true' && secrets.SLACK_NOTIFY_NIGHTLY_WEBHOOK_URL != ''(Refers to line 95)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Resolved in a6ef73f. Guarded the step so it skips (not fails) when the webhook is absent. Note the suggested secrets.SLACK_NOTIFY_NIGHTLY_WEBHOOK_URL != '' in the step if isn't valid — the secrets context isn't available in step if conditions — so I bound the secret to a job-level env var and guarded on env.SLACK_WEBHOOK_URL != ''.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughThe pull request modifies the reusable Slack notification GitHub Actions workflow, changing the SLACK_NOTIFY_NIGHTLY_WEBHOOK_URL secret declaration from required to optional, with added comments clarifying runtime handling when the secret is absent. ChangesSlack Notify Workflow Update
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
…tup_failure The Post-Merge CI pipeline began failing at startup (startup_failure, 0 jobs, ~1s) on every push after PR #11365 extracted the Slack notifier into the reusable workflow .github/workflows/notify-slack.yml. Confirmed on run 28982827972 (commit f1de717, PR #11365 itself) and run 28985152222 (commit 88ddd8a, a descendant merged after #11365) -- every post-merge run started before #11365 created jobs normally, so the reusable refactor is the cause. The reusable workflow declared its Slack webhook secret as `required: true` under `on.workflow_call.secrets`. GitHub gates the CALLER's startup on a `required: true` secret existing even when the caller passes `secrets: inherit`; when the secret is absent in a given context the whole calling run fails at startup with 0 jobs. This server-side validation is invisible to YAML parsing and to actionlint (both are clean on the committed files). The pre-#11365 inline notifier referenced the same webhook lazily, so a missing secret simply resolved to empty at runtime instead of blocking startup. Mark the secret `required: false` to remove the startup gate while still inheriting it when present -- restoring the pre-refactor behavior for both the Post-merge and Nightly callers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
13d9e92 to
0bbc615
Compare
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
|
The reusable notifier fired on failures with an empty webhook when the now-optional SLACK_NOTIFY_NIGHTLY_WEBHOOK_URL secret is missing, and the Slack action rejects an empty URL — turning a graceful skip into a job failure. Bind the secret to a job-level env var (the secrets context is unavailable in step if conditions) and guard the step on it being non-empty, matching the PR's stated intent to tolerate absence at runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Problem
The Post-Merge CI Pipeline is currently down — it fails to start (
startup_failure, 0 jobs, ~1s) on every commit that includes #11365 ("ci: extract shared Slack notifier into a reusable workflow").Confirmed by recurrence:
f1de717b) →startup_failure, 0 jobs88ddd8ae) →startup_failure, 0 jobsRoot cause
When #11365 moved the Slack notifier into the reusable
.github/workflows/notify-slack.yml, it declared the webhook secret asrequired: true:A reusable workflow that marks a secret
required: truemakes GitHub gate the caller's startup on that secret being present — even when the caller passessecrets: inherit. If the secret isn't available in the calling context, GitHub aborts the entire calling run at compile time withstartup_failureand 0 jobs. This is invisible to YAML parsing and toactionlint(both lint clean).The pre-#11365 inline notifier referenced the same webhook lazily inside a step, where a missing secret simply resolves to empty at runtime — no startup gate. That single lazy→
required: truechange is the whole regression.This also affects
nightly-ci.yml, which uses the same reusable workflow withsecrets: inheritand will hit the same startup gate on its next scheduled run.Fix
Declare the secret
required: false.secrets: inheritstill passes it through when present; when absent it resolves empty at runtime — matching the pre-#11365 inline behavior. This removes the startup gate and fixes both callers (post-merge and nightly).Validation
actionlintonnotify-slack.yml+post-merge-ci.yml: exit 0, clean.🤖 Generated with Claude Code
Summary by CodeRabbit