Skip to content

[build] pin Firefox BiDi vendor CDDL from the Firefox repository - #18094

Merged
titusfortner merged 5 commits into
SeleniumHQ:trunkfrom
titusfortner:firefox-vendor-cddl
Oct 1, 2026
Merged

titusfortner merged 5 commits into
SeleniumHQ:trunkfrom
titusfortner:firefox-vendor-cddl

Conversation

@titusfortner

@titusfortner titusfortner commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

Builds on #18071 (which copied the Firefox grammars into common/bidi)
Firefox now publishes them in Bug 2057588
Unblocks #18108 (the automated repin renames the bluetooth repo, which py/BUILD.bazel still named)

💥 What does this PR do?

  • The Firefox moz: CDDL grammars are now pinned from mozilla-firefox/firefox by scripts/update_cddl.py instead of being copied into common/bidi
  • The scheduled CDDL update now repins them along with the webref grammars

🔧 Implementation Notes

  • No change to common/bidi/schema.json; the upstream files are byte-identical to the deleted copies
  • Every .cddl file in remote/webdriver-bidi/cddl is merged, so a new Firefox module does not need a script change
  • The grammar comes from the most stable branch that has the directory (release, then beta, then main); today that is main, because the directory first appears in Firefox 159
  • The pin is the last commit to change the directory instead of the branch tip, so it only moves when the grammar does
  • Running the script also advanced the webref pin, which renames bluetooth-scanning to bluetooth; py/BUILD.bazel is updated to match

🤖 AI assistance

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

💡 Additional Considerations

  • Once the grammar reaches release, the pinned schema describes stable Firefox, so moz: features that are only in beta are missing from it until they ship
  • Options for testing stable and beta against the right grammar:
    • Record the Firefox version of each definition (from browser/config/version.txt at the commit that added it) as a since field in the schema's vendor section, and guard tests on the browser version
    • Pin the grammar from beta instead, and guard the stable tests for features it does not have yet
    • Pin one grammar per channel and select the schema by the browser under test; this tests generated code that differs from what ships

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Sep 29, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Pin Firefox BiDi vendor CDDL from the Firefox repository

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Replace local Firefox BiDi grammar copies with SHA-verified files pinned from Firefox.
Diagram

graph TD
  W["Scheduled update"] --> U["CDDL updater"] --> F["Firefox CDDL"] --> E["Bazel extension"] --> S["BiDi schema"]
  U --> R["Webref CDDL"] --> E
  U --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep local vendor grammar copies
  • ➕ Avoids additional Bazel downloads.
  • ➖ Requires manual synchronization with Firefox and retains duplicated source files.
2. Pin Firefox beta instead of preferring release
  • ➕ Exposes pre-release moz: definitions sooner for beta testing.
  • ➖ Can describe features unavailable in stable Firefox.

Recommendation: Use the PR's upstream, SHA-verified pins to eliminate duplicated grammars and make updates reproducible. Release-first selection suits a shipping-browser schema, though beta-only features will need a separate testing strategy once the grammar reaches release.

Files changed (5) +140 / -45

Enhancement (1) +102 / -29
update_cddl.pyAutomatically discover and pin Firefox vendor CDDL +102/-29

Automatically discover and pin Firefox vendor CDDL

• Selects the first Firefox branch with the CDDL directory, preferring release over beta and main, then pins its latest directory-changing commit. Discovers every CDDL file there and regenerates hashed download entries, vendor labels, and Bazel repository imports.

scripts/update_cddl.py

Other (4) +38 / -16
update-cddl.ymlDocument Firefox grammars in automated update PRs +2/-2

Document Firefox grammars in automated update PRs

• The scheduled workflow's PR body now identifies Firefox vendor CDDL as an updated source.

.github/workflows/update-cddl.yml

MODULE.bazelRegister pinned Firefox grammar repositories +6/-2

Register pinned Firefox grammar repositories

• Adds four moz: CDDL repositories to the module extension's imports. Renames the Bluetooth grammar and definitions imports to match the advanced webref pin.

MODULE.bazel

webref_cddl.bzlFetch SHA-verified Firefox vendor grammars +29/-11

Fetch SHA-verified Firefox vendor grammars

• Pins the Firefox CDDL directory commit and registers its four files as hashed Bazel downloads, replacing references to local copies. Also advances the webref pin and adopts its Bluetooth file names.

common/webref_cddl.bzl

BUILD.bazelUse the renamed Bluetooth CDDL repository +1/-1

Use the renamed Bluetooth CDDL repository

• Points Python BiDi source generation at the Bluetooth repository name produced by the new webref pin.

py/BUILD.bazel

@qodo-code-review

qodo-code-review Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Vendor branch fallback goes untested ✗ Dismissed
Description
The new resolve_path_commit_for and pin_vendors logic has no focused tests for branch fallback
or directory enumeration. If Firefox moves or removes the grammar on a preferred branch, only the
scheduled update exercises that path, so a wrong selection can fail the update before it produces a
pin.
Code

scripts/update_cddl.py[R191-195]

+    for branch in branches:
+        data = get(f"https://api.github.com/repos/{repo}/commits?sha={branch}&path={path}&per_page=1", API_HEADERS)
+        commits = json.loads(data)
+        if commits:
+            return branch, commits[0]["sha"]
Evidence
Checklist item 3 requires appropriate coverage for changed behavior. The PR adds branch selection
and file discovery, while the script has only a binary target and the scheduled workflow runs that
binary without focused tests.

AGENTS.md: Include Focused Tests Without Misleading Mocks
scripts/update_cddl.py[189-196]
scripts/update_cddl.py[262-275]
scripts/BUILD.bazel[52-58]
.github/workflows/update-cddl.yml[12-20]

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 vendor pinning behavior lacks focused tests for ordered branch fallback and CDDL file discovery.

## Fix Focus Areas
- scripts/update_cddl.py[189-196]
- scripts/update_cddl.py[262-275]
- scripts/BUILD.bazel[52-58]

## Recommended Fix
Add small tests with representative API responses for release, beta, and main fallback, including a preferred branch whose directory was removed, and verify that all CDDL files at the selected commit are pinned.

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



Remediation recommended

2. New vendor files can break updates ✗ Dismissed
Description
vendor_repo_name() maps distinct filenames such as Foo-Bar.cddl and foo_bar.cddl to the same
Bazel repository name. If Firefox adds both files, pin_vendors() accepts them and
render_vendors() emits duplicate repository declarations, so the scheduled update cannot build the
generated schema.
Code

scripts/update_cddl.py[R257-259]

+def vendor_repo_name(namespace, filename):
+    stem = re.sub(r"[^a-z0-9]+", "_", Path(filename).stem.lower())
+    return f"{namespace}_{stem}_cddl"
Evidence
The name function lowercases stems and replaces punctuation with underscores; the file discovery
accepts every .cddl file, while rendering emits an entry for each one. The generated extension
passes each entry's name to http_file, and the module-name set does not preserve evidence of a
collision.

scripts/update_cddl.py[257-259]
scripts/update_cddl.py[267-279]
scripts/update_cddl.py[292-298]
common/webref_cddl.bzl[103-109]

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

## Issue description
Distinct vendor CDDL filenames can normalize to the same Bazel repository name, causing generated updates to fail.

## Fix Focus Areas
- scripts/update_cddl.py[257-259]
- scripts/update_cddl.py[262-279]

## Recommended Fix
Check that generated repository names are unique across vendor files before rendering or writing outputs. Raise an error identifying the conflicting filenames, or use a collision-free naming scheme.

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


3. Removed vendor grammars stop updates ✗ Dismissed
Description
resolve_path_commit_for() treats any historical commit touching the grammar directory as proof
that the directory exists on that branch, and returns a deletion or rename commit without checking
it. If Firefox removes or moves the directory on release while it remains on beta or main,
pin_vendors() fails when it lists the directory at that commit instead of trying the next branch.
Code

scripts/update_cddl.py[R192-195]

+        data = get(f"https://api.github.com/repos/{repo}/commits?sha={branch}&path={path}&per_page=1", API_HEADERS)
+        commits = json.loads(data)
+        if commits:
+            return branch, commits[0]["sha"]
Evidence
The new commit query selects the first branch with a commit touching the path, but performs no
existence check before returning it. pin_vendors() immediately requests that directory at the
returned commit, and get() raises on a non-200 response, so the ordered branch fallback cannot
continue. GitHub documents that the commits endpoint filters by path, whereas the contents endpoint
returns 404 when the requested path is absent.

scripts/update_cddl.py[189-196]
scripts/update_cddl.py[262-274]
scripts/update_cddl.py[176-180]
scripts/update_cddl.py[65-72]
🌐 The list-commits endpoint accepts a path filter for commits involving that path.
🌐 The repository-contents endpoint can return 404 when the requested path is not found.

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

## Issue description
A branch can have historical commits touching the vendor CDDL directory even when its latest such commit removes that directory. The updater then fails instead of trying the next configured branch.

## Fix Focus Areas
- scripts/update_cddl.py[189-196]
- scripts/update_cddl.py[262-274]

## Recommended Fix
Verify that the vendor CDDL directory exists and contains CDDL files at the selected commit before accepting a branch. If it does not, continue to the next configured branch; retain an error if none qualifies.

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This changes runtime build-generation logic, Bazel repository wiring, and external vendor pin resolution across multiple paths, creating genuine correctness and supply-chain risks that warrant a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit b7b7093

Results up to commit 0bad04a ⚖️ Balanced


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


Action required
1. Vendor branch fallback goes untested ✗ Dismissed
Description
The new resolve_path_commit_for and pin_vendors logic has no focused tests for branch fallback
or directory enumeration. If Firefox moves or removes the grammar on a preferred branch, only the
scheduled update exercises that path, so a wrong selection can fail the update before it produces a
pin.
Code

scripts/update_cddl.py[R191-195]

+    for branch in branches:
+        data = get(f"https://api.github.com/repos/{repo}/commits?sha={branch}&path={path}&per_page=1", API_HEADERS)
+        commits = json.loads(data)
+        if commits:
+            return branch, commits[0]["sha"]
Evidence
Checklist item 3 requires appropriate coverage for changed behavior. The PR adds branch selection
and file discovery, while the script has only a binary target and the scheduled workflow runs that
binary without focused tests.

AGENTS.md: Include Focused Tests Without Misleading Mocks
scripts/update_cddl.py[189-196]
scripts/update_cddl.py[262-275]
scripts/BUILD.bazel[52-58]
.github/workflows/update-cddl.yml[12-20]

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 vendor pinning behavior lacks focused tests for ordered branch fallback and CDDL file discovery.

## Fix Focus Areas
- scripts/update_cddl.py[189-196]
- scripts/update_cddl.py[262-275]
- scripts/BUILD.bazel[52-58]

## Recommended Fix
Add small tests with representative API responses for release, beta, and main fallback, including a preferred branch whose directory was removed, and verify that all CDDL files at the selected commit are pinned.

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



Remediation recommended
2. Removed vendor grammars stop updates ✗ Dismissed
Description
resolve_path_commit_for() treats any historical commit touching the grammar directory as proof
that the directory exists on that branch, and returns a deletion or rename commit without checking
it. If Firefox removes or moves the directory on release while it remains on beta or main,
pin_vendors() fails when it lists the directory at that commit instead of trying the next branch.
Code

scripts/update_cddl.py[R192-195]

+        data = get(f"https://api.github.com/repos/{repo}/commits?sha={branch}&path={path}&per_page=1", API_HEADERS)
+        commits = json.loads(data)
+        if commits:
+            return branch, commits[0]["sha"]
Evidence
The new commit query selects the first branch with a commit touching the path, but performs no
existence check before returning it. pin_vendors() immediately requests that directory at the
returned commit, and get() raises on a non-200 response, so the ordered branch fallback cannot
continue. GitHub documents that the commits endpoint filters by path, whereas the contents endpoint
returns 404 when the requested path is absent.

scripts/update_cddl.py[189-196]
scripts/update_cddl.py[262-274]
scripts/update_cddl.py[176-180]
scripts/update_cddl.py[65-72]
🌐 The list-commits endpoint accepts a path filter for commits involving that path.
🌐 The repository-contents endpoint can return 404 when the requested path is not found.

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

## Issue description
A branch can have historical commits touching the vendor CDDL directory even when its latest such commit removes that directory. The updater then fails instead of trying the next configured branch.

## Fix Focus Areas
- scripts/update_cddl.py[189-196]
- scripts/update_cddl.py[262-274]

## Recommended Fix
Verify that the vendor CDDL directory exists and contains CDDL files at the selected commit before accepting a branch. If it does not, continue to the next configured branch; retain an error if none qualifies.

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


Grey Divider

Qodo Logo

Comment thread scripts/update_cddl.py
Comment thread scripts/update_cddl.py
Comment thread scripts/update_cddl.py
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner merged commit bdb7ccc into SeleniumHQ:trunk Oct 1, 2026
53 checks passed
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 B-devtools Includes everything BiDi or Chrome DevTools related C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants