Skip to content

[rb] tolerate and warn on missing required inbound BiDi fields, with SE_BIDI_STRICT to escalate - #17844

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:rb-bidi-inbound-tolerance
Jul 29, 2026
Merged

[rb] tolerate and warn on missing required inbound BiDi fields, with SE_BIDI_STRICT to escalate#17844
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:rb-bidi-inbound-tolerance

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

Brings the Ruby low-level BiDi layer into compliance with the updated behavioral contract proposed in #17786

💥 What does this PR do?

  • A required field missing from an inbound BiDi message is now tolerated: it stays omitted and the layer warns, instead of erroring and rejecting the whole message. This keeps callers unblocked when Selenium's schema is ahead of the browser
  • An omitted field stays distinct from an explicit null for required-and-nullable fields (network context/navigation, response sizes, log text, ~30 in all), so a caller can tell "the browser didn't send it" from "sent null"
  • An undeclared inbound property now warns (in addition to the existing tolerate/retain behavior), on both extensible types (kept) and closed types (dropped).
  • Setting SE_BIDI_STRICT restores the previous strict behavior: a missing required inbound field escalates from a warning to an error.

🔧 Implementation Notes

  • All changes live in the handwritten runtime bases (serialization.rb, serialization/record.rb) that every generated protocol class builds on — no generator or per-type changes. The omit sentinel (UNSET) already existed and is distinct from nil, so required the same one-line change in wire_value.
  • Warnings are tagged id: :bidi_missing_required / :bidi_undeclared_property, so callers can silence a category via logger.ignore(...), and tests assert them with the have_warning matcher. They fire on every occurrence (the Ruby logger does not de-duplicate by id).

🤖 AI assistance

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

💡 Additional Considerations

  • This code will continue to be updated based on decisions made in that ADR if we decide to change an item in it.
  • Follow-up: agree on the SE_BIDI_STRICT name (or a shared strict-mode mechanism) across bindings.

🔄 Types of changes

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

@selenium-ci selenium-ci added B-grid Everything grid and server related C-py Python Bindings C-rb Ruby Bindings C-dotnet .NET Bindings C-java Java Bindings C-nodejs JavaScript Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Jul 29, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Advertise se:remoteUrl across bindings; make Grid prefer it for proxied URLs

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Inject se:remoteUrl in clients so Grid can build client-reachable proxied URLs.
• Prefer configured grid-url, otherwise use se:remoteUrl, and strip it from responses.
• Make Ruby BiDi inbound parsing warn on schema drift, with strict-mode escalation.
Diagram

graph TD
  A["Bindings: .NET/Java/JS/Py/Rb"] --> B["New Session payload"] --> C["Grid/Node: LocalNode"] --> D["Resolve public base URI"] --> E["Rewrite proxied URLs (CDP/BiDi/VNC)"] --> F["Return session caps (sanitized)"]
  G["Ruby BiDi Record deserializer"] --> H["Warn or error on missing/extra fields"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize se:remoteUrl injection in a shared remote-session builder API
  • ➕ Reduces duplicated logic across bindings
  • ➕ Makes precedence rules (explicit vs injected) consistent by construction
  • ➖ Not feasible across languages without broader refactors
  • ➖ Would delay delivering the capability to existing users
2. Make strict-mode configurable via logger/policy object instead of ENV var
  • ➕ More discoverable and testable than global environment
  • ➕ Allows per-driver or per-session strictness
  • ➖ Requires public API surface decisions and coordination across bindings
  • ➖ More code churn than a simple compatibility toggle

Recommendation: The PR’s approach is pragmatic: injecting se:remoteUrl in each binding immediately enables Grid to produce client-reachable proxied URLs, while the Node’s precedence rules (grid-url wins) avoid surprising operators. For Ruby BiDi, warning-by-default with an opt-in strict toggle is a good forward-compat stance; consider a future cross-binding strictness mechanism, but the ENV var is a reasonable interim solution.

Files changed (18) +559 / -32

Enhancement (11) +162 / -11
HttpCommandExecutor.csExpose RemoteServerUri for capability advertisement +5/-0

Expose RemoteServerUri for capability advertisement

• Adds an internal accessor to the remote server URI so higher-level session creation can advertise the effective remote endpoint in capabilities.

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs

WebDriver.csAdvertise se:remoteUrl and avoid mutating caller settings +59/-1

Advertise se:remoteUrl and avoid mutating caller settings

• Injects 'se:remoteUrl' into new-session capabilities based on 'HttpCommandExecutor'’s remote URI. For 'RemoteSessionSettings', builds a copied 'alwaysMatch' dictionary and only injects when 'se:remoteUrl' is not already present in 'alwaysMatch'/'firstMatch', avoiding overlaps and not mutating caller-owned settings.

dotnet/src/webdriver/WebDriver.cs

LocalNodeFactory.javaWire gridUrlSpecified flag from config into LocalNode builder +1/-0

Wire gridUrlSpecified flag from config into LocalNode builder

• Passes whether 'public-grid-uri' was provided into the 'LocalNode' builder so the node can decide when client-advertised 'se:remoteUrl' is allowed to influence proxied URL rewriting.

java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java

RemoteWebDriver.javaSet client base URL and advertise se:remoteUrl for remote sessions +13/-2

Set client base URL and advertise se:remoteUrl for remote sessions

• Ensures the 'ClientConfig' base URL is set from the provided remote address. Adds 'se:remoteUrl' to capabilities at session start when a base URI exists, while leaving local-driver sessions unchanged (base URI is null).

java/src/org/openqa/selenium/remote/RemoteWebDriver.java

RemoteWebDriverBuilder.javaAuto-inject se:remoteUrl unless explicitly provided +17/-1

Auto-inject se:remoteUrl unless explicitly provided

• Builds a fresh 'alwaysMatch' map and injects 'se:remoteUrl' from the builder’s base URI when running against a remote server (no DriverService) and no requested capability explicitly sets it, preventing alwaysMatch/firstMatch conflicts.

java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java

index.jsAdvertise se:remoteUrl when Builder is given a string URL +5/-0

Advertise se:remoteUrl when Builder is given a string URL

• When creating a remote session with a string URL, sets 'se:remoteUrl' on capabilities so Grid can prefer the client-reachable endpoint for proxied URLs.

javascript/selenium-webdriver/index.js

webdriver.pyAdvertise se:remoteUrl for remote sessions, not local services +10/-0

Advertise se:remoteUrl for remote sessions, not local services

• Adds '_remote_url' helper to detect the remote server address from the command executor’s client config, and injects 'se:remoteUrl' into session capabilities only when not using a local DriverService.

py/selenium/webdriver/remote/webdriver.py

serialization.rbAdd SE_BIDI_STRICT toggle for inbound strictness +13/-1

Add SE_BIDI_STRICT toggle for inbound strictness

• Introduces 'Serialization.strict?' to control whether missing required inbound fields should raise or be tolerated with warnings, based on the 'SE_BIDI_STRICT' environment variable.

rb/lib/selenium/webdriver/bidi/serialization.rb

record.rbWarn on missing required fields and undeclared properties (strict-mode optional) +30/-5

Warn on missing required fields and undeclared properties (strict-mode optional)

• Changes record deserialization to treat missing required fields as omitted ('UNSET') while warning, escalating to an error under strict mode. Also warns on undeclared inbound properties while preserving them for extensible records and dropping them for closed records.

rb/lib/selenium/webdriver/bidi/serialization/record.rb

driver.rbAdvertise se:remoteUrl from Ruby remote driver initialization +3/-1

Advertise se:remoteUrl from Ruby remote driver initialization

• Captures the final server URL used by the HTTP client and injects a normalized (no trailing slash) 'se:remoteUrl' capability so Grid can compute client-reachable proxied URLs.

rb/lib/selenium/webdriver/remote/driver.rb

serialization.rbsUpdate RBS signatures for strict mode and new warning helpers +6/-0

Update RBS signatures for strict mode and new warning helpers

• Adds type signatures for 'Serialization.strict?', 'missing_required', and 'warn_undeclared' to match the updated Ruby BiDi runtime behavior.

rb/sig/lib/selenium/webdriver/bidi/serialization.rbs

Bug fix (1) +64 / -7
LocalNode.javaPrefer configured grid-url; otherwise use se:remoteUrl for proxied URLs +64/-7

Prefer configured grid-url; otherwise use se:remoteUrl for proxied URLs

• Tracks whether the grid URL was explicitly configured and resolves the public base URI accordingly. Uses the resolved base URI when rewriting CDP/BiDi/VNC websocket URLs, normalizes scheme case-insensitively, strips transport-only 'se:remoteUrl' from returned capabilities, and warns when 'se:remoteUrl' is unusable.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java

Tests (6) +333 / -14
NodeTest.javaTest se:remoteUrl precedence, credentials retention, and fallback behavior +191/-0

Test se:remoteUrl precedence, credentials retention, and fallback behavior

• Adds coverage ensuring proxied URLs (e.g., 'se:vnc') use client-provided 'se:remoteUrl' when appropriate, preserve embedded credentials, and are ignored when 'grid-url' is configured (even if equal to node URI). Also verifies non-http schemes are rejected and default fallback uses the grid URI.

java/test/org/openqa/selenium/grid/node/NodeTest.java

RemoteWebDriverBuilderTest.javaTest builder advertises se:remoteUrl and skips for DriverService +47/-0

Test builder advertises se:remoteUrl and skips for DriverService

• Verifies 'RemoteWebDriver.builder().address(...)' injects 'se:remoteUrl' into the payload, and that using a 'DriverService' suppresses advertising the remote URL.

java/test/org/openqa/selenium/remote/RemoteWebDriverBuilderTest.java

RemoteWebDriverUnitTest.javaTest RemoteWebDriver injects se:remoteUrl only for remote sessions +26/-0

Test RemoteWebDriver injects se:remoteUrl only for remote sessions

• Adds unit tests confirming 'se:remoteUrl' is present when starting a remote session with a non-null 'ClientConfig.baseUri', and absent for local-style sessions where baseUri is null.

java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java

new_session_tests.pyAdjust session tests for se:remoteUrl injection and local-driver suppression +32/-2

Adjust session tests for se:remoteUrl injection and local-driver suppression

• Updates existing assertions to ignore injected 'se:remoteUrl', and adds new tests validating 'se:remoteUrl' is included for remote sessions and omitted for local-driver-like sessions that set 'service' before session start.

py/test/unit/selenium/webdriver/remote/new_session_tests.py

serialization_spec.rbAdd warning/strict-mode coverage for BiDi inbound schema drift +33/-10

Add warning/strict-mode coverage for BiDi inbound schema drift

• Updates extensible/closed record tests to assert warnings on undeclared properties. Replaces the previous hard-error expectation for missing required fields with warning + 'UNSET' behavior, and adds a strict-mode test asserting escalation when 'SE_BIDI_STRICT' is enabled.

rb/spec/unit/selenium/webdriver/bidi/serialization_spec.rb

driver_spec.rbAdjust remote driver request stubs for se:remoteUrl injection +4/-2

Adjust remote driver request stubs for se:remoteUrl injection

• Updates request expectations to include injected 'se:remoteUrl' derived from the endpoint URL (without '/session'), keeping existing tests stable while validating the new payload shape.

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

@qodo-code-review

qodo-code-review Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Remote::Driver overwrites se:remoteUrl ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The Ruby remote driver now unconditionally sets caps['se:remoteUrl'], overwriting any
caller-provided value. This is a public behavior change that can break users who explicitly set
se:remoteUrl (e.g., to advertise a proxy-reachable URL).
Code

rb/lib/selenium/webdriver/remote/driver.rb[R40-42]

+          server_url = url || client_config&.server_url || "http://#{Platform.localhost}:4444/wd/hub"
+          http_client.server_url = server_url
+          caps['se:remoteUrl'] = server_url.to_s.chomp('/')
Evidence
PR Compliance ID 1 requires backward-compatible public behavior. The new code overwrites
se:remoteUrl regardless of whether the caller already set it.

AGENTS.md: Maintain API/ABI Compatibility for Public Interfaces
rb/lib/selenium/webdriver/remote/driver.rb[40-42]

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

## Issue description
`rb/lib/selenium/webdriver/remote/driver.rb` unconditionally assigns `caps['se:remoteUrl']`, which can clobber a user-supplied capability value.

## Issue Context
Compliance requires public-interface behavior changes to remain backward compatible. Callers who already set `se:remoteUrl` lose control of that value.

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/driver.rb[40-42]

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


2. JS remoteUrl missing for Promise ✓ Resolved 🐞 Bug ≡ Correctness
Description
In JS Builder.build(), se:remoteUrl is only set when the remote URL is a string, so sessions
started via SELENIUM_SERVER_JAR (which produces a Promise<string>) won't advertise the
client-reachable URL. This causes Grid nodes to fall back to their auto-detected address when
generating proxied ws/CDP/VNC URLs, which can be unreachable behind Docker/proxy.
Code

javascript/selenium-webdriver/index.js[R662-664]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence
startSeleniumServer() returns a promise, and build() already supports thenable URLs for the HTTP
client via Promise.resolve(url), but capability injection is gated to strings only. The Grid node
consumes se:remoteUrl for proxy URL rewriting only when configured grid-url is absent; otherwise
it falls back to its own gridUri.

javascript/selenium-webdriver/index.js[57-68]
javascript/selenium-webdriver/index.js[647-679]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1320-1346]

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

### Issue description
`se:remoteUrl` is currently injected only when `url` is a primitive string. When `SELENIUM_SERVER_JAR` is used, `startSeleniumServer()` returns a `Promise<string>`, and the capability is never advertised.

### Issue Context
`Builder.build()` already handles async URLs for HTTP transport via `Promise.resolve(url)`; the `se:remoteUrl` injection should use the same resolved URL so the Grid can rewrite proxied endpoints using the client-reachable base.

### Fix Focus Areas
- javascript/selenium-webdriver/index.js[658-668]

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


3. WebDriver.start_session overwrites se:remoteUrl ✓ Resolved 📘 Rule violation ≡ Correctness
Description
Python WebDriver.start_session injects se:remoteUrl by rebuilding the capabilities dict such
that the injected value overwrites any caller-provided se:remoteUrl. This is a
backward-incompatible behavior change that prevents users from explicitly advertising a different
client-reachable URL (e.g., via tunnel/proxy/LB) and diverges from other bindings that avoid
injection when the capability is already set.
Code

py/selenium/webdriver/remote/webdriver.py[R394-396]

+        remote_url = self._remote_url()
+        if remote_url:
+            capabilities = {**capabilities, "se:remoteUrl": remote_url}
Evidence
PR Compliance ID 1 requires preserving backward compatibility, but the current merge pattern
{**capabilities, 'se:remoteUrl': remote_url} places se:remoteUrl last, which necessarily
replaces any existing se:remoteUrl supplied by the caller. The cited behavior is specifically in
Python’s start_session, and contrasts with other bindings mentioned in the findings (Java’s
builder and .NET’s RemoteSessionSettings path) that explicitly suppress auto-injection when
se:remoteUrl is already present, preserving the user-provided value.

AGENTS.md: Maintain API/ABI Compatibility for Public Interfaces
py/selenium/webdriver/remote/webdriver.py[388-397]
py/selenium/webdriver/remote/webdriver.py[388-413]
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[529-550]
dotnet/src/webdriver/WebDriver.cs[622-651]

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

## Issue description
Python’s `WebDriver.start_session` currently injects `se:remoteUrl` by rebuilding the capabilities dict in a way that overwrites any existing caller-provided `se:remoteUrl` (e.g., via `capabilities = {**capabilities, "se:remoteUrl": remote_url}`), which changes public behavior and breaks backward compatibility for users who explicitly set this capability.

## Issue Context
Compliance requirements call for preserving backward compatibility for public behavior. If a user sets `se:remoteUrl` explicitly, Python’s auto-injection should not clobber that value; this also aligns Python with other bindings in this PR that suppress injection when `se:remoteUrl` is already present, allowing callers to advertise a different client-reachable URL (tunnel/proxy/LB).

## Fix Focus Areas
- py/selenium/webdriver/remote/webdriver.py[391-397]

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


View more (3)
4. WebDriver.StartSession overwrites se:remoteUrl ✓ Resolved 📘 Rule violation ≡ Correctness
Description
In the non-RemoteSessionSettings path, StartSession unconditionally sets
matchCapabilities['se:remoteUrl'], overwriting a caller-provided se:remoteUrl. This breaks
backward compatibility for consumers who explicitly set this capability.
Code

dotnet/src/webdriver/WebDriver.cs[R607-610]

+            if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
+            {
+                matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
+            }
Evidence
PR Compliance ID 1 requires public behavior to remain backward compatible. The injected assignment
replaces any existing se:remoteUrl value in matchCapabilities.

AGENTS.md: Maintain API/ABI Compatibility for Public Interfaces
dotnet/src/webdriver/WebDriver.cs[605-610]

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

## Issue description
`dotnet/src/webdriver/WebDriver.cs` overwrites a user-supplied `se:remoteUrl` in the `capabilities is not RemoteSessionSettings` code path.

## Issue Context
PR Compliance ID 1 forbids breaking behavior changes to public surfaces. Callers should be able to explicitly set `se:remoteUrl` without it being replaced.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[605-610]

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


5. Builder overwrites se:remoteUrl ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The JS builder sets capabilities.set('se:remoteUrl', url) whenever url is a string, overwriting
any existing se:remoteUrl the caller configured. This is a backward-incompatible behavior change
for callers who explicitly set that capability.
Code

javascript/selenium-webdriver/index.js[R662-664]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence
PR Compliance ID 1 requires maintaining backward-compatible public behavior. The new assignment
always writes se:remoteUrl when connecting to a string URL, regardless of whether the capability
was already set.

AGENTS.md: Maintain API/ABI Compatibility for Public Interfaces
javascript/selenium-webdriver/index.js[659-665]

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

## Issue description
`javascript/selenium-webdriver/index.js` unconditionally sets `se:remoteUrl` when `url` is a string, which can overwrite a user-provided `se:remoteUrl`.

## Issue Context
PR Compliance ID 1 requires backward-compatible public behavior. Auto-injection should be suppressed when the capability is explicitly provided.

## Fix Focus Areas
- javascript/selenium-webdriver/index.js[659-665]

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


6. RemoteWebDriver overwrites se:remoteUrl ✓ Resolved 📘 Rule violation ≡ Correctness
Description
Java RemoteWebDriver now injects se:remoteUrl via setCapability during startSession,
overwriting any explicit se:remoteUrl provided by the caller whenever ClientConfig.baseUri() is
non-null. This potentially breaks backward compatibility for consumers relying on an explicitly set
value and conflicts with the suppression policy already implemented in RemoteWebDriverBuilder for
advanced proxy/tunnel use cases.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R263-270]

+  private Capabilities addRemoteUrl(Capabilities capabilities) {
+    URI baseUri = clientConfig.baseUri();
+    if (baseUri == null) {
+      return capabilities;
+    }
+    MutableCapabilities withRemoteUrl = new MutableCapabilities(capabilities);
+    withRemoteUrl.setCapability("se:remoteUrl", baseUri.toString());
+    return withRemoteUrl;
Evidence
PR Compliance ID 1 calls for backward-compatible public behavior, but the new addRemoteUrl logic
creates a mutable copy of the caller-provided capabilities and unconditionally sets se:remoteUrl
whenever clientConfig.baseUri() != null, thereby replacing any existing caller-supplied value;
startSession then uses this modified capabilities object, causing the overwrite to take effect. In
contrast, the Java RemoteWebDriverBuilder code in the same PR explicitly checks for se:remoteUrl
being already requested/present and avoids injecting it in that case, demonstrating the intended
behavior and highlighting the inconsistency with RemoteWebDriver’s unconditional injection.

AGENTS.md: Maintain API/ABI Compatibility for Public Interfaces
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[263-271]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[263-279]
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[529-550]

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

## Issue description
`java/src/org/openqa/selenium/remote/RemoteWebDriver.java` unconditionally injects `se:remoteUrl` (derived from `ClientConfig.baseUri()`) by setting the capability on a copy during `startSession`, which overwrites any caller-provided `se:remoteUrl` when `baseUri` is non-null.

## Issue Context
PR Compliance ID 1 requires backward-compatible public behavior, so explicitly supplied capabilities should not be clobbered by auto-injection. Additionally, `RemoteWebDriverBuilder` in this PR already suppresses injection when `se:remoteUrl` is explicitly present/requested, and the current `RemoteWebDriver` behavior can block advanced proxy/tunnel setups that rely on a custom `se:remoteUrl`.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[263-277]
- java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[529-546]

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



Remediation recommended

7. C# se:remoteUrl untested ✓ Resolved 📘 Rule violation ☼ Reliability
Description
The C# session-creation payload is behaviorally changed by injecting se:remoteUrl, but this PR
does not add/update any .NET tests to validate the new payload behavior. Lack of targeted tests
increases regression risk for session creation.
Code

dotnet/src/webdriver/WebDriver.cs[R607-610]

+            if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
+            {
+                matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
+            }
Evidence
PR Compliance ID 5 expects behavior changes to be covered by tests. The PR adds new C# logic that
changes the new-session payload, but no .NET test changes accompany it.

AGENTS.md: Prefer Adding/Updating Tests for Changes; Prefer Small Unit Tests Over Browser Tests
dotnet/src/webdriver/WebDriver.cs[605-650]

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

## Issue description
C# now injects `se:remoteUrl` during session creation, but there are no corresponding .NET tests in this PR asserting the new capabilities payload shape/override behavior.

## Issue Context
PR Compliance ID 5 prefers adding/updating tests for behavior changes to reduce regression risk.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[605-650]

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


8. JS se:remoteUrl untested ✓ Resolved 📘 Rule violation ☼ Reliability
Description
The JS builder now injects se:remoteUrl for string remote URLs, but this PR does not add/update JS
tests to validate the new session payload behavior. This increases the chance of unnoticed
regressions in client session creation.
Code

javascript/selenium-webdriver/index.js[R662-664]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence
PR Compliance ID 5 requires tests for behavior-changing code. The PR adds new logic that mutates
capabilities before session creation, with no accompanying JS test updates in this change.

AGENTS.md: Prefer Adding/Updating Tests for Changes; Prefer Small Unit Tests Over Browser Tests
javascript/selenium-webdriver/index.js[659-668]

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 JavaScript `Builder` now sets `se:remoteUrl` when connecting to a remote URL string, but this PR includes no test coverage to verify the injection and its interaction with user-supplied capabilities.

## Issue Context
PR Compliance ID 5 prefers tests for behavior changes, ideally small unit tests.

## Fix Focus Areas
- javascript/selenium-webdriver/index.js[659-668]

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


9. Python tests mock execute() ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
New/modified Python unit tests validate se:remoteUrl injection by mocking WebDriver.execute,
substituting a core API boundary with a mock. This conflicts with the guideline to avoid mocks that
can diverge from real behavior.
Code

py/test/unit/selenium/webdriver/remote/new_session_tests.py[R45-56]

+def test_advertises_remote_url_for_remote_session(mocker):
+    mock = mocker.patch("selenium.webdriver.remote.webdriver.WebDriver.execute")
+    driver = WebDriver(command_executor="http://remote.example:4444", options=ArgOptions())
+    command, params = mock.call_args[0]
+    assert command == Command.NEW_SESSION
+    assert driver._remote_url() is not None
+    assert params["capabilities"]["alwaysMatch"]["se:remoteUrl"] == driver._remote_url()
+
+
+def test_does_not_advertise_remote_url_for_local_driver(mocker):
+    mock = mocker.patch("selenium.webdriver.remote.webdriver.WebDriver.execute")
+
Evidence
PR Compliance ID 6 asks to avoid mocks in tests. The updated tests use mocker.patch to replace
WebDriver.execute to inspect the new-session payload.

AGENTS.md: Avoid Mocks in Tests
py/test/unit/selenium/webdriver/remote/new_session_tests.py[30-68]

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 `se:remoteUrl` tests mock `selenium.webdriver.remote.webdriver.WebDriver.execute`, which can make tests less representative of real behavior.

## Issue Context
PR Compliance ID 6 prefers tests that exercise real implementations or realistic fixtures over mocks.

## Fix Focus Areas
- py/test/unit/selenium/webdriver/remote/new_session_tests.py[30-68]

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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread rb/lib/selenium/webdriver/remote/driver.rb Outdated
Comment thread dotnet/src/webdriver/WebDriver.cs Outdated
Comment thread py/selenium/webdriver/remote/webdriver.py Outdated
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java Outdated
Comment thread javascript/selenium-webdriver/index.js Outdated
Comment thread dotnet/src/webdriver/WebDriver.cs Outdated
Comment thread javascript/selenium-webdriver/index.js Outdated
Comment thread py/test/unit/selenium/webdriver/remote/new_session_tests.py Outdated
Comment thread javascript/selenium-webdriver/index.js Outdated
@titusfortner
titusfortner force-pushed the rb-bidi-inbound-tolerance branch from c761c6f to 34cc62a Compare July 29, 2026 14:15
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 34cc62a

@titusfortner
titusfortner merged commit 618f12b into SeleniumHQ:trunk Jul 29, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related B-grid Everything grid and server related C-dotnet .NET Bindings C-java Java Bindings C-nodejs JavaScript Bindings C-py Python Bindings C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants