Skip to content

fix(update_cdp): update release fetch URL to include milestone 146#17318

Merged
diemol merged 1 commit intotrunkfrom
force_cdp_146
Apr 8, 2026
Merged

fix(update_cdp): update release fetch URL to include milestone 146#17318
diemol merged 1 commit intotrunkfrom
force_cdp_146

Conversation

@diemol
Copy link
Copy Markdown
Member

@diemol diemol commented Apr 8, 2026

Focing CDP 146 to do a new Selenium release. I will rollback this to release the next version with 147.

Copilot AI review requested due to automatic review settings April 8, 2026 16:35
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Pin Chrome milestone 146 in CDP update script

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Updates CDP fetch URL to pin Chrome milestone 146
• Adds milestone parameter to chromiumdash API request
• Temporary change for forcing specific CDP version

Grey Divider

File Changes

1. scripts/update_cdp.py 🐞 Bug fix +1/-1

Add milestone 146 parameter to chromiumdash API request

• Modified fetch_releases API call to include milestone=146 parameter
• Removes num=1 parameter and replaces with explicit milestone constraint
• Forces Chrome milestone 146 selection for CDP updates

scripts/update_cdp.py


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Apr 8, 2026

Code Review by Qodo

🐞 Bugs (2)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ≡ Correctness (2)

Grey Divider


Action required

1. Hardcoded milestone pin 🐞
Description
get_chrome_milestone() now always queries ChromiumDash with milestone=146, so running
scripts/update_cdp.py will keep generating CDP updates for v146 even after the selected Chrome
channel moves on, resulting in stale/incorrect version directories being added/removed. This breaks
the script’s intended behavior of deriving the milestone from the channel at runtime.
Code

scripts/update_cdp.py[30]

+        f"https://chromiumdash.appspot.com/fetch_releases?channel={channel}&milestone=146&platform=Mac,Linux",
Evidence
The milestone is now forced via a hardcoded query parameter, but the rest of the script uses the
derived milestone to decide which version directories to add/remove and to update multiple language
bindings. In contrast, the repository’s equivalent milestone-derivation logic in
scripts/pinned_browsers.py remains dynamic (no hardcoded milestone).

scripts/update_cdp.py[17-47]
scripts/update_cdp.py[56-66]
scripts/pinned_browsers.py[28-49]

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/update_cdp.py:get_chrome_milestone()` is now pinned to milestone 146 via a hardcoded URL query parameter. This causes the CDP update workflow to keep targeting v146 even after Chrome advances, producing incorrect updates.

### Issue Context
The script already parses CLI args (`--chrome_channel`) inside `get_chrome_milestone()`. This is a natural place to add an optional milestone override while keeping the default behavior dynamic.

### Fix Focus Areas
- scripts/update_cdp.py[17-47]

### Suggested change
- Add a CLI arg like `--chrome_milestone` (default: unset).
- If `--chrome_milestone` is provided, use that value for `milestone` (and optionally skip the ChromiumDash call entirely).
- If not provided, keep the previous dynamic ChromiumDash query (with `num=1`) to derive the milestone from the chosen channel.

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



Remediation recommended

2. Removed release result limit 🐞
Description
The ChromiumDash request no longer includes num=1, but the code still computes `milestone =
min(milestones)`; if the response includes multiple releases/milestones, the script will select the
oldest milestone present and update CDP to the wrong major version. This is especially risky because
the same algorithm is only safe when the response is constrained to a single release.
Code

scripts/update_cdp.py[30]

+        f"https://chromiumdash.appspot.com/fetch_releases?channel={channel}&milestone=146&platform=Mac,Linux",
Evidence
Both update_cdp.py and pinned_browsers.py use milestone = min(milestones). However,
pinned_browsers.py keeps num=1 in the ChromiumDash URL to constrain the result set, while the PR
removes that constraint in update_cdp.py, increasing the chance that min() selects an unintended
(older) milestone if multiple milestones are returned.

scripts/update_cdp.py[28-37]
scripts/pinned_browsers.py[28-38]

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 ChromiumDash fetch URL in `scripts/update_cdp.py` dropped `num=1`, but the milestone selection still uses `min(milestones)`. If multiple milestones appear in the response, `min()` will choose the oldest and the script will update CDP for the wrong major version.

### Issue Context
The repo already has the same milestone-selection pattern in `scripts/pinned_browsers.py`, where the request is constrained by `num=1`.

### Fix Focus Areas
- scripts/update_cdp.py[28-37]

### Suggested change
- Add `&num=1` back to the ChromiumDash URL (even if you keep `milestone=146`) to constrain the response and keep the `min(milestones)` logic safe.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Apr 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR pins the Chromium Dash release lookup used by scripts/update_cdp.py to milestone 146 in order to force generating CDP 146 artifacts for a Selenium release.

Changes:

  • Update the fetch_releases URL in get_chrome_milestone() to include milestone=146 (instead of using num=1).

@diemol diemol merged commit 5d67147 into trunk Apr 8, 2026
31 checks passed
@diemol diemol deleted the force_cdp_146 branch April 8, 2026 16:45
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.

3 participants