Alert live-canary Slack channel on merge queue failures - #7007
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-7007 environment in ironclaw-ci-preview
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe CI Slack alert workflow now handles ChangesCI alerting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant GitHubAPI
participant SlackWebhook
GitHubActions->>GitHubAPI: Retrieve failed jobs and annotations
GitHubAPI-->>GitHubActions: Return CI and merge-queue details
GitHubActions->>SlackWebhook: Send event-specific failure alert
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
scope: ci
🔎 Review · PR #7007
Submitted review →Reviewed the complete trusted base-to-head comparison. The merge-queue alert routing, permissions, payload construction, Slack escaping and bounds, existing main-branch behavior, documentation, and Code Style integration are coherent. No concrete actionable defects were found. Automatic · PR opened · attempt 1 of 3 · completed in 1m 43s Run details
|
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Review complete · PR #7007
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. The merge-queue alert routing, permissions, payload construction, Slack escaping and bounds, existing main-branch behavior, documentation, and Code Style integration are coherent. No concrete actionable defects were found.
Validation and technical details
- Inspected all four changed files and surrounding workflow code across refs/ironloop/base..refs/ironloop/head.
- Ran
bash scripts/ci/test-main-ci-slack-alerts.shsuccessfully. - Ran
bash -n scripts/ci/test-main-ci-slack-alerts.shsuccessfully. - Ran
git diff --check refs/ironloop/base..refs/ironloop/headsuccessfully. - Verified the workflow retains event/conclusion gating, separates main and merge-queue webhook destinations, grants read-only permissions, escapes and bounds externally sourced Slack content, and handles API/webhook failures.
- Base:
main - Head:
codex/merge-queue-slack-alertsate9ee94e - Run:
08108289-8cfa-43ff-aae0-5f1a360dae99
🔎 Review · PR #7007
1 actionable findings →The workflow implementation is plausibly correct and preserves least-privilege permissions, event gating, Slack escaping, and webhook separation. However, the newly added automated test does not exercise the notifier’s behavior, leaving this substantial CI side effect effectively untested. Automatic · PR opened · attempt 1 of 3 · completed in 1m 40s Run details
|
There was a problem hiding this comment.
🔍 Review complete · PR #7007
The workflow implementation is plausibly correct and preserves least-privilege permissions, event gating, Slack escaping, and webhook separation. However, the newly added automated test does not exercise the notifier’s behavior, leaving this substantial CI side effect effectively untested.
Findings
- 🟠 Medium · Notifier behavior is not covered by the contract test —
scripts/ci/test-main-ci-slack-alerts.sh:16-24
Details are attached to the relevant diff.
Validation and technical details
- Inspected the complete trusted comparison refs/ironloop/base (a50ad06) through refs/ironloop/head (e9ee94e), covering all four changed files.
git diff --check refs/ironloop/base refs/ironloop/headpassed.bash scripts/ci/test-main-ci-slack-alerts.shpassed.bash -n scripts/ci/test-main-ci-slack-alerts.shpassed.- Reviewed the full embedded notifier command and surrounding workflow permissions, triggers, event filtering, GitHub API pagination, Slack escaping and truncation, webhook selection, and error handling.
- YAML parsing could not be independently rerun because neither Ruby nor Python's PyYAML module is installed in the checkout environment.
- Base:
main - Head:
codex/merge-queue-slack-alertsate9ee94e - Run:
e5849c80-99e1-441e-a4a9-c6e87ee8d91b
| assert_contains "- gh-readonly-queue/main/**" | ||
| assert_contains "contains(fromJSON('[\"push\",\"merge_group\"]'), github.event.workflow_run.event)" | ||
| assert_contains "checks: read" | ||
| assert_contains "pull-requests: read" | ||
| assert_contains 'LIVE_CANARY_SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}' | ||
| assert_contains 'if [[ "$HEAD_BRANCH" =~ ^gh-readonly-queue/main/pr-([0-9]+)- ]]; then' | ||
| assert_contains '"repos/${GITHUB_REPOSITORY}/pulls/${pr_number}"' | ||
| assert_contains '*Failed jobs / steps:*' | ||
| assert_contains '*Failure annotations (when available):*' |
There was a problem hiding this comment.
🟠 Medium · Notifier behavior is not covered by the contract test
The test only searches the workflow text for several independent substrings. It never executes the embedded shell with mocked gh and curl, nor asserts the generated payload or selected webhook. Consequently, regressions such as reversing the push/merge-queue webhook routing, breaking the PR-ref parser, producing invalid Slack JSON, or introducing a shell runtime error can all pass while these strings remain somewhere in the file. Extract the notifier into a testable script or build a harness that runs the workflow command with representative push and merge-group fixtures and verifies API calls, payload escaping/bounds, routing, and failure behavior.
This branch was successfully deployed
2 participants
Summary
merge_groupworkflow runs in the existing external CI alert workflowChange Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildbash scripts/ci/test-main-ci-slack-alerts.shcargo test --features integrationif database-backed or integration behavior changed30644625064with Slack delivery mockedreview-prorpr-shepherd --fixwas run before requesting reviewTest Strategy
User behavior:
A failed merge-queue workflow posts an alarm to the existing live-canary Slack channel. The alarm identifies the queued PR and reports failed jobs/steps plus check annotations when GitHub publishes them.
Risk areas:
Tests added or updated:
scripts/ci/test-main-ci-slack-alerts.shlocks the queue branch filter, event gate, least-privilege permissions, live-canary webhook routing, PR lookup, failure detail fields, and its own Code Style execution path.What the tests prove:
The alert workflow observes
merge_groupruns, resolves the PR from the repository's queue ref, routes queue failures throughSLACK_WEBHOOK_URL, includes structured failure metadata, and remains covered whenever the workflow changes.Commands run:
bash scripts/ci/test-main-ci-slack-alerts.shbash -n scripts/ci/test-main-ci-slack-alerts.sh.github/workflows/main-ci-slack-alerts.ymland.github/workflows/code_style.ymlgit diff origin/main...HEAD --check30644625064withcurlmocked; no Slack message sentSecurity Impact
The alert job gains
checks: readandpull-requests: readso it can fetch failure annotations and PR metadata. It continues to use repository-scopedGITHUB_TOKENaccess and the existingSLACK_WEBHOOK_URLsecret. PR titles, job names, and annotation text are Slack-escaped and bounded before posting; arbitrary workflow logs are not forwarded.Reborn Trust-Boundary Checklist
N/A: this changes GitHub Actions alerting only and does not modify Reborn runtime, policy, evidence, persistence, or trust-bearing types.
Database Impact
None.
Blast Radius
Limited to the external CI Slack alert workflow and the Code Style self-test list. A notifier regression could omit or duplicate an alert, but cannot affect the required workflow run that it observes.
Rollback Plan
Revert this commit to restore push-only main CI alerts. The existing live-canary and nightly Slack reporting paths remain independent.
Review Follow-Through
Please verify that routing merge-queue failures to the shared live-canary channel matches the desired notification volume. The PR is draft pending normal CI and review.
Review track: C (CI)