Skip to content

[build] stop updating browsers and CDP during release preparation - #17940

Merged
titusfortner merged 2 commits into
trunkfrom
release-verify-cdp
Aug 24, 2026
Merged

[build] stop updating browsers and CDP during release preparation#17940
titusfortner merged 2 commits into
trunkfrom
release-verify-cdp

Conversation

@titusfortner

@titusfortner titusfortner commented Aug 22, 2026

Copy link
Copy Markdown
Member

🔗 Related Issues

Follow-on to #17872

💥 What does this PR do?

  • Removes Browser updates and CDP updates from release workflow, since they are being managed in a separate workflow, with daily PRs as necessary
  • Adds a script to check that we've updated to the latest version of CDP to ensure we don't release without it being done
  • Removes the early-stable Chrome channel option from the release workflow since the current goal is to keep stable pinned browser and CDP in sync. We haven't been using early stable, and it shouldn't matter if we can release more quickly/easily.

🔧 Implementation Notes

  • Expectation is that maintainers will verify the Browser Update PR with the CDP update has merged before starting the Release workflow, so trunk isn't locked while any issues are addressed.
  • The CDP check compares Chrome majors, so routine patch and minor bumps never hold up a release — only a promotion the daily workflow hasn't landed yet. It is also limited to full releases, so a language-scoped patch release is never blocked by an unrelated Chrome bump.
  • verify-cdp runs ahead of restrict-trunk, so a stale-CDP abort happens quickly and leaves nothing locked to unwind.
  • The check is one HTTPS GET and one directory test, so it runs as a plain job with a sparse checkout of the two paths it reads, rather than through Bazel and a full toolchain setup.

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 5)
    • What was generated: the workflow and Rakefile changes, verify_cdp.sh, and this description
    • I reviewed all AI output and can explain the change

🔄 Types of changes

  • Bug fix (backwards compatible)

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

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Gate release prep on current CDP; drop browser/CDP updates from workflow

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove pinned browser and CDP updates from pre-release; handled by separate daily workflow PRs.
• Add a fast verify-cdp gate to prevent releasing without DevTools for current Stable Chrome.
• Simplify release inputs and Rake tasks by removing the early-stable Chrome channel option.
Diagram

graph TD
  A["workflow_dispatch\npre-release"] --> B["parse-tag"] --> C["verify-cdp job"] --> D["restrict-trunk"] --> E["release prep jobs\n(versions/changelogs/manager)"]
  C --> F["Chrome for Testing\nversions.json"] --> G["DevTools dir exists\ncommon/devtools/chromium/v<major>"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Derive Chrome major from pinned browser data in-repo
  • ➕ Eliminates external network dependency during release prep
  • ➕ Guarantees the check matches whatever the repo currently pins
  • ➖ Would no longer enforce 'current Stable'—it only enforces consistency with repo state
  • ➖ Still needs an independent mechanism to ensure pins are up-to-date
2. Implement verify step in Python using existing urllib3 tooling
  • ➕ Avoids relying on runner-provided jq
  • ➕ Can share logic with scripts/update_cdp.py (single source of truth)
  • ➖ Adds interpreter/tooling expectations and slightly more overhead than a small bash check
  • ➖ Still needs careful dependency management in GitHub Actions environment

Recommendation: The PR’s approach (a lightweight bash job with sparse checkout) is the best tradeoff for release-time ergonomics: it fails fast before trunk restriction, does minimal work, and enforces the intended policy (don’t release without CDP for current Stable major). If flakiness from the external GET ever becomes an issue, consider switching to a small Python check (no jq) or adding a retry/backoff around the request.

Files changed (4) +47 / -73

Bug fix (1) +16 / -0
verify_cdp.shAdd script to block releases when CDP lags Stable Chrome major +16/-0

Add script to block releases when CDP lags Stable Chrome major

• Adds a bash script that fetches the current Stable Chrome version (major) and verifies a matching common/devtools/chromium/v<major> directory exists. Emits a GitHub Actions error and exits non-zero to stop the workflow early when DevTools are missing.

scripts/github-actions/verify_cdp.sh

Documentation (1) +1 / -2
update_cdp.pyClarify milestone docstring (remove early-stable mention) +1/-2

Clarify milestone docstring (remove early-stable mention)

• Updates the get_chrome_milestone docstring to remove references to early-stable/beta handling, matching the new Stable-only release policy.

scripts/update_cdp.py

Other (2) +30 / -71
pre-release.ymlAdd verify-cdp gate and remove browser/CDP update steps +23/-52

Add verify-cdp gate and remove browser/CDP update steps

• Introduces a new verify-cdp job that runs before restrict-trunk to ensure DevTools for the current Stable Chrome major are checked in. Removes the chrome_channel input and deletes the devtools and pinned-browsers update/commit steps from the release preparation flow. Updates job dependencies and failure notifications to reflect the new gate and simplified update set.

.github/workflows/pre-release.yml

RakefileRemove early-stable/channel plumbing from update tasks and pre_release +7/-19

Remove early-stable/channel plumbing from update tasks and pre_release

• Simplifies update_browsers and update_cdp tasks to always use Stable behavior without channel arguments. Updates the pre_release task signature and removes browser/CDP updates from the patch==0 release-prep path, aligning local tooling with the new workflow responsibilities.

Rakefile

@qodo-code-review

qodo-code-review Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. CDP gate blocks patch releases ✓ Resolved 🐞 Bug ≡ Correctness
Description
restrict-trunk now requires verify-cdp, but verify-cdp runs unconditionally for all tags, so a
language-only patch release (e.g., selenium-X.Y.Z-ruby) can be blocked when Stable Chrome advances
before CDP is checked in. This is a workflow regression because patch releases are explicitly
required to be language-scoped, yet they now inherit a global CDP freshness gate.
Code

.github/workflows/pre-release.yml[R83-86]

  restrict-trunk:
    name: Restrict Trunk Branch
-    needs: parse-tag
+    needs: [parse-tag, verify-cdp]
    uses: ./.github/workflows/restrict-trunk.yml
Evidence
The PR adds an unconditional verify-cdp job and makes restrict-trunk depend on it, meaning
*every* release run must pass the CDP check before trunk is restricted. Separately, the tag parser
explicitly allows and requires language-only patch releases, and the release workflow
builds/publishes only the selected language for non-all tags—so this new global gate can block a
valid language-only release.

.github/workflows/pre-release.yml[68-89]
.github/workflows/parse-release-tag.yml[39-73]
.github/workflows/release.yml[145-152]
.github/workflows/release.yml[213-218]

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 `verify-cdp` job gates `restrict-trunk`, but it runs for every release tag (including language-only patch releases). That means patch releases can fail early due to a Stable Chrome/CDP mismatch even though the workflow explicitly supports language-only tags.

### Issue Context
- Patch releases must include a language suffix; `parse-release-tag.yml` enforces this.
- `restrict-trunk` now depends on `verify-cdp`, so any failure (or skip) blocks the entire workflow.

### Fix Focus Areas
- .github/workflows/pre-release.yml[68-86]

### Recommended change
Keep the `verify-cdp` *job* so downstream `needs` continues to work, but make the actual verification step conditional so the job still succeeds for language-only releases:
- Add `if: needs.parse-tag.outputs.language == 'all'` on the step that runs `./scripts/github-actions/verify_cdp.sh`.
- Add a second step for the non-`all` case that logs a skip reason (optional) and exits 0.

This preserves the fast-fail behavior for full releases while avoiding blocking language-only patch releases.

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



Remediation recommended

2. Brittle CDP verification script ✓ Resolved 🐞 Bug ☼ Reliability
Description
verify_cdp.sh has no timeout/retry on its network fetch and does not validate that the parsed
Chrome major is non-empty/numeric, so transient network/schema issues can hang or fail release
preparation with misleading errors (e.g., checking for vnull/v). This makes the release workflow
less reliable than the existing browser-update script, which includes explicit parsing validation.
Code

scripts/github-actions/verify_cdp.sh[R7-12]

+versions_url="https://googlechromelabs.github.io/chrome-for-testing/last-known-good-versions.json"
+major=$(curl -fsS "$versions_url" | jq -r '.channels.Stable.version | split(".")[0]')
+
+root=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)
+if [ ! -d "${root}/common/devtools/chromium/v${major}" ]; then
+  echo "::error::No CDP for Chrome v${major}; merge the pending browser update before releasing" >&2
Evidence
The new script unconditionally curls an external URL and pipes to jq under set -euo pipefail,
with no timeout/retry and no guard that major is valid before interpolating it into the directory
check. A nearby, existing workflow script demonstrates the expected pattern of validating parsed
Chrome major and emitting a dedicated error when parsing fails.

scripts/github-actions/verify_cdp.sh[5-13]
scripts/github-actions/update_browsers.sh[28-35]

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

### Issue description
`scripts/github-actions/verify_cdp.sh` depends on an external HTTPS GET and a `jq` parse but provides no retry/timeout and does not validate the extracted `major`. In failure modes (network blip, upstream JSON shape change), this can either hang the job or fail with a confusing message (e.g., missing `v`/`vnull`).

### Issue Context
There is already a precedent for robust parsing/error messaging in `scripts/github-actions/update_browsers.sh`.

### Fix Focus Areas
- scripts/github-actions/verify_cdp.sh[5-13]
- scripts/github-actions/update_browsers.sh[28-35]

### Recommended change
1) Make the network call bounded and resilient:
- Use curl flags like `--retry 3 --retry-all-errors --connect-timeout 10 --max-time 30`.

2) Validate the parsed major before using it:
- After computing `major=...`, check `[[ "$major" =~ ^[0-9]+$ ]]` and fail with a clear `::error::Failed to parse Stable Chrome major` message if it doesn’t match.

This keeps the intended fast-fail behavior but avoids flaky hangs and misleading error output.

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


Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/pre-release.yml
Comment thread scripts/github-actions/verify_cdp.sh
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit a17e371

@titusfortner
titusfortner merged commit a4b3c3d into trunk Aug 24, 2026
28 checks passed
@titusfortner
titusfortner deleted the release-verify-cdp branch August 24, 2026 17:30
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