Skip to content

[rb] Implement driver methods for installing and uninstalling web extensions via BiDi - #17879

Merged
titusfortner merged 7 commits into
SeleniumHQ:trunkfrom
titusfortner:rb-install-web-extension
Sep 15, 2026
Merged

titusfortner merged 7 commits into
SeleniumHQ:trunkfrom
titusfortner:rb-install-web-extension

Conversation

@titusfortner

@titusfortner titusfortner commented Aug 5, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

Implements accepted ADR #17817 (tracking: #17933) for Ruby.

💥 What does this PR do?

Implements the ADR for the Ruby bindings:

  1. Adds cross-browser Driver#install_web_extension and Driver#uninstall_web_extension, backed by the
    generated BiDi webExtension protocol classes. Install accepts a directory, a packed archive, or
    base64 bytes (plus Firefox's permanent / allow_private_browsing options) and returns a
    WebExtension wrapping the id; uninstall takes that object.
  2. Backwards compatible: on Firefox the methods also work with BiDi off (classic moz/addon endpoint), and
    the legacy #install_addon / #uninstall_addon are deprecated toward the new methods — the two names
    converge rather than one routing to the other.
  3. Works with the Grid: a directory is uploaded to the remote end via se/file and referenced by the
    returned path, so unpacked-directory installs run on a Grid node, not just locally.
  4. On a non-BiDi session, a browser with no classic install path (Chromium, Safari, …) raises a clear
    "enable BiDi" error instead of NoMethodError.

🔧 Implementation Notes

  • The BiDi bridge picks the transport in one place (web_extension_data): a directory is uploaded via
    se/file and passed as a path; an archive or base64 travels inline as base64. This satisfies the ADR
    consequence that a client-local path can't be handed to a remote end — it must be delivered and referenced.
  • Zipper.zip_root packs a file or directory as the archive's single top-level entry, which is what the Grid
    se/file node unpack requires; the file-only zip_file is kept as an alias to it for backwards compatibility.
  • The BiDi-gated bridge methods share one "enable BiDi" raiser via aliases, so every entry point yields the
    same error on a non-BiDi session.

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: implementation, RBS signatures, and specs
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

@selenium-ci selenium-ci added C-rb Ruby Bindings B-build Includes scripting, bazel and CI integrations labels Aug 5, 2026
@titusfortner
titusfortner marked this pull request as ready for review August 5, 2026 23:26
@titusfortner
titusfortner marked this pull request as draft August 5, 2026 23:26
@qodo-code-review

qodo-code-review Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

[rb] Add cross-browser Driver#install_web_extension / uninstall_web_extension

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Adds cross-browser Driver#install_web_extension / #uninstall_web_extension backed by BiDi
 webExtension.
• Supports directory/archive/base64 inputs; Firefox falls back to classic moz/addon without BiDi.
• Adds Grid+BiDi coverage for directory upload, plus deprecations and helper refactors.
Diagram

graph TD
  A["Driver#install_web_extension"] --> B["Bridge (base)"]
  B -->|"BiDi enabled"| C["BiDiBridge"] --> D["webExtension BiDi protocol"]
  B -->|"Firefox, BiDi off"| E["Firefox::Features"] --> F["moz/addon classic endpoint"]
  E -->|"BiDi on"| D
  B -->|"no BiDi, no classic path"| G["Raise enable-BiDi error"]
  C --> H["WebExtension handle"]
  F --> H

  subgraph Legend
    direction LR
    _svc([Component]) ~~~ _ext{{External protocol}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Wait for chromium-bidi base64/archivePath support before shipping
2. Implement Chromium-only client-side pack/unpack + upload pipeline
  • ➕ Could enable archive/base64 install on Chromium sooner than upstream BiDi support
  • ➖ Adds significant complexity and duplicates transport/protocol concerns outside BiDi
  • ➖ Risks divergence from the intended BiDi protocol semantics and future maintenance burden

Recommendation: The chosen strategy (primary BiDi webExtension implementation + Firefox classic fallback + clear enable-BiDi errors) is the right incremental approach. Given chromium-bidi’s current limitations, shipping with explicit unsupported modes (and tests that assert them) is preferable to delaying the API or introducing a heavy Chromium-only transport workaround.

Files changed (24) +551 / -86

Enhancement (5) +108 / -7
common.rbRequire WebExtension class +1/-0

Require WebExtension class

• Loads the new common WebExtension class via selenium/webdriver/common.rb so it is available from the public API.

rb/lib/selenium/webdriver/common.rb

driver.rbExpose install/uninstall_web_extension on Driver +24/-0

Expose install/uninstall_web_extension on Driver

• Adds Driver#install_web_extension (delegates to bridge) and Driver#uninstall_web_extension (uses returned handle id) with API documentation and browser-specific notes.

rb/lib/selenium/webdriver/common/driver.rb

web_extension.rbAdd WebExtension handle wrapper +42/-0

Add WebExtension handle wrapper

• Introduces a WebExtension value object that stores the browser-assigned extension id for later uninstall.

rb/lib/selenium/webdriver/common/web_extension.rb

features.rbFirefox web extension install/uninstall (BiDi + classic fallback) +18/-7

Firefox web extension install/uninstall (BiDi + classic fallback)

• Reuses a shared encode_extension helper for classic install_addon, and adds install_web_extension/uninstall_web_extension that choose between BiDi moz vendor webExtension commands and the classic moz/addon endpoint depending on bidi?.

rb/lib/selenium/webdriver/firefox/features.rb

bidi_bridge.rbAdd BiDiBridge web extension support and Grid directory handling +23/-0

Add BiDiBridge web extension support and Grid directory handling

• Adds install_web_extension/uninstall_web_extension using the generated BiDi WebExtension protocol and a web_extension_data helper that uploads directories (se/file) for remote sessions while sending archives/base64 inline.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb

Refactor (4) +43 / -25
has_addons.rbDeprecate Firefox addon helpers toward new API +2/-0

Deprecate Firefox addon helpers toward new API

• Adds deprecation warnings to install_addon and uninstall_addon pointing users at install_web_extension/uninstall_web_extension.

rb/lib/selenium/webdriver/common/driver_extensions/has_addons.rb

zipper.rbAdd zip_root and share archive encoding logic +12/-11

Add zip_root and share archive encoding logic

• Refactors zip creation into a private encode_zip helper and adds zip_root to preserve a single top-level entry, matching Grid upload behavior. zip now delegates to encode_zip with the previous flattening semantics.

rb/lib/selenium/webdriver/common/zipper.rb

bridge.rbCentralize BiDi-gated errors and add encode_extension helper +20/-6

Centralize BiDi-gated errors and add encode_extension helper

• Makes bidi accept splats, aliases connection/web_extension/install/uninstall_web_extension to the same enable-BiDi error, adds bidi? predicate, and introduces encode_extension to normalize directory/file/base64 inputs.

rb/lib/selenium/webdriver/remote/bridge.rb

features.rbUse zip_root for se/file uploads and tighten validation path +9/-8

Use zip_root for se/file uploads and tighten validation path

• Changes upload to use Zipper.zip_root and moves non-file validation into upload_if_necessary (so upload itself is used after validation).

rb/lib/selenium/webdriver/remote/features.rb

Tests (6) +281 / -51
driver_spec.rbAdd BiDi integration coverage for install_web_extension +98/-0

Add BiDi integration coverage for install_web_extension

• Adds BiDi-enabled integration tests for installing from directory/archive/base64, asserting content injection and removal. Marks Chromium archive/base64 as pending due to upstream limitations.

rb/spec/integration/selenium/webdriver/driver_spec.rb

driver_spec.rbAdd classic-mode Firefox coverage for new API and deprecations +89/-36

Add classic-mode Firefox coverage for new API and deprecations

• Restructures existing tests under a BiDi-off context, asserts install_addon deprecation warnings, and adds classic install_web_extension tests including permanent and private-browsing behavior.

rb/spec/integration/selenium/webdriver/firefox/driver_spec.rb

options_spec.rbAdd unit coverage for BiDi not injecting debugging args +5/-0

Add unit coverage for BiDi not injecting debugging args

• Adds a unit test asserting Chrome Options JSON does not include injected debugging args when BiDi (web_socket_url) is enabled.

rb/spec/unit/selenium/webdriver/chrome/options_spec.rb

web_extension_spec.rbAdd unit coverage for WebExtension id +32/-0

Add unit coverage for WebExtension id

• Verifies WebExtension exposes the browser-assigned id via an accessor.

rb/spec/unit/selenium/webdriver/common/web_extension_spec.rb

bridge_spec.rbAdd unit coverage for upload validation and BiDi-gated extension errors +33/-6

Add unit coverage for upload validation and BiDi-gated extension errors

• Validates upload_if_necessary raises on non-file paths and asserts install/uninstall_web_extension raise a helpful enable-BiDi error when BiDi is not enabled, including for Chromium sessions.

rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb

zipper_spec.rbUpdate zipper unit tests for zip_root behavior +24/-9

Update zipper unit tests for zip_root behavior

• Replaces zip_file usage with zip_root and adds assertions that zip_root produces a single top-level entry for both file and directory inputs.

rb/spec/unit/selenium/webdriver/zipper_spec.rb

Other (9) +119 / -3
bridge.rbsAdd bridge interface types for web extension helpers +8/-0

Add bridge interface types for web extension helpers

• Extends the Bridge interface with bidi?, web_extension, web_extension_data, and encode_extension signatures.

rb/sig/interfaces/bridge.rbs

driver.rbsType Driver web extension methods +4/-0

Type Driver web extension methods

• Adds RBS signatures for Driver#install_web_extension and #uninstall_web_extension.

rb/sig/lib/selenium/webdriver/common/driver.rbs

web_extension.rbsAdd WebExtension RBS definitions +29/-0

Add WebExtension RBS definitions

• Defines the WebExtension class and its id accessor in RBS.

rb/sig/lib/selenium/webdriver/common/web_extension.rbs

zipper.rbsUpdate Zipper RBS for zip_root/encode_zip +3/-1

Update Zipper RBS for zip_root/encode_zip

• Replaces zip_file with zip_root and adds encode_zip signature.

rb/sig/lib/selenium/webdriver/common/zipper.rbs

features.rbsType Firefox Features web extension API +4/-0

Type Firefox Features web extension API

• Adds signatures for install_web_extension and uninstall_web_extension on Firefox::Features.

rb/sig/lib/selenium/webdriver/firefox/features.rbs

bidi_bridge.rbsType BiDiBridge web extension support +10/-0

Type BiDiBridge web extension support

• Adds the @web_extension ivar signature and types for install/uninstall_web_extension, web_extension, and web_extension_data.

rb/sig/lib/selenium/webdriver/remote/bidi_bridge.rbs

bridge.rbsType base Bridge BiDi predicates and web extension methods +10/-0

Type base Bridge BiDi predicates and web extension methods

• Adds bidi? plus install/uninstall_web_extension signatures and private web_extension/encode_extension definitions.

rb/sig/lib/selenium/webdriver/remote/bridge.rbs

BUILD.bazelGenerate Grid+BiDi integration targets for driver_spec +20/-1

Generate Grid+BiDi integration targets for driver_spec

• Introduces a _GRID_BIDI list and generates a bidi+grid test target for driver_spec.rb, plus ensures devtools tests run with bidi enabled in their Bazel target.

rb/spec/integration/selenium/webdriver/BUILD.bazel

tests.bzlAdd grid_bidi mode to rb_integration_test macro +31/-1

Add grid_bidi mode to rb_integration_test macro

• Extends the Bazel macro with a grid_bidi flag that generates an additional remote+BiDi target (WD_SPEC_DRIVER=remote + WEBDRIVER_BIDI=true).

rb/spec/tests.bzl

@qodo-code-review

qodo-code-review Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. devtools raises under BiDi ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
Calling driver.devtools now raises when @bridge.bidi? is true, which is a user-visible behavior
change for BiDi users without an explicit deprecation/migration period. This can break downstream
code that previously used CDP while experimenting with BiDi.
Code

rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb[R32-35]

+          if @bridge.bidi?
+            raise Error::WebDriverError,
+                  'CDP (DevTools) is disabled when BiDi is enabled; use the WebDriver BiDi APIs instead'
+          end
Evidence
PR Compliance ID 1 requires preserving public behavior by default. The changed devtools method now
explicitly raises an error whenever BiDi is enabled, making driver.devtools unusable in that
configuration.

AGENTS.md: Maintain API/ABI Compatibility by Default
rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb[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
`Driver#devtools` now raises when BiDi is enabled, which is a breaking user-visible behavior change without an explicit deprecation/migration period.

## Issue Context
The PR introduces a hard failure path (`raise Error::WebDriverError`) for `devtools` when `@bridge.bidi?` is true. To maintain compatibility expectations, provide an explicit deprecation/migration path (or a documented compatibility switch) before enforcing this behavior.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb[28-35]

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


2. BiDi args NoMethodError ⊘ Outdated 🐞 Bug ☼ Reliability
Description
In Chromium::Options#process_browser_options, when BiDi is enabled it calls options['args'].to_a,
which raises NoMethodError if args is a String (or other non-Array) that can be set via Options
initialization/merge or add_option. This crashes option processing before session start, instead
of producing a clear argument/type error.
Code

rb/lib/selenium/webdriver/chromium/options.rb[R239-241]

+          if bidi?
+            options['args'] = options['args'].to_a | %w[--enable-unsafe-extension-debugging --remote-debugging-pipe]
+          end
Evidence
process_browser_options unconditionally calls options['args'].to_a under BiDi, but args can be
overridden to a non-Array via the options merge and via add_option, so BiDi-enabled sessions can
fail locally with NoMethodError.

rb/lib/selenium/webdriver/chromium/options.rb[227-241]
rb/lib/selenium/webdriver/chromium/options.rb[72-85]
rb/lib/selenium/webdriver/common/options.rb[93-96]

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

### Issue description
When BiDi is enabled, Chromium options processing does `options['args'].to_a`, which can raise `NoMethodError` if `args` is not an Array (e.g., a String). This prevents session creation and yields an opaque error.

### Issue Context
`Chromium::Options#initialize` merges defaults with user-supplied `@options`, so `args:` provided as a non-Array can override the default `[]`. `Common::Options#add_option` also stores values without type validation.

### Fix Focus Areas
- rb/lib/selenium/webdriver/chromium/options.rb[227-241]

### Suggested change
- Replace `options['args'].to_a` with safer normalization, e.g.:
 - `args = options['args']
   args = args.nil? ? [] : Array(args)
   options['args'] = args | %w[--enable-unsafe-extension-debugging --remote-debugging-pipe]`
- Alternatively, if you want strictness, explicitly raise a `WebDriverError` when `options['args']` is present and not an `Array`, with a clear message (`'args' must be an Array of Strings`).

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced: Comparison failure: full PR diff.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb Outdated
Comment thread rb/lib/selenium/webdriver/chromium/options.rb Outdated
@titusfortner
titusfortner force-pushed the rb-install-web-extension branch 4 times, most recently from dfa8d6b to 9e869f9 Compare August 24, 2026 18:04
@titusfortner
titusfortner marked this pull request as ready for review August 24, 2026 18:04
@qodo-code-review

qodo-code-review Bot commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Chromium extension flags missing ✗ Dismissed 🐞 Bug ≡ Correctness
Description
install_web_extension invokes Chromium’s BiDi command without enabling --remote-debugging-pipe
and --enable-unsafe-extension-debugging. Consequently, callers that only enable BiDi—as the new
public API requires—cannot install a Chromium extension unless they discover and add both internal
flags themselves.
Code

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[R48-50]

+        def install_web_extension(path)
+          result = web_extension.install(extension_data: web_extension_data(path))
+          WebExtension.new(result.extension)
Evidence
The added bridge method is the new Chromium installation route. Chromium options do not add either
required argument when web_socket_url is set, and the pre-existing protocol extension integration
test explicitly has to add both flags before every Chromium installation.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[48-50]
rb/lib/selenium/webdriver/chromium/options.rb[72-91]
rb/spec/integration/selenium/webdriver/bidi/protocol/web_extension_spec.rb[32-49]
py/selenium/webdriver/chromium/options.py[140-156]

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

## Issue description
Chromium WebExtension installation requires `--remote-debugging-pipe` and `--enable-unsafe-extension-debugging`, but the Ruby BiDi path does not add them when BiDi is enabled. The new public installation API therefore fails for ordinary Chromium BiDi sessions.

## Issue Context
Existing Chromium protocol integration coverage manually resets the driver with both flags before installing an extension, while `Chromium::Options` only requests `webSocketUrl` and leaves its argument list unchanged.

## Fix Focus Areas
- rb/lib/selenium/webdriver/chromium/options.rb[72-91]
- rb/lib/selenium/webdriver/remote/bidi_bridge.rb[48-50]
- rb/spec/unit/selenium/webdriver/chrome/options_spec.rb[286-289]

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



Remediation recommended

2. Trailing slash flattens uploads ✗ Dismissed 🐞 Bug ≡ Correctness
Description
zip_root derives its archive base directly from the input, so a directory path ending in a
separator makes File.dirname(path) equal the directory itself and strips the directory name from
every archive entry. Remote install_web_extension then uploads an archive with multiple top-level
entries (which Grid rejects) or receives a child path instead of the extension root.
Code

rb/lib/selenium/webdriver/common/zipper.rb[R64-65]

+        def zip_root(path)
+          encode_zip(path, File.dirname(path))
Evidence
web_extension_data sends directories through upload, which uses zip_root. zip_root passes
File.dirname(path) as the base; for a trailing-separator directory, descendant names are therefore
stripped to the directory's contents rather than retaining the root. Grid explicitly requires
exactly one extracted top-level entry and returns that entry as the remote path, proving the
flattened archive either fails or points at the wrong location.

rb/lib/selenium/webdriver/remote/bidi_bridge.rb[108-115]
rb/lib/selenium/webdriver/remote/features.rb[44-46]
rb/lib/selenium/webdriver/common/zipper.rb[62-74]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1133-1144]

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

## Issue description
`Zipper.zip_root` does not preserve the root directory when its input has a trailing path separator. This breaks remote/Grid installation for valid directory paths such as `/tmp/extension/`.

## Issue Context
Normalize the input path before deriving its parent and traversing it, while retaining the intended single top-level archive entry. Add coverage for directory paths ending in a separator and assert that extraction still yields the original directory name as the sole top-level entry.

## Fix Focus Areas
- rb/lib/selenium/webdriver/common/zipper.rb[62-65]
- rb/spec/unit/selenium/webdriver/zipper_spec.rb[74-84]

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced: Comparison failure: full PR diff.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread rb/lib/selenium/webdriver/common/zipper.rb
Comment thread rb/lib/selenium/webdriver/remote/bidi_bridge.rb
@titusfortner titusfortner changed the title [rb] Add cross-browser Driver#install_web_extension [rb] Implement driver methods for installing and uninstalling web extensions via BiDi Aug 24, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 7e229b9

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 3b7ab15

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner force-pushed the rb-install-web-extension branch from bbbfaff to 6274d7b Compare September 15, 2026 18:57
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 6274d7b

@titusfortner
titusfortner merged commit d32c5f2 into SeleniumHQ:trunk Sep 15, 2026
26 checks passed
AutomatedTester added a commit that referenced this pull request Sep 16, 2026
The grid_bidi flag added in #17879 was only enabled for driver_spec, so
the BiDi spec suites themselves still only ever ran against a local
driver and nothing exercised BiDi over Grid's websocket proxy.

Enable it for the remaining BiDi suites: bidi/, bidi/protocol/, and the
bidi_spec/network_spec/navigation_spec entries in the integration root.
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 C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants