[build] Resolve Chrome milestone from Chrome-for-Testing channel version

[titusfortner]

🔗 Related Issues

💥 What does this PR do?

Resolve the Chrome milestone from Chrome-for-Testing's channel version, which always reflects the current stable/beta release.

Previously the devtools and pinned-browser updaters took the lowest of chromiumdash's latest release per platform, so a late N-1 security respin made them report the old milestone — the Update Devtools job produced no changes while Chrome 150 was already stable.

🔧 Implementation Notes

The resolver is inlined in each script rather than shared; they are independent tools with different consumers (repositories.bzl binaries vs. devtools protocol files).

Dropping the cross-platform min changes two behaviors:

• The milestone is now the latest stable one, adopted as soon as Chrome-for-Testing publishes it, rather than the lowest common across platforms — a new milestone is picked up sooner instead of waiting for every platform's rollout.
• An incomplete or missing download now fails loudly via the non-200 guards, rather than the old implicit fallback to a lower, complete milestone — a 404 body would otherwise be hashed into a wrong sha256 or written into a .pdl.

🤖 AI assistance

• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: milestone-resolution fix and fetch hardening
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

pinned_browsers will bump Chrome/ChromeDriver to the current stable milestone on its next run.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Fix Chrome milestone resolution: conservative pinning, latest devtools

🐞 Bug fix ✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Split Chrome milestone resolution by purpose: conservative pinning vs latest devtools.
• Share milestone→download lookup via a new scripts/chrome_version.py Bazel py_library.
• Fail fast on non-200 downloads to avoid silent bad pins and corrupted protocol files.

Diagram

graph TD
  A["pinned_browsers.py"] --> C["chrome_version.py"] --> F[("Pinned browser pins")]
  B["update_cdp.py"] --> C --> G[("CDP protocol files")]
  C --> D["ChromiumDash releases"] --> C
  C --> E["Chrome-for-Testing JSON"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use Chrome-for-Testing only (drop ChromiumDash for pinning)

• ➕ Fewer external dependencies/endpoints
• ➕ Simpler logic and fewer failure modes
• ➖ Cannot enforce consumer-channel lock-step across pinned platforms
• ➖ Reintroduces risk of getting ahead of a platform during staggered rollouts

2. Persist last accepted milestone/version in-repo (stateful pinning)

• ➕ Fully respin-immune and deterministic across runs
• ➕ Allows explicit promotion criteria and manual override
• ➖ Adds state management and human/process overhead
• ➖ More complex automation and review workflow

3. Query OmahaProxy-style endpoints per platform/channel

• ➕ Purpose-built for consumer-channel platform/version tracking
• ➕ Potentially more stable semantics than ChromiumDash event feeds
• ➖ Different data shape; still requires mapping to Chrome-for-Testing downloads
• ➖ May require additional parsing and platform normalization

Recommendation: Keep the PR’s split-policy approach. It cleanly separates concerns: devtools tracks the latest stable Chrome-for-Testing milestone, while pinning stays conservative and rollout-aware across platforms. The shared chrome_version module reduces duplication while still allowing different selection semantics, which is the core requirement for avoiding N-1 respin regressions without over-complicating the workflow.

Files changed (4) +82 / -63

Bug fix (2) +17 / -61

pinned_browsers.pyUse conservative milestone policy and harden downloads for pinning +7/-25

Use conservative milestone policy and harden downloads for pinning

• Replaces inline ChromiumDash/CfT milestone logic with conservative_for_channel from chrome_version. Adds explicit failure on non-200 download responses and validates chromedriver availability for the selected version.

scripts/pinned_browsers.py

update_cdp.pyTrack latest stable milestone and fail fast on protocol fetch errors +10/-36

Track latest stable milestone and fail fast on protocol fetch errors

• Switches milestone selection to latest_for_channel with a --chrome_channel argument. Adds HTTP status checks to avoid writing 404 bodies into downloaded protocol artifacts and other generated files.

scripts/update_cdp.py

Refactor (1) +52 / -0

chrome_version.pyCentralize Chrome milestone resolution and CfT version lookup +52/-0

Centralize Chrome milestone resolution and CfT version lookup

• Adds shared functions to map a milestone to the newest Chrome-for-Testing version+downloads entry. Provides two policies: latest_for_channel (tracks CfT last-known-good) and conservative_for_channel (lock-step milestone across pinned platforms using ChromiumDash).

scripts/chrome_version.py

Other (1) +13 / -2

BUILD.bazelAdd chrome_version py_library and wire scripts to it +13/-2

Add chrome_version py_library and wire scripts to it

• Introduces a new Bazel py_library target for chrome_version.py and adds it as a dependency for pinned_browsers and update_cdp. Updates rules_python load to include py_library.

scripts/BUILD.bazel

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 11 rules

1. Milestone lookup IndexError ✓ Resolved 🐞 Bug ☼ Reliability

Description

chrome_for_milestone() indexes the filtered version list with [-1] and never checks HTTP status, so
missing milestones or non-JSON upstream responses will fail with IndexError/JSONDecodeError instead
of a clear, actionable error.

Code

scripts/chrome_version.py[R18-25]

+def chrome_for_milestone(milestone):
+    """Return the newest Chrome-for-Testing version entry (version + downloads) for a milestone."""
+    r = http.request("GET", KNOWN_GOOD_WITH_DOWNLOADS)
+    versions = json.loads(r.data)["versions"]
+    return sorted(
+        filter(lambda v: v["version"].split(".")[0] == str(milestone), versions),
+        key=lambda v: parse(v["version"]),
+    )[-1]

Evidence

The new resolver performs HTTP GETs and immediately parses/selects data without checking status, and
it assumes at least one matching milestone exists by indexing the last element of a possibly-empty
list.

scripts/chrome_version.py[18-25]
scripts/chrome_version.py[28-35]
scripts/chrome_version.py[38-52]

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/chrome_version.py` fetches JSON from upstream endpoints and immediately parses/selects data without validating HTTP status codes or guarding against an empty milestone match. This can crash automation with `IndexError` (no matching milestone) or `JSONDecodeError` (e.g., HTML error body), producing low-signal failures.

### Issue Context
This module is shared by `pinned_browsers` and `update_cdp`, so an unclear failure here blocks scheduled pinning and release workflows.

### Fix Focus Areas
- scripts/chrome_version.py[18-25]
- scripts/chrome_version.py[28-35]
- scripts/chrome_version.py[38-52]

### What to change
- Check `r.status` for each `http.request(...)` and raise `ValueError` with the URL and status when non-200.
- In `chrome_for_milestone`, materialize the filtered list and raise a `ValueError` like `No Chrome-for-Testing versions found for milestone X` rather than indexing `[-1]`.
- Optionally, validate expected JSON shape (e.g., presence of `versions` / `channels[channel]`) to fail with clearer errors.

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

---

2. No tests for chrome_version 📘 Rule violation ▣ Testability

Description

Milestone resolution and failure handling behavior was added/changed, but no automated tests were
added/updated to cover latest_for_channel()/conservative_for_channel() and their HTTP/error
paths. This risks regressions that can silently break Chrome pinning and CDP updates.

Code

scripts/chrome_version.py[R18-52]

+def chrome_for_milestone(milestone):
+    """Return the newest Chrome-for-Testing version entry (version + downloads) for a milestone."""
+    r = http.request("GET", KNOWN_GOOD_WITH_DOWNLOADS)
+    versions = json.loads(r.data)["versions"]
+    return sorted(
+        filter(lambda v: v["version"].split(".")[0] == str(milestone), versions),
+        key=lambda v: parse(v["version"]),
+    )[-1]
+
+
+def latest_for_channel(channel):
+    """Newest version Chrome-for-Testing designates for a channel (e.g. "Stable", "Beta").
+
+    Adopts a new milestone as soon as its binaries ship, without waiting for consumer rollout.
+    """
+    r = http.request("GET", LAST_KNOWN_GOOD)
+    milestone = json.loads(r.data)["channels"][channel]["version"].split(".")[0]
+    return chrome_for_milestone(milestone)
+
+
+def conservative_for_channel(channel):
+    """Newest milestone that has reached the consumer channel on every pinned platform.
+
+    Per-platform we take the highest milestone seen (respin-immune: an N-1 security respin
+    can't hide that the platform also has N), then the lowest across platforms so a staggered
+    rollout steps back to N-1 until every platform has N.
+    """
+    milestones = []
+    for platform in PINNED_PLATFORMS:
+        r = http.request("GET", f"{RELEASES}?channel={channel}&num=20&platform={platform}")
+        seen = [release["milestone"] for release in json.loads(r.data) if release["milestone"]]
+        if not seen:
+            raise ValueError(f"No {channel} milestone found for platform '{platform}'")
+        milestones.append(max(seen))
+    return chrome_for_milestone(min(milestones))

Evidence

PR Compliance ID 389273 requires automated tests for new functionality and bug fixes. The PR
introduces new milestone-selection behavior in scripts/chrome_version.py and changes consumers to
use it, but no corresponding test changes are present in the change set.

Rule 389273: Require tests for all new functionality and bug fixes
scripts/chrome_version.py[18-52]
scripts/pinned_browsers.py[533-544]
scripts/update_cdp.py[16-22]

Agent prompt

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

## Issue description
New/changed Chrome milestone resolution logic is untested, despite being a bug fix that affects build/update outputs.

## Issue Context
This PR adds `scripts/chrome_version.py` and changes `pinned_browsers.py` and `update_cdp.py` to rely on it (including new non-200 failure behavior). Add automated tests that would fail if the old (stuck-on-N-1) behavior returns, and that validate error handling for missing milestones / missing downloads.

## Fix Focus Areas
- scripts/chrome_version.py[18-52]
- scripts/pinned_browsers.py[21-28]
- scripts/pinned_browsers.py[533-544]
- scripts/update_cdp.py[16-22]
- scripts/update_cdp.py[47-54]

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

---

3. --chrome_channel restricts values ✓ Resolved 📘 Rule violation ≡ Correctness

Description

The new choices=["Stable", "Beta", "Dev", "Canary"] constraint changes the script’s CLI contract
by rejecting any other previously accepted --chrome_channel values (e.g., case variants used in
automation). This can break existing callers and violates the backward-compatibility requirement for
public interfaces.

Code

scripts/update_cdp.py[R210-215]

+    parser.add_argument(
+        "--chrome_channel",
+        default="Stable",
+        choices=["Stable", "Beta", "Dev", "Canary"],
+        help="Set the Chrome channel (use Beta for early stable)",
+    )

Evidence

PR Compliance ID 389266 requires backward-compatible public interfaces. The updated argument
definition adds strict choices validation for --chrome_channel, which narrows accepted inputs
and can break existing callers.

Rule 389266: Maintain backward-compatible public API and ABI
scripts/update_cdp.py[210-215]

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` now enforces a fixed set of `--chrome_channel` values via `choices=...`, which can break existing automation that previously passed other (but functionally equivalent) values such as different casing.

## Issue Context
This script is commonly used in CI/automation; CLI flags are a de facto public interface. The new strict validation is a behavior change that can be made backward-compatible by normalizing inputs (e.g., case-insensitive mapping) or expanding accepted aliases.

## Fix Focus Areas
- scripts/update_cdp.py[210-215]

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

---

4. Unchecked milestone HTTP status ✓ Resolved 🐞 Bug ☼ Reliability

Description

latest_for_channel() fetches Chrome-for-Testing JSON and immediately parses it without checking
response status, so upstream 404/500/proxy error bodies can surface as JSONDecodeError/KeyError
instead of a clear “fetch failed” exception. This reduces debuggability and is inconsistent with the
newly-hardened fetch helpers elsewhere in the same scripts.

Code

scripts/pinned_browsers.py[R36-39]

+    r = http.request("GET", "https://googlechromelabs.github.io/chrome-for-testing/last-known-good-versions.json")
+    milestone = json.loads(r.data)["channels"][channel]["version"].split(".")[0]
+    r = http.request("GET", "https://googlechromelabs.github.io/chrome-for-testing/known-good-versions-with-downloads.json")
+    versions = json.loads(r.data)["versions"]

Evidence

Both new latest_for_channel() implementations parse JSON from last-known-good-versions.json and
known-good-versions-with-downloads.json without any status checks, while nearby helpers in the
same files explicitly raise on non-200 responses.

scripts/pinned_browsers.py[30-39]
scripts/update_cdp.py[17-26]
scripts/pinned_browsers.py[19-24]
scripts/update_cdp.py[33-36]

Agent prompt

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

### Issue description
`latest_for_channel()` performs `http.request()` and immediately `json.loads(r.data)` without verifying `r.status`. When Chrome-for-Testing endpoints return non-200 bodies (404, 500, proxy HTML, etc.), the scripts fail with parsing/KeyError exceptions that lack the URL and HTTP status.

### Issue Context
Other fetch paths in these scripts were hardened to explicitly raise on non-200, so this new path should behave similarly to keep failures actionable.

### Fix Focus Areas
- scripts/pinned_browsers.py[30-39]
- scripts/update_cdp.py[17-26]

### Implementation notes
- After each `http.request(...)`, check `resp.status != 200` and raise a `ValueError` that includes HTTP status and URL (similar to `calculate_hash()` / `fetch_and_save()`).
- Consider reading/including a short snippet of the body (optional) if it helps diagnose auth/proxy issues, but don’t log full bodies if they might be large.

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

---

5. Case-sensitive channel lookup ✓ Resolved 🐞 Bug ☼ Reliability

Description

latest_for_channel() indexes the upstream JSON using the provided channel string without
normalization/validation, so update_cdp can crash with a KeyError when users pass values like
"stable"/"beta" instead of "Stable"/"Beta".

Code

scripts/chrome_version.py[R20-21]

+    r = http.request("GET", LAST_KNOWN_GOOD)
+    milestone = json.loads(r.data)["channels"][channel]["version"].split(".")[0]

Evidence

The channel value is used as a direct key into the JSON response without any canonicalization, and
update_cdp forwards a user-controlled CLI argument into that function, making a KeyError likely for
non-canonical casing.

scripts/chrome_version.py[14-23]
scripts/update_cdp.py[185-191]

Agent prompt

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

### Issue description
`latest_for_channel(channel)` performs a case-sensitive dictionary lookup (`channels[channel]`). When `update_cdp.py` passes a user-provided `--chrome_channel` value through unchanged, common casing variants (e.g., `stable`, `BETA`) crash the script with `KeyError`.

### Issue Context
This is a CLI-exposed footgun: the error is not actionable and will look like an internal failure instead of invalid input.

### Fix Focus Areas
- scripts/chrome_version.py[14-27]

### Suggested fix approach
- Normalize channel input (e.g., map lowercase to canonical keys) or validate against the keys returned by `last-known-good-versions.json`.
- On mismatch, raise `ValueError` with a clear message listing allowed channels.

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

---

View more (1)

6. HTTP response not released ✓ Resolved 🐞 Bug ☼ Reliability

Description

calculate_hash() streams downloads with preload_content=False but never releases the urllib3
connection, which can leak pooled connections across multiple large downloads in a pinning run.

Code

scripts/pinned_browsers.py[R21-28]

    print(f"Calculate hash for {url}", file=sys.stderr)
    h = hashlib.sha256()
    r = http.request("GET", url, preload_content=False)
+    if r.status != 200:
+        raise ValueError(f"Download unavailable (HTTP {r.status}): {url}")
    for b in iter(lambda: r.read(4096), b""):
        h.update(b)
    return h.hexdigest()

Evidence

pinned_browsers streams a response without releasing it, while another script in this repo uses the
same pattern and explicitly calls release_conn(), indicating the intended cleanup behavior.

scripts/pinned_browsers.py[20-28]
scripts/update_docfx.py[53-59]

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/pinned_browsers.py:calculate_hash()` streams response bodies (`preload_content=False`) but does not call `release_conn()` (or close the response) after reading. This can retain connections in the pool longer than needed and potentially exhaust the pool during the script's many downloads.

### Issue Context
The repo already has a similar streaming download pattern that explicitly releases the connection.

### Fix Focus Areas
- scripts/pinned_browsers.py[20-28]

### What to change
- Wrap the streaming read in `try/finally` and call `r.release_conn()` in the `finally` block.
- If `r.status != 200`, release the connection before raising.

Example structure:
```py
r = http.request(..., preload_content=False)
try:
   if r.status != 200:
       raise ...
   for chunk in ...:
       ...
   return ...
finally:
   r.release_conn()
```

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

---

Previous review results

Review updated until commit 8c69597

Results up to commit e907606

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

1. No tests for chrome_version 📘 Rule violation ▣ Testability

Description

Milestone resolution and failure handling behavior was added/changed, but no automated tests were
added/updated to cover latest_for_channel()/conservative_for_channel() and their HTTP/error
paths. This risks regressions that can silently break Chrome pinning and CDP updates.

Code

scripts/chrome_version.py[R18-52]

+def chrome_for_milestone(milestone):
+    """Return the newest Chrome-for-Testing version entry (version + downloads) for a milestone."""
+    r = http.request("GET", KNOWN_GOOD_WITH_DOWNLOADS)
+    versions = json.loads(r.data)["versions"]
+    return sorted(
+        filter(lambda v: v["version"].split(".")[0] == str(milestone), versions),
+        key=lambda v: parse(v["version"]),
+    )[-1]
+
+
+def latest_for_channel(channel):
+    """Newest version Chrome-for-Testing designates for a channel (e.g. "Stable", "Beta").
+
+    Adopts a new milestone as soon as its binaries ship, without waiting for consumer rollout.
+    """
+    r = http.request("GET", LAST_KNOWN_GOOD)
+    milestone = json.loads(r.data)["channels"][channel]["version"].split(".")[0]
+    return chrome_for_milestone(milestone)
+
+
+def conservative_for_channel(channel):
+    """Newest milestone that has reached the consumer channel on every pinned platform.
+
+    Per-platform we take the highest milestone seen (respin-immune: an N-1 security respin
+    can't hide that the platform also has N), then the lowest across platforms so a staggered
+    rollout steps back to N-1 until every platform has N.
+    """
+    milestones = []
+    for platform in PINNED_PLATFORMS:
+        r = http.request("GET", f"{RELEASES}?channel={channel}&num=20&platform={platform}")
+        seen = [release["milestone"] for release in json.loads(r.data) if release["milestone"]]
+        if not seen:
+            raise ValueError(f"No {channel} milestone found for platform '{platform}'")
+        milestones.append(max(seen))
+    return chrome_for_milestone(min(milestones))

Evidence

PR Compliance ID 389273 requires automated tests for new functionality and bug fixes. The PR
introduces new milestone-selection behavior in scripts/chrome_version.py and changes consumers to
use it, but no corresponding test changes are present in the change set.

Rule 389273: Require tests for all new functionality and bug fixes
scripts/chrome_version.py[18-52]
scripts/pinned_browsers.py[533-544]
scripts/update_cdp.py[16-22]

Agent prompt

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

## Issue description
New/changed Chrome milestone resolution logic is untested, despite being a bug fix that affects build/update outputs.

## Issue Context
This PR adds `scripts/chrome_version.py` and changes `pinned_browsers.py` and `update_cdp.py` to rely on it (including new non-200 failure behavior). Add automated tests that would fail if the old (stuck-on-N-1) behavior returns, and that validate error handling for missing milestones / missing downloads.

## Fix Focus Areas
- scripts/chrome_version.py[18-52]
- scripts/pinned_browsers.py[21-28]
- scripts/pinned_browsers.py[533-544]
- scripts/update_cdp.py[16-22]
- scripts/update_cdp.py[47-54]

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

---

2. Milestone lookup IndexError ✓ Resolved 🐞 Bug ☼ Reliability

Description

chrome_for_milestone() indexes the filtered version list with [-1] and never checks HTTP status, so
missing milestones or non-JSON upstream responses will fail with IndexError/JSONDecodeError instead
of a clear, actionable error.

Code

scripts/chrome_version.py[R18-25]

+def chrome_for_milestone(milestone):
+    """Return the newest Chrome-for-Testing version entry (version + downloads) for a milestone."""
+    r = http.request("GET", KNOWN_GOOD_WITH_DOWNLOADS)
+    versions = json.loads(r.data)["versions"]
+    return sorted(
+        filter(lambda v: v["version"].split(".")[0] == str(milestone), versions),
+        key=lambda v: parse(v["version"]),
+    )[-1]

Evidence

The new resolver performs HTTP GETs and immediately parses/selects data without checking status, and
it assumes at least one matching milestone exists by indexing the last element of a possibly-empty
list.

scripts/chrome_version.py[18-25]
scripts/chrome_version.py[28-35]
scripts/chrome_version.py[38-52]

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/chrome_version.py` fetches JSON from upstream endpoints and immediately parses/selects data without validating HTTP status codes or guarding against an empty milestone match. This can crash automation with `IndexError` (no matching milestone) or `JSONDecodeError` (e.g., HTML error body), producing low-signal failures.

### Issue Context
This module is shared by `pinned_browsers` and `update_cdp`, so an unclear failure here blocks scheduled pinning and release workflows.

### Fix Focus Areas
- scripts/chrome_version.py[18-25]
- scripts/chrome_version.py[28-35]
- scripts/chrome_version.py[38-52]

### What to change
- Check `r.status` for each `http.request(...)` and raise `ValueError` with the URL and status when non-200.
- In `chrome_for_milestone`, materialize the filtered list and raise a `ValueError` like `No Chrome-for-Testing versions found for milestone X` rather than indexing `[-1]`.
- Optionally, validate expected JSON shape (e.g., presence of `versions` / `channels[channel]`) to fail with clearer errors.

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

---

3. HTTP response not released ✓ Resolved 🐞 Bug ☼ Reliability

Description

calculate_hash() streams downloads with preload_content=False but never releases the urllib3
connection, which can leak pooled connections across multiple large downloads in a pinning run.

Code

scripts/pinned_browsers.py[R21-28]

    print(f"Calculate hash for {url}", file=sys.stderr)
    h = hashlib.sha256()
    r = http.request("GET", url, preload_content=False)
+    if r.status != 200:
+        raise ValueError(f"Download unavailable (HTTP {r.status}): {url}")
    for b in iter(lambda: r.read(4096), b""):
        h.update(b)
    return h.hexdigest()

Evidence

pinned_browsers streams a response without releasing it, while another script in this repo uses the
same pattern and explicitly calls release_conn(), indicating the intended cleanup behavior.

scripts/pinned_browsers.py[20-28]
scripts/update_docfx.py[53-59]

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/pinned_browsers.py:calculate_hash()` streams response bodies (`preload_content=False`) but does not call `release_conn()` (or close the response) after reading. This can retain connections in the pool longer than needed and potentially exhaust the pool during the script's many downloads.

### Issue Context
The repo already has a similar streaming download pattern that explicitly releases the connection.

### Fix Focus Areas
- scripts/pinned_browsers.py[20-28]

### What to change
- Wrap the streaming read in `try/finally` and call `r.release_conn()` in the `finally` block.
- If `r.status != 200`, release the connection before raising.

Example structure:
```py
r = http.request(..., preload_content=False)
try:
   if r.status != 200:
       raise ...
   for chunk in ...:
       ...
   return ...
finally:
   r.release_conn()
```

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

---

Results up to commit 000e450

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

1. Case-sensitive channel lookup ✓ Resolved 🐞 Bug ☼ Reliability

Description

latest_for_channel() indexes the upstream JSON using the provided channel string without
normalization/validation, so update_cdp can crash with a KeyError when users pass values like
"stable"/"beta" instead of "Stable"/"Beta".

Code

scripts/chrome_version.py[R20-21]

+    r = http.request("GET", LAST_KNOWN_GOOD)
+    milestone = json.loads(r.data)["channels"][channel]["version"].split(".")[0]

Evidence

The channel value is used as a direct key into the JSON response without any canonicalization, and
update_cdp forwards a user-controlled CLI argument into that function, making a KeyError likely for
non-canonical casing.

scripts/chrome_version.py[14-23]
scripts/update_cdp.py[185-191]

Agent prompt

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

### Issue description
`latest_for_channel(channel)` performs a case-sensitive dictionary lookup (`channels[channel]`). When `update_cdp.py` passes a user-provided `--chrome_channel` value through unchanged, common casing variants (e.g., `stable`, `BETA`) crash the script with `KeyError`.

### Issue Context
This is a CLI-exposed footgun: the error is not actionable and will look like an internal failure instead of invalid input.

### Fix Focus Areas
- scripts/chrome_version.py[14-27]

### Suggested fix approach
- Normalize channel input (e.g., map lowercase to canonical keys) or validate against the keys returned by `last-known-good-versions.json`.
- On mismatch, raise `ValueError` with a clear message listing allowed channels.

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

---

Results up to commit 0cbabf1

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

1. Unchecked milestone HTTP status ✓ Resolved 🐞 Bug ☼ Reliability

Description

latest_for_channel() fetches Chrome-for-Testing JSON and immediately parses it without checking
response status, so upstream 404/500/proxy error bodies can surface as JSONDecodeError/KeyError
instead of a clear “fetch failed” exception. This reduces debuggability and is inconsistent with the
newly-hardened fetch helpers elsewhere in the same scripts.

Code

scripts/pinned_browsers.py[R36-39]

+    r = http.request("GET", "https://googlechromelabs.github.io/chrome-for-testing/last-known-good-versions.json")
+    milestone = json.loads(r.data)["channels"][channel]["version"].split(".")[0]
+    r = http.request("GET", "https://googlechromelabs.github.io/chrome-for-testing/known-good-versions-with-downloads.json")
+    versions = json.loads(r.data)["versions"]

Evidence

Both new latest_for_channel() implementations parse JSON from last-known-good-versions.json and
known-good-versions-with-downloads.json without any status checks, while nearby helpers in the
same files explicitly raise on non-200 responses.

scripts/pinned_browsers.py[30-39]
scripts/update_cdp.py[17-26]
scripts/pinned_browsers.py[19-24]
scripts/update_cdp.py[33-36]

Agent prompt

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

### Issue description
`latest_for_channel()` performs `http.request()` and immediately `json.loads(r.data)` without verifying `r.status`. When Chrome-for-Testing endpoints return non-200 bodies (404, 500, proxy HTML, etc.), the scripts fail with parsing/KeyError exceptions that lack the URL and HTTP status.

### Issue Context
Other fetch paths in these scripts were hardened to explicitly raise on non-200, so this new path should behave similarly to keep failures actionable.

### Fix Focus Areas
- scripts/pinned_browsers.py[30-39]
- scripts/update_cdp.py[17-26]

### Implementation notes
- After each `http.request(...)`, check `resp.status != 200` and raise a `ValueError` that includes HTTP status and URL (similar to `calculate_hash()` / `fetch_and_save()`).
- Consider reading/including a short snippet of the body (optional) if it helps diagnose auth/proxy issues, but don’t log full bodies if they might be large.

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

---

Results up to commit fab7a83

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

1. --chrome_channel restricts values ✓ Resolved 📘 Rule violation ≡ Correctness

Description

The new choices=["Stable", "Beta", "Dev", "Canary"] constraint changes the script’s CLI contract
by rejecting any other previously accepted --chrome_channel values (e.g., case variants used in
automation). This can break existing callers and violates the backward-compatibility requirement for
public interfaces.

Code

scripts/update_cdp.py[R210-215]

+    parser.add_argument(
+        "--chrome_channel",
+        default="Stable",
+        choices=["Stable", "Beta", "Dev", "Canary"],
+        help="Set the Chrome channel (use Beta for early stable)",
+    )

Evidence

PR Compliance ID 389266 requires backward-compatible public interfaces. The updated argument
definition adds strict choices validation for --chrome_channel, which narrows accepted inputs
and can break existing callers.

Rule 389266: Maintain backward-compatible public API and ABI
scripts/update_cdp.py[210-215]

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` now enforces a fixed set of `--chrome_channel` values via `choices=...`, which can break existing automation that previously passed other (but functionally equivalent) values such as different casing.

## Issue Context
This script is commonly used in CI/automation; CLI flags are a de facto public interface. The new strict validation is a behavior change that can be made backward-compatible by normalizing inputs (e.g., case-insensitive mapping) or expanding accepted aliases.

## Fix Focus Areas
- scripts/update_cdp.py[210-215]

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

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 000e450

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0cbabf1

[Copilot]

Pull request overview

Updates Selenium’s automation scripts to determine the current Chrome milestone via Chrome-for-Testing channel metadata (avoiding N-1 security respin edge cases) and improves robustness of some network fetches used by the CDP/devtools and pinned-browser update flows.

Changes:

• Switch milestone selection to Chrome-for-Testing “last known good” channel versions, then select the newest matching version-with-downloads entry.
• Add explicit non-200 handling for some download/fetch paths (e.g., .pdl fetches and SHA computation inputs).
• Add a guard for missing chromedriver downloads in the selected Chrome-for-Testing entry.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
scripts/update_cdp.py	Resolves Chrome milestone/version from Chrome-for-Testing channel metadata; adds non-200 checks for protocol fetches.
scripts/pinned_browsers.py	Resolves Chrome/ChromeDriver pinned versions from Chrome-for-Testing channel metadata; adds non-200/availability guards.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 0fa35a5

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[qodo-code-review]

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

[qodo-code-review]

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 4984c25

[qodo-code-review]

Code review by qodo was updated up to the latest commit 66081a9

[qodo-code-review]

Code review by qodo was updated up to the latest commit 50f4c76

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8c69597