Skip to content

[build] alert Slack when a CDP update lands on trunk - #17950

Merged
titusfortner merged 2 commits into
trunkfrom
slack-cdp-release-alert
Aug 25, 2026
Merged

[build] alert Slack when a CDP update lands on trunk#17950
titusfortner merged 2 commits into
trunkfrom
slack-cdp-release-alert

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

Builds on #17872, which moved CDP regeneration into the daily browser update

💥 What does this PR do?

  • Posts a Slack message to selenium-tlc when a daily browser update that regenerated CDP merges to trunk, saying the next Selenium release is ready to go out and asking whether anything still needs to land before it does.

🔧 Implementation Notes

  • Matches on contains rather than the full title, because (major) and with CDP are independent suffixes — [build] Automated Browser Version Update with CDP #17873 regenerated CDP without a major browser bump, and an exact match would have missed it.
  • Notifies directly instead of reusing get-approval.yml, which skips its notification when github.actor is a TLC member and so would stay silent on exactly these merges.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 5)
    • What was generated: the workflow and this description
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • New feature (CI automation, nothing user-facing)

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Alert Selenium TLC when merged browser updates regenerate CDP

✨ Enhancement ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Detects merged daily browser updates that regenerated Chrome DevTools Protocol bindings.
• Alerts selenium-tlc that the next release is ready and solicits pending changes.
Diagram

graph TD
  A["Closed pull request"] --> B{"Merged?"} --> C{"Browser CDP update?"} --> D["Slack action"] --> E["selenium-tlc"]
  B -- "No" --> F["No alert"]
  C -- "No" --> F
Loading
High-Level Assessment

A dedicated event-driven workflow is the best fit because the existing approval workflow intentionally suppresses notifications for TLC actors, which conflicts with this alert's purpose. Filtering by branch, merge status, changed path, and a title substring keeps the notification narrowly targeted while supporting independent title suffixes.

Files changed (1) +31 / -0

Enhancement (1) +31 / -0
release-ready.ymlNotify TLC when CDP browser updates merge +31/-0

Notify TLC when CDP browser updates merge

• Adds a closed-pull-request workflow scoped to DevTools changes. It sends a release-readiness Slack alert only when a merged automated browser-update PR has the 'with CDP' title marker.

.github/workflows/release-ready.yml

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Aug 25, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Release alert lacks regression tests 📘 Rule violation ☼ Reliability
Description
The new Slack notification behavior is introduced without tests covering when it should and should
not run. This leaves merge status, source branch, title matching, and the intended trunk-only
behavior unprotected against regressions.
Code

.github/workflows/release-ready.yml[R14-18]

+    if: >
+      github.event.repository.fork == false &&
+      github.event.pull_request.merged == true &&
+      github.event.pull_request.head.ref == 'pinned-browser-updates' &&
+      contains(github.event.pull_request.title, 'with CDP')
Evidence
PR Compliance ID 5 requires behavioral changes to have meaningful regression coverage. The cited
workflow lines define entirely new notification behavior, while the PR adds only the workflow and no
test exercising its event predicate or Slack-step selection.

AGENTS.md: Add Reliable Tests for Behavioral Changes
.github/workflows/release-ready.yml[14-18]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new release-ready notification condition has no meaningful regression coverage.

## Issue Context
Add fast tests that exercise positive and negative pull-request event cases, including merged versus closed, the `pinned-browser-updates` source branch, the `with CDP` title fragment, fork handling, and a `trunk` base branch. The intended behavior is specifically to alert only when the qualifying update merges to trunk.

## Fix Focus Areas
- .github/workflows/release-ready.yml[14-18]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This is a behavioral CI workflow that triggers Slack notifications based on merge, path, branch, and title conditions; it is localized but has meaningful automation and secret-handling implications, warranting a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/release-ready.yml
@titusfortner
titusfortner merged commit 498c0e7 into trunk Aug 25, 2026
30 checks passed
@titusfortner
titusfortner deleted the slack-cdp-release-alert branch August 25, 2026 19:48
This was referenced Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants