You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
• 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.
• Adds four moz: CDDL repositories to the module extension's imports. Renames the Bluetooth grammar and definitions imports to match the advanced webref pin.
• 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.
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.
+ 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.
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
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.
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.
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
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.
+ 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.
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
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.
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
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.
+ 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.
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
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.
+ 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.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
B-buildIncludes scripting, bazel and CI integrationsB-devtoolsIncludes everything BiDi or Chrome DevTools relatedC-pyPython Bindings
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 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.bazelstill named)💥 What does this PR do?
moz:CDDL grammars are now pinned frommozilla-firefox/firefoxbyscripts/update_cddl.pyinstead of being copied intocommon/bidi🔧 Implementation Notes
common/bidi/schema.json; the upstream files are byte-identical to the deleted copies.cddlfile inremote/webdriver-bidi/cddlis merged, so a new Firefox module does not need a script changerelease, thenbeta, thenmain); today that ismain, because the directory first appears in Firefox 159bluetooth-scanningtobluetooth;py/BUILD.bazelis updated to match🤖 AI assistance
💡 Additional Considerations
release, the pinned schema describes stable Firefox, somoz:features that are only in beta are missing from it until they shipbrowser/config/version.txtat the commit that added it) as asincefield in the schema'svendorsection, and guard tests on the browser versionbetainstead, and guard the stable tests for features it does not have yet🔄 Types of changes