Skip to content

[grid] honor client-advertised se:remoteUrl for reachable BiDi/CDP/VNC URLs - #17790

Open
titusfortner wants to merge 4 commits into
SeleniumHQ:trunkfrom
titusfortner:grid-se-remote-url
Open

[grid] honor client-advertised se:remoteUrl for reachable BiDi/CDP/VNC URLs#17790
titusfortner wants to merge 4 commits into
SeleniumHQ:trunkfrom
titusfortner:grid-se-remote-url

Conversation

@titusfortner

@titusfortner titusfortner commented Jul 16, 2026

Copy link
Copy Markdown
Member

🔗 Related Issues

Fixes #17782
Fixes #17788

💥 What does this PR do?

When a Grid is reached through Docker port-mapping or a reverse proxy, the BiDi/CDP/VNC URLs it hands back (webSocketUrl, se:cdp, se:vnc) point at the Grid's own view of its address — e.g. a container-internal ws://172.20.0.2:4444/... — which the client cannot reach, and causes attempted connections to error.

The Grid currently addresses this with a grid-url configuration (--grid-url / SE_NODE_GRID_URL): when set to the externally-reachable address the Node uses it for the proxied URLs. Except:

  • Users have to know this configuration exists as we've seen. People appear to be editing the capabilities themselves.
  • this requires knowing the full URL, including port when the grid is started, which means it won't work for containers with dynamically mapped ports as @AB-xdev reported in [🐛 Bug]: Do not establish BiDiConnection on startup #17782

This PR has each bindiung advertise the url they actually reached the Grid on via a new se:remoteUrl capability, which the Node uses to build the proxied URLs. Additional connections to Docker then work with no extra configuration. Note that an explicitly configured grid-url setting still takes precedence.

🔧 Implementation Notes

  • The fix lives in the Node (LocalNode.resolvePublicGridUri), so one server-side change makes the returned URLs reachable for every client; each binding only adds the small producer that sends se:remoteUrl.
  • Precedence is decided in resolvePublicGridUri: a configured public Grid URL (grid-url / hub) is used as-is; otherwise se:remoteUrl; otherwise the Node's auto-detected address (previous behavior). So the capability only fills the gap grid-url leaves and never overrides operator intent.
  • se:remoteUrl is not sent for local driver services (only for remote sessions), and when it is absent the behavior is unchanged for older clients.
  • Chose a client-advertised address over server-side Host-header inference: the client is authoritative about its own reachable address, it is robust behind proxies, and it avoids changes to Grid routing.

💡 Additional Considerations

  • Credentials embedded in the connection URL (https://user:key@host) are preserved in the resolved URL, identical to how an operator-configured grid-url with credentials already behaves. This keeps BiDi/CDP auth working against a basic-auth-protected Grid, where the returned websocket URL must carry the credentials. Under HTTPS these are protected in transit; the only residual is at-rest logging, consistent with the Grid already logging full capabilities today. Reducing that would be a separate, Grid-wide log-redaction change rather than mangling the URL the auth path depends on.
  • Most service providers now recommend supplying credentials via capabilities and using ClientConfig to set local authentication and proxy behavior, in which case the connection URL carries no credentials and se:remoteUrl carries none.

🤖 AI assistance

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

🔄 Types of changes

  • Bug fix (backwards compatible)

@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 labels Jul 16, 2026
@qodo-code-review

qodo-code-review Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Grid: use client-advertised se:remoteUrl for reachable BiDi/CDP/VNC endpoints

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description 

• Advertise the client-reachable Grid URL via a new se:remoteUrl session capability.
• Prefer grid-url, else se:remoteUrl, else auto-detected address when rewriting proxied
 endpoints.
• Add cross-binding and server-side tests covering precedence and credential preservation.
Diagram 

graph TD
  A["Client bindings"] --> B["NEW_SESSION caps include se:remoteUrl"] --> C["Grid Node LocalNode"] --> D{"grid-url configured?"}
  D -->|"Yes"| E["Use grid-url"] --> I["Rewrite se:cdp/webSocketUrl/se:vnc"] --> J["Session response endpoints"]
  D -->|"No"| F{"se:remoteUrl valid?"}
  F -->|"Yes"| G["Use se:remoteUrl"] --> I
  F -->|"No"| H["Use auto gridUri"] --> I
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Infer public base URL from Host/X-Forwarded-* headers
  • ➕ No client-side changes required
  • ➕ Keeps capability payload unchanged
  • ➖ Proxy/header setups vary; easy to misconfigure and hard to validate
  • ➖ Requires Grid routing/proxy awareness and consistent forwarded-header handling across deployments
  • ➖ Less authoritative than the client’s actual reachable URL
2. Extend `grid-url` to support dynamic port mapping (templating/runtime discovery)
  • ➕ Keeps operator control centralized
  • ➕ Avoids passing connection URL in capabilities
  • ➖ Still requires users to discover/configure it correctly
  • ➖ Runtime discovery across Docker/proxy topologies is environment-specific and brittle
  • ➖ Doesn’t help when multiple public entrypoints exist
3. Always trust `se:remoteUrl` over `grid-url`
  • ➕ Maximizes client control; simplest rule
  • ➖ Violates operator intent and makes behavior harder to reason about in managed Grids
  • ➖ Can introduce security/observability concerns if clients spoof URLs

Recommendation: The chosen approach (client-advertised se:remoteUrl with strict precedence: configured grid-url first, then se:remoteUrl, then fallback) is the best trade-off. It fixes Docker/reverse-proxy reachability without new server-side proxy inference and without overriding explicit operator configuration; tests also lock in precedence and credential-preserving behavior.

Files changed (13) +245 / -12

Enhancement (3) +23 / -1
HttpCommandExecutor.csExpose remote server URI to callers +5/-0

Expose remote server URI to callers

• Adds an internal 'RemoteServerUri' accessor so higher-level session creation can reliably retrieve the exact address used for remote connections.

dotnet/src/webdriver/Remote/HttpCommandExecutor.cs

RemoteWebDriver.javaSend se:remoteUrl from ClientConfig baseUri on session start +11/-0

Send se:remoteUrl from ClientConfig baseUri on session start

• Adds 'se:remoteUrl' to requested capabilities when 'ClientConfig.baseUri()' is set, ensuring the server can return client-reachable proxied endpoints.

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

RemoteWebDriverBuilder.javaInclude se:remoteUrl in alwaysMatch payload +7/-1

Include se:remoteUrl in alwaysMatch payload

• Copies 'additionalCapabilities' into a new alwaysMatch map and appends 'se:remoteUrl' using the builder base URI before building the W3C new-session payload.

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

Bug fix (5) +50 / -7
WebDriver.csAdvertise se:remoteUrl when starting remote sessions +5/-0

Advertise se:remoteUrl when starting remote sessions

• When using 'Remote.HttpCommandExecutor', injects 'se:remoteUrl' into the W3C capabilities payload based on the executor’s remote server address.

dotnet/src/webdriver/WebDriver.cs

LocalNode.javaRewrite BiDi/CDP/VNC URLs using resolved public Grid URI +29/-7

Rewrite BiDi/CDP/VNC URLs using resolved public Grid URI

• Refactors URL rewriting to accept a chosen base URI and introduces 'resolvePublicGridUri' to apply precedence: configured 'grid-url' > client 'se:remoteUrl' > prior auto-detected address. Preserves scheme (ws/wss), path prefix, port, and userinfo when constructing proxied endpoints.

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

index.jsAdd se:remoteUrl to capabilities for remote Builder URL strings +5/-0

Add se:remoteUrl to capabilities for remote Builder URL strings

• When the remote server URL is a string, sets 'se:remoteUrl' on the outgoing capabilities before creating the HTTP client/executor.

javascript/selenium-webdriver/index.js

webdriver.pyAdvertise se:remoteUrl for remote (non-service) sessions +10/-0

Advertise se:remoteUrl for remote (non-service) sessions

• Injects 'se:remoteUrl' into capabilities when the driver is connecting to a remote server (and not using a local 'service'). Adds a helper to retrieve the remote address from the command executor client config.

py/selenium/webdriver/remote/webdriver.py

driver.rbAdvertise se:remoteUrl from resolved server_url +1/-0

Advertise se:remoteUrl from resolved server_url

• Sets 'caps['se:remoteUrl']' to the HTTP client’s resolved 'server_url', ensuring remote sessions carry the client-reachable Grid URL.

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

Tests (5) +172 / -4
NodeTest.javaTest se:remoteUrl precedence and credential preservation +130/-0

Test se:remoteUrl precedence and credential preservation

• Adds coverage verifying 'se:remoteUrl' is used for proxied endpoints when provided, ignored when 'grid-url' is configured, and that userinfo (credentials) is preserved in rewritten URLs.

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

RemoteWebDriverBuilderTest.javaVerify builder advertises se:remoteUrl +18/-0

Verify builder advertises se:remoteUrl

• Adds a unit test asserting the builder includes 'se:remoteUrl' in the capabilities sent to the server.

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

RemoteWebDriverUnitTest.javaVerify RemoteWebDriver advertises se:remoteUrl +14/-0

Verify RemoteWebDriver advertises se:remoteUrl

• Adds a unit test ensuring 'RemoteWebDriver' includes 'se:remoteUrl' derived from 'ClientConfig.baseUri()' during session creation.

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

new_session_tests.pyUpdate new-session test to tolerate se:remoteUrl injection +6/-2

Update new-session test to tolerate se:remoteUrl injection

• Adjusts the capability assertions to remove 'se:remoteUrl' before comparing, keeping existing expectations stable while allowing the new capability.

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

driver_spec.rbUpdate remote driver spec to expect se:remoteUrl +4/-2

Update remote driver spec to expect se:remoteUrl

• Modifies request stubbing to automatically include 'se:remoteUrl' in the outgoing alwaysMatch payload, matching the new driver behavior.

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

@qodo-code-review

qodo-code-review Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. gridUrlSpecified() missing Javadoc ✗ Dismissed 📘 Rule violation ✧ Quality
Description 
The new public method LocalNode.Builder.gridUrlSpecified(boolean configured) lacks a Javadoc block
with @param/@return, violating the requirement for complete Javadoc on public API methods. This
reduces API clarity and makes the builder contract harder to maintain.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1577-1580]

+    public Builder gridUrlSpecified(boolean configured) {
+      this.gridUrlSpecified = configured;
+      return this;
+    }
Evidence 
PR Compliance ID 330201 requires complete Javadoc for changed/added public methods. The added public
builder method gridUrlSpecified(boolean configured) has no Javadoc directly above it.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1577-1580]

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 new public method was added without the required complete Javadoc (description + `@param` + `@return`).

## Issue Context
`LocalNode.Builder.gridUrlSpecified(boolean configured)` is public and should be documented per the project’s Javadoc requirements.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1577-1580]

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


Remediation recommended

2. JS se:remoteUrl lacks test 📘 Rule violation ▣ Testability ⭐ New
Description 
The JavaScript binding now injects se:remoteUrl into capabilities when building a remote session,
but this PR does not add a regression test to verify the payload. Without coverage, the behavior
could silently regress and reintroduce unreachable BiDi/CDP/VNC URLs for JS clients.
Code

javascript/selenium-webdriver/index.js[R654-656]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence 
Rule 389273 requires automated tests for bug fixes/new functionality. The PR introduces new behavior
in the JS builder by setting se:remoteUrl from the remote server URL, but no corresponding JS test
change is present to validate this behavior.

Rule 389273: Require tests for all new functionality and bug fixes
javascript/selenium-webdriver/index.js[651-656]
javascript/selenium-webdriver/test/builder_test.js[1-110]

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 JS binding injects `se:remoteUrl` for remote sessions, but no test asserts that the new-session capabilities include the expected value.

## Issue Context
This is user-visible behavior for remote/Grid usage. A lightweight unit test should ensure the remote server URL string is propagated into capabilities as `se:remoteUrl`.

## Fix Focus Areas
- javascript/selenium-webdriver/index.js[651-660]
- javascript/selenium-webdriver/test/builder_test.js[1-110]

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

3. .NET se:remoteUrl lacks test 📘 Rule violation ▣ Testability ⭐ New
Description 
The .NET binding now injects se:remoteUrl into the W3C capabilities it sends when creating a
session, but this PR does not add a regression test to confirm the request payload. This makes it
easy for future refactors to break the behavior without detection.
Code

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

+            if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
+            {
+                matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
+            }
Evidence 
Rule 389273 requires tests for bug fixes/new functionality. The PR changes how the .NET binding
constructs the new-session request (injecting se:remoteUrl derived from
HttpCommandExecutor.RemoteServerUri), but no .NET test is added/updated here to assert that
behavior.

Rule 389273: Require tests for all new functionality and bug fixes
dotnet/src/webdriver/WebDriver.cs[603-610]
dotnet/src/webdriver/Remote/HttpCommandExecutor.cs[114-118]
dotnet/test/remote/RemoteSessionCreationTests.cs[1-135]

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 .NET binding adds `se:remoteUrl` to `alwaysMatch` capabilities for remote sessions, but there is no automated test asserting this field is present/accurate.

## Issue Context
This is a behavioral change intended to fix reachable BiDi/CDP/VNC URLs when behind proxies/Docker port-mapping. A test should validate `se:remoteUrl` is sent when `CommandExecutor` is an `HttpCommandExecutor`.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[603-619]
- dotnet/src/webdriver/Remote/HttpCommandExecutor.cs[114-118]
- dotnet/test/remote/RemoteSessionCreationTests.cs[1-135]

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

4. RemoteSessionSettings omits remoteUrl ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description 
In .NET WebDriver.StartSession, se:remoteUrl is injected only for non-RemoteSessionSettings
capabilities; when callers use RemoteSessionSettings, the payload is sent via
remoteSettings.ToDictionary() with no injection. This makes se:remoteUrl inconsistent across
.NET session creation APIs and can break reachable BiDi/CDP/VNC URL construction behind proxies/NAT
unless users manually add the metadata setting.
Code

dotnet/src/webdriver/WebDriver.cs[R604-614]

        {
            Dictionary<string, object> matchCapabilities = this.GetCapabilitiesDictionary(capabilities);

+            if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
+            {
+                matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
+            }
+
            List<object> firstMatchCapabilitiesList = new List<object>();
            firstMatchCapabilitiesList.Add(matchCapabilities);
Evidence 
The injection is performed only inside the capabilities is not RemoteSessionSettings branch; the
RemoteSessionSettings branch sends remoteSettings.ToDictionary() directly.
RemoteSessionSettings explicitly supports metadata settings and includes them when producing its
dictionary, so the missing injection is an observable omission for that API path.

dotnet/src/webdriver/WebDriver.cs[588-623]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[111-134]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[225-251]

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 added only in the non-`RemoteSessionSettings` branch. When `RemoteSessionSettings` is used, the request bypasses the injection and omits `se:remoteUrl`.

### Issue Context
`RemoteSessionSettings` supports adding metadata settings via `AddMetadataSetting` and serializes them in `ToDictionary()`. The injection should be applied in this branch too (ideally only if the user hasn’t already supplied `se:remoteUrl`).

### Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[603-623]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[111-134]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[225-251]

### Suggested change
In the `else` branch:
- if `CommandExecutor` is `Remote.HttpCommandExecutor` and `remoteSettings["se:remoteUrl"]` is null (or `GetCapability("se:remoteUrl")` is null), call `remoteSettings.AddMetadataSetting("se:remoteUrl", httpExecutor.RemoteServerUri.AbsoluteUri)` before `ToDictionary()`.
- avoid overwriting an explicitly supplied `se:remoteUrl`.

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

View more (7)
5. Promise URL skips remoteUrl 🐞 Bug ≡ Correctness ⭐ New
Description 
In JS Builder, se:remoteUrl is only set when the server address is a string, so sessions created
with a Promise-backed URL (e.g., SELENIUM_SERVER_JAR) or a URL object won’t advertise
se:remoteUrl. In proxied/Docker Grid setups this makes Nodes fall back to their own (often
unreachable) address when building returned BiDi/CDP/VNC URLs.
Code

javascript/selenium-webdriver/index.js[R654-656]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence 
startSeleniumServer returns a Promise, and Builder.build() assigns that Promise to url when
SELENIUM_SERVER_JAR is set. The new typeof url === 'string' guard therefore skips setting
se:remoteUrl exactly in the async URL case, even though the executor later resolves the URL via
Promise.resolve(url).

javascript/selenium-webdriver/index.js[57-68]
javascript/selenium-webdriver/index.js[639-660]

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 only added when `url` is a string, but `url` can be a Promise (from `startSeleniumServer`) or potentially a `URL` object. In those cases the capability is never sent.

### Issue Context
`Builder.build()` already uses `Promise.resolve(url)` to construct the HTTP client, so you can reuse that same resolved value to set `se:remoteUrl` deterministically.

### Fix Focus Areas
- javascript/selenium-webdriver/index.js[651-660]

### Suggested change
Create a `resolvedUrl` promise (stringified) and:
- in the `.then(...)` handler, set `capabilities.set('se:remoteUrl', resolvedString)`
- use the same promise to build the `HttpClient`
This ensures the capability is set for string, Promise, and URL inputs without racing session creation.

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

6. LocalNode constructor signature changed ✗ Dismissed 📘 Rule violation ≡ Correctness
Description 
The protected LocalNode(...) constructor now requires an additional boolean gridUrlSpecified
parameter, which is a potentially breaking ABI/API change for any external subclasses calling
super(...). Consider adding a backward-compatible overload delegating to the new constructor to
preserve compatibility.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R176-180]

      EventBus bus,
      URI uri,
      URI gridUri,
+      boolean gridUrlSpecified,
      @Nullable HealthCheck healthCheck,
Evidence 
PR Compliance ID 389266 requires maintaining backward-compatible public API/ABI surfaces. The
constructor signature was modified by adding the gridUrlSpecified parameter, which changes the
callable signature for subclasses.

Rule 389266: Maintain backward-compatible public API and ABI
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[174-205]

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 new required parameter was added to a `protected` constructor, which can break downstream subclasses compiled against the previous signature.

## Issue Context
Even though call sites in-repo were updated, `protected` constructors are part of the subclassing surface and can impact external users.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[174-205]

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

7. RemoteUrl scheme not validated ✓ Resolved 🐞 Bug ≡ Correctness
Description 
LocalNode.resolvePublicGridUri will accept any se:remoteUrl URI that merely has a non-null host,
even if it’s not an absolute http/https URL. This can produce incorrect ws/wss URL construction (and
can break path handling) when the accepted URI is later consumed by rewrite().
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1303-1321]

+  // A configured grid-url always wins; only when the node falls back to its auto-detected address
+  // (which may be unreachable behind Docker/proxy) do we use the client-advertised se:remoteUrl.
+  private URI resolvePublicGridUri(Capabilities caps) {
+    if (gridUrlSpecified) {
+      return gridUri;
+    }
+    Object raw = caps.getCapability("se:remoteUrl");
+    if (raw instanceof String && !((String) raw).isEmpty()) {
+      try {
+        URI uri = new URI((String) raw);
+        if (uri.getHost() != null) {
+          return uri;
+        }
+      } catch (URISyntaxException e) {
+        // Fall back to gridUri.
+      }
+    }
+    return gridUri;
+  }
Evidence 
The new resolver returns any URI with a host, but rewrite() assumes an http/https-like base and
uses its scheme/path to generate websocket URLs. Separately, NodeOptions.normalizeSubPath calls
trim() on its input, which will throw if a null path ever reaches it.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1292-1321]
java/src/org/openqa/selenium/grid/node/config/NodeOptions.java[171-184]

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

### Issue description
`resolvePublicGridUri` currently accepts any `se:remoteUrl` string that parses as a `URI` with a non-null host. Because this URI is later used as the base for websocket URL generation, accepting scheme-less or non-http(s) URIs can lead to incorrect ws/wss selection and/or invalid path normalization.

### Issue Context
Downstream, `rewrite()` derives websocket scheme from `baseUri.getScheme()` and normalizes `baseUri.getPath()`.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1303-1321]

### Suggested fix
- In `resolvePublicGridUri`, require an absolute URI with an explicit scheme of `http` or `https` (and a non-null host) before returning it.
- Optionally harden `rewrite()` to treat a null path as empty (defensive coding), but the key is to avoid returning invalid bases from `resolvePublicGridUri`.

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

8. gridUrlSpecified footgun 🐞 Bug ⚙ Maintainability
Description 
LocalNode.Builder defaults gridUrlSpecified to false and requires callers to set it separately
from the gridUri they pass. Any caller that supplies a configured/public gridUri but forgets to
set gridUrlSpecified(true) will silently allow client se:remoteUrl to override what the caller
intended to be authoritative.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1577-1581]

+    public Builder gridUrlSpecified(boolean configured) {
+      this.gridUrlSpecified = configured;
+      return this;
+    }
+
Evidence 
The builder stores gridUri and gridUrlSpecified independently, with the flag defaulting to
false, so callers can accidentally create contradictory state. There are existing call sites that
pass a publicUri distinct from the node URI without setting the new flag, demonstrating the ease
of omission.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1476-1505]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1577-1603]
java/test/org/openqa/selenium/grid/graphql/GraphqlHandlerTest.java[255-305]

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 precedence decision in `resolvePublicGridUri` depends on `gridUrlSpecified`, but the builder API lets callers pass a `gridUri` while leaving `gridUrlSpecified` at its default `false`. This creates an easy-to-miss misconfiguration where an explicitly supplied public grid URI can be overridden by client-provided `se:remoteUrl`.

### Issue Context
`LocalNodeFactory` sets the flag correctly, but other builder call sites (tests/extensions) can pass a non-default `gridUri` without remembering to also set the boolean.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1485-1505]
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1577-1588]

### Suggested fix
Implement one of:
1) Make `gridUrlSpecified` derive from constructor inputs by default (e.g., initialize it in `Builder` based on whether `gridUri` differs from `uri`), while still allowing explicit override for the "equal URIs but configured" edge case.
2) Change the builder API to make the configured-vs-auto-detected nature explicit (e.g., introduce a dedicated `publicGridUri(URI configuredUri)` setter that both stores the URI and marks it configured, instead of a separate boolean).

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

9. RemoteUrl added for local 🐞 Bug ☼ Reliability
Description 
Java RemoteWebDriver injects se:remoteUrl into every new-session request when
clientConfig.baseUri() is non-null, and DriverCommandExecutor sets baseUri to the local
DriverService URL. As a result, local drivers like ChromeDriver will send se:remoteUrl
pointing at the local driver service, and any user-provided se:remoteUrl capability is
overwritten.
Code

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

+  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;
+  }
+
  protected void startSession(Capabilities capabilities) {
    checkNonW3CCapabilities(capabilities);
    checkChromeW3CFalse(capabilities);
+    capabilities = addRemoteUrl(capabilities);
Evidence 
RemoteWebDriver adds se:remoteUrl whenever clientConfig.baseUri() is set.
DriverCommandExecutor explicitly sets clientConfig.baseUri(service.getUrl().toURI()), and local
drivers (e.g., Chrome) use DriverCommandExecutor subclasses and inherit
RemoteWebDriver.startSession, so local driver sessions will carry this capability.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[263-277]
java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java[84-102]
java/src/org/openqa/selenium/chromium/ChromiumDriverCommandExecutor.java[35-50]
java/src/org/openqa/selenium/chrome/ChromeDriver.java[90-97]
java/src/org/openqa/selenium/chromium/ChromiumDriver.java[68-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
`RemoteWebDriver.startSession` now unconditionally calls `addRemoteUrl`, which injects `se:remoteUrl` whenever `clientConfig.baseUri()` is present. Local driver implementations use `DriverCommandExecutor`, which populates `clientConfig.baseUri()` with the local `DriverService` URL, so `se:remoteUrl` is sent to local driver services too.

### Issue Context
`se:remoteUrl` is intended to describe how the *client reached the Grid*, not how Selenium reached a local `DriverService`. Injecting it for local-driver sessions provides a misleading value and overwrites any explicit user-supplied `se:remoteUrl`.

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

### Implementation sketch
- In `addRemoteUrl(...)` (or before calling it), detect local driver-service sessions via `executor instanceof org.openqa.selenium.remote.service.DriverCommandExecutor` (and possibly known subclasses) and return capabilities unchanged.
- Keep current behavior for true remote sessions created with `HttpCommandExecutor`/`TracedCommandExecutor`.
- Consider adding a unit test that builds a `RemoteWebDriver` with a `DriverCommandExecutor` and asserts the outgoing NEW_SESSION payload does not contain `se:remoteUrl`.

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

10. Grid-url precedence misdetected ✓ Resolved 🐞 Bug ≡ Correctness
Description 
LocalNode.resolvePublicGridUri decides whether a public grid-url is configured by checking
!gridUri.equals(externalUri), so an explicitly configured grid-url that happens to equal
externalUri will be treated as “not configured” and can be overridden by client se:remoteUrl.
This violates the intended precedence and can yield incorrect proxied BiDi/CDP/VNC URLs even though
an operator configured grid-url.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1300-1318]

+  // A configured grid-url always wins; only when the node falls back to its auto-detected address
+  // (which may be unreachable behind Docker/proxy) do we use the client-advertised se:remoteUrl.
+  private URI resolvePublicGridUri(Capabilities caps) {
+    if (!gridUri.equals(externalUri)) {
+      return gridUri;
+    }
+    Object raw = caps.getCapability("se:remoteUrl");
+    if (raw instanceof String && !((String) raw).isEmpty()) {
+      try {
+        URI uri = new URI((String) raw);
+        if (uri.getHost() != null) {
+          return uri;
+        }
+      } catch (URISyntaxException e) {
+        // Fall back to gridUri.
+      }
+    }
+    return gridUri;
+  }
Evidence 
The Node currently has enough information at construction time to know whether gridUri was
explicitly configured (node.grid-url/node.hub) or just defaulted, but resolvePublicGridUri
instead uses gridUri.equals(externalUri) as a proxy for “configured”. If the configured value
equals externalUri, the method will incorrectly fall through to client se:remoteUrl.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1289-1318]
java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java[65-72]
java/src/org/openqa/selenium/grid/node/config/NodeOptions.java[108-123]

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

### Issue description
`LocalNode.resolvePublicGridUri` infers whether `gridUri` came from an explicit operator configuration by comparing `gridUri` to `externalUri`. This loses provenance information when the configured `grid-url` equals `externalUri`.

### Issue Context
`LocalNodeFactory` already knows whether `gridUri` came from `nodeOptions.getPublicGridUri()` (configured) or from the fallback `serverOptions.getExternalUri()`. That boolean should be passed into `LocalNode` instead of reconstructing intent from URI equality.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1300-1318]
- java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java[65-72]

### Implementation sketch
- Change `LocalNode.builder(...)` / `LocalNode` ctor to accept a boolean like `isPublicGridUriConfigured` (or accept `Optional<URI> configuredGridUri`).
- In `resolvePublicGridUri`, use that flag/optional to decide whether to always return the configured value.
- Add a regression test where configured `gridUri` equals `externalUri` and assert client `se:remoteUrl` does **not** override it.

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

11. new_session_tests patches WebDriver.execute 📘 Rule violation ▣ Testability
Description 
The updated tests mock remote WebDriver session/HTTP interactions using pytest-mock (patching
WebDriver.execute) and Ruby WebMock (stub_request) rather than exercising a real integration or
a contract-driven stub. This reduces test fidelity and violates the requirement to avoid mocks for
external/service integrations unless they are backed by a machine-checked contract.
Code

py/test/unit/selenium/webdriver/remote/new_session_tests.py[R37-42]

+    command, params = mock.call_args[0]
+    assert command == Command.NEW_SESSION
+    always_match = params["capabilities"]["alwaysMatch"]
+    always_match.pop("se:remoteUrl", None)
+    assert params["capabilities"]["firstMatch"] == [{}]
+    assert always_match == w3c_caps
Evidence 
PR Compliance ID 389270 disallows using mocking frameworks for external/service interactions unless
the stub is contract-driven. The Python unit test patches
selenium.webdriver.remote.webdriver.WebDriver.execute and inspects mock.call_args, demonstrating
the remote interaction is simulated rather than validated against a real or contract-backed
endpoint, and the Ruby helper similarly configures stub_request(:post, endpoint), which is an
HTTP-level mock of the remote session creation endpoint rather than a real/contract-verified
integration.

Rule 389270: Avoid mocks in tests; use real or contract-driven integrations
py/test/unit/selenium/webdriver/remote/new_session_tests.py[30-42]
rb/spec/unit/selenium/webdriver/remote/driver_spec.rb[32-37]

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 tests currently rely on mocking frameworks (`pytest-mock` patching `selenium.webdriver.remote.webdriver.WebDriver.execute` and Ruby WebMock `stub_request`) to simulate remote WebDriver session/HTTP interactions. Update these tests to avoid hand-written mocks for external/service integrations unless the stubs are backed by a machine-checked contract, and instead use a real integration approach or a contract-driven stub.

## Issue Context
Compliance (PR Compliance ID 389270) requires avoiding mocks for external API/service interactions unless they are backed by a contract that ensures the stub matches the real API. The current changes simulate the remote new-session flow by patching `WebDriver.execute`/asserting on `mock.call_args` in Python and by stubbing HTTP POST requests via `stub_request` in Ruby, which reduces fidelity and is not contract-driven.

## Fix Focus Areas
- py/test/unit/selenium/webdriver/remote/new_session_tests.py[30-42]
- rb/spec/unit/selenium/webdriver/remote/driver_spec.rb[32-37]

ⓘ 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

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 16, 2026
View reviewed changes
Comment thread py/test/unit/selenium/webdriver/remote/new_session_tests.py
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 17, 2026
View reviewed changes
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
@qodo-code-review

qodo-code-review Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 9a20e5d

@qodo-code-review

qodo-code-review Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner requested review from Copilot and diemol July 17, 2026 01:53
Copilot AI reviewed Jul 17, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes Selenium Grid return BiDi/CDP/VNC endpoints (webSocketUrl, se:cdp, se:vnc) that are reachable from the client when Grid is accessed via Docker port-mapping or reverse proxies, by having clients advertise the URL they actually used to reach the Grid (se:remoteUrl) and teaching the Node to prefer it when rewriting proxied URLs (unless an operator-configured grid-url is set).

Changes:

  • Add se:remoteUrl advertising in all bindings (Java, JS, Python, Ruby, .NET) when starting remote sessions.
  • Update Grid Node URL rewriting to use se:remoteUrl when grid-url is not explicitly configured, while preserving credentials and keeping grid-url precedence.
  • Add/adjust unit tests (notably Java Node tests and binding-level unit tests) to cover precedence and credential preservation.

Reviewed changes

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

Show a summary per file
File Description
rb/spec/unit/selenium/webdriver/remote/driver_spec.rb Updates request expectations to include se:remoteUrl in new-session payloads.
rb/lib/selenium/webdriver/remote/driver.rb Ruby Remote driver now advertises se:remoteUrl based on the configured server URL.
py/test/unit/selenium/webdriver/remote/new_session_tests.py Adjusts assertions to ignore the new se:remoteUrl capability when comparing payloads.
py/selenium/webdriver/remote/webdriver.py Python Remote WebDriver adds se:remoteUrl for remote (non-service) sessions.
javascript/selenium-webdriver/index.js JS Builder advertises se:remoteUrl when creating remote sessions.
java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java Adds tests for advertising se:remoteUrl in Java RemoteWebDriver (remote vs local).
java/test/org/openqa/selenium/remote/RemoteWebDriverBuilderTest.java Adds test ensuring builder-advertised se:remoteUrl is sent to the server.
java/test/org/openqa/selenium/grid/node/NodeTest.java Adds regression tests for Node URL rewriting using se:remoteUrl, precedence, and credential preservation.
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java Injects se:remoteUrl into alwaysMatch when a base URI is configured.
java/src/org/openqa/selenium/remote/RemoteWebDriver.java Adds se:remoteUrl to capabilities when the RemoteWebDriver has a non-null base URI.
java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java Plumbs whether grid-url was explicitly specified into LocalNode construction.
java/src/org/openqa/selenium/grid/node/local/LocalNode.java Rewrites BiDi/CDP/VNC URLs using resolved “public Grid URI” with precedence rules.
dotnet/src/webdriver/WebDriver.cs .NET WebDriver adds se:remoteUrl when using HttpCommandExecutor (remote sessions).
dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Exposes the remote server URI internally to support se:remoteUrl advertising.

Comment thread javascript/selenium-webdriver/index.js
qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 17, 2026
View reviewed changes
Comment thread javascript/selenium-webdriver/index.js
Comment thread dotnet/src/webdriver/WebDriver.cs
Comment thread javascript/selenium-webdriver/index.js
Comment thread dotnet/src/webdriver/WebDriver.cs
@qodo-code-review

qodo-code-review Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

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

@qodo-code-review

qodo-code-review Bot commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. se:remoteUrl removal lacks test 📘 Rule violation ☼ Reliability ⭐ New
Description 
The Node now removes se:remoteUrl from returned session capabilities, but no focused test verifies
this new behavior. A regression could expose embedded credentials through capabilities,
session-created events, or logs.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1229-1232]

+    // se:remoteUrl is transport-only: it is consumed above to resolve the public URI, so drop it
+    // from the returned capabilities rather than echo it (and any embedded credentials) back to the
+    // client, into session-created events, or into the session-created log line.
+    toUse = removeCapability(toUse, "se:remoteUrl");
Evidence 
PR Compliance ID 5 requires focused regression coverage for behavioral changes. LocalNode now
explicitly removes se:remoteUrl, while the related Node tests only assert the rewritten VNC URL
and do not verify removal from returned capabilities.

AGENTS.md: Provide Focused Tests for Behavioral Changes
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1229-1232]
java/test/org/openqa/selenium/grid/node/NodeTest.java[212-250]

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

## Issue description
Add focused regression coverage proving that `se:remoteUrl` is consumed for URL resolution but omitted from the resulting session capabilities.

## Issue Context
The capability may contain embedded credentials, and the new removal behavior is intended to prevent it from being echoed into session state, events, and logs. Existing Node tests verify the rewritten VNC URL but do not assert that `se:remoteUrl` is absent.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1229-1232]
- java/test/org/openqa/selenium/grid/node/NodeTest.java[212-250]

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

2. anyMatch suppresses remote URL 📘 Rule violation ≡ Correctness
Description 
Because hasExplicitRemoteUrl() treats se:remoteUrl in any firstMatch candidate as global, one
explicitly configured alternative suppresses automatic advertisement for every other independent
candidate. If Grid selects a candidate without that capability, proxied BiDi/CDP/VNC URLs can fall
back to a potentially unreachable Node address, causing inconsistent remote behavior.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[R531-532]

+        || requestedCapabilities.stream()
+            .anyMatch(caps -> caps.getCapabilityNames().contains("se:remoteUrl"));
Evidence 
Compliance rule 4 requires consistent user-visible remote behavior. oneOf and addAlternative
store independent entries in requestedCapabilities, and getPayload() sends them as separate
firstMatch alternatives that NewSessionPayload expands independently; therefore, the aggregate
anyMatch check lets one candidate with an explicit se:remoteUrl suppress injection for unrelated
candidates, leaving them without the reachable address required by Grid.

AGENTS.md: Maintain Cross-Binding Consistency for User-Visible Behavior
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[529-547]
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[109-138]
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[529-548]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[270-300]

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 single explicit `se:remoteUrl` in one `firstMatch` candidate currently causes `hasExplicitRemoteUrl()` to suppress automatic injection for all alternatives. Preserve explicitly supplied values while adding the detected base URI to every selectable alternative that lacks `se:remoteUrl`.

## Issue Context

`requestedCapabilities` contains independent alternatives, and Grid may select any resulting `firstMatch` entry. Ensure every selectable remote-session capability set contains `se:remoteUrl` without overwriting explicit values or creating an `alwaysMatch`/`firstMatch` overlap.

## Fix Focus Areas

- java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[529-548]
- java/test/org/openqa/selenium/remote/RemoteWebDriverBuilderTest.java[183-199]

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

3. Standard Java sessions omit URL ✓ Resolved 🐞 Bug ≡ Correctness
Description 
addRemoteUrl reads the driver's stored clientConfig, but URL-based constructors configure only a
separate immutable copy for the executor while retaining a default config whose baseUri is null.
Ordinary new RemoteWebDriver(URL, capabilities) sessions therefore omit se:remoteUrl, leaving
Grid to use an address that may be unreachable in proxied or containerized deployments.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R264-266]

+    URI baseUri = clientConfig.baseUri();
+    if (baseUri == null) {
+      return capabilities;
Evidence 
The standard URL constructor starts with ClientConfig.defaultConfig() and passes the unchanged
object into the driver, while createExecutor assigns baseUrl(remoteAddress) only to a local
copy. Since the default configuration has a null base URI and ClientConfig.baseUri returns a new
instance, the added null branch skips advertisement on this common remote-session path.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[173-246]
java/src/org/openqa/selenium/remote/http/ClientConfig.java[89-140]

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

## Issue description
Standard URL-based `RemoteWebDriver` constructors do not advertise `se:remoteUrl` because the field consulted by `addRemoteUrl` retains the original null-base configuration.

## Issue Context
`ClientConfig` is immutable. `createExecutor` creates a configured copy for HTTP transport, but the constructor stores the unchanged original configuration. Preserve the intentional omission for local `DriverService` sessions.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[173-246]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[263-276]
- java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java[77-100]

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

View more (4)
4. se:remoteUrl overlaps firstMatch ✓ Resolved 📘 Rule violation ≡ Correctness
Description 
The builder unconditionally adds se:remoteUrl to alwaysMatch, so options that already contain
this capability create an overlapping firstMatch key and fail payload validation. This breaks
previously valid builder inputs instead of normalizing or replacing the client-advertised value.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[535]

+      alwaysMatch.put("se:remoteUrl", baseUri.toString());
Evidence 
Rule 1 requires existing public API behavior to remain compatible. The changed builder inserts
se:remoteUrl into alwaysMatch while leaving requested capabilities in firstMatch;
NewSessionPayload explicitly rejects any overlap between those maps, so a caller-provided
se:remoteUrl now causes build() to fail.

AGENTS.md: Maintain Public API and ABI Compatibility
java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[532-542]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[125-133]

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

## Issue description
Prevent the automatically advertised `se:remoteUrl` from overlapping with the same capability in any `firstMatch` entry.

## Issue Context
`NewSessionPayload` rejects keys present in both `alwaysMatch` and `firstMatch`. Normalize the payload while retaining the executor-derived URL as the authoritative advertised address, and add a regression test using options that already contain `se:remoteUrl`.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java[532-542]
- java/test/org/openqa/selenium/remote/RemoteWebDriverBuilderTest.java[183-200]

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

5. firstMatch remote URL conflicts ✓ Resolved 📘 Rule violation ≡ Correctness
Description 
The new alwaysMatch injection only checks top-level metadata, so it can overwrite a
caller-provided se:remoteUrl in must-match DriverOptions or duplicate one from
RemoteSessionSettings.firstMatch. The latter creates a W3C capability conflict that can turn a
previously valid request into a rejected session creation, contrary to the stated intent to preserve
explicit values and existing behavior.
Code

dotnet/src/webdriver/WebDriver.cs[R642-643]

+                alwaysMatch["se:remoteUrl"] = remoteHttpExecutor.RemoteServerUri.AbsoluteUri;
+                remoteSettingsDictionary["alwaysMatch"] = alwaysMatch;
Evidence 
Rule 1 requires existing session APIs and behavior to remain compatible, but
RemoteSessionSettings.HasCapability checks only remoteMetadataSettings, while must-match and
first-match DriverOptions are serialized separately into alwaysMatch and firstMatch. Because
DriverOptions.AddAdditionalOption allows callers to define se:remoteUrl, unconditionally writing
the executor URL into alwaysMatch after this shallow check can overwrite an existing must-match
value or leave the same key in a first-match entry; Grid validation explicitly rejects capability
keys that overlap between alwaysMatch and firstMatch.

AGENTS.md: Maintain Public API and ABI Compatibility
dotnet/src/webdriver/WebDriver.cs[632-643]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[189-197]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[229-250]
dotnet/src/webdriver/WebDriver.cs[629-643]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[189-248]
dotnet/src/webdriver/DriverOptions.cs[195-213]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[125-133]

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

## Issue description

Prevent automatic `se:remoteUrl` injection from overwriting a caller-provided value in must-match `DriverOptions` or duplicating the capability in any `firstMatch` entry, since duplicate capability names across the W3C match sets invalidate the new-session payload.

## Issue Context

`RemoteSessionSettings.HasCapability("se:remoteUrl")` only examines top-level metadata and does not inspect capabilities serialized from must-match or first-match `DriverOptions`. Before injecting the executor URL, inspect the serialized `alwaysMatch` and every `firstMatch` entry, preserve caller-provided values, and ensure the resulting payload never contains the same capability in both match sections.

## Fix Focus Areas

- dotnet/src/webdriver/WebDriver.cs[629-643]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[189-248]

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

6. remoteUrl outside capability matches ✓ Resolved 📘 Rule violation ≡ Correctness
Description 
The RemoteSessionSettings path adds se:remoteUrl beside alwaysMatch and firstMatch rather
than inside a matched capability set, so Grid does not forward it to LocalNode as a session
capability. Consequently, .NET sessions created through this path can still receive unreachable
BiDi/CDP/VNC URLs in proxied or containerized deployments.
Code

dotnet/src/webdriver/WebDriver.cs[630]

+                remoteSettingsDictionary["se:remoteUrl"] = remoteHttpExecutor.RemoteServerUri.AbsoluteUri;
Evidence 
RemoteSessionSettings.ToDictionary() emits an envelope containing metadata plus alwaysMatch and
firstMatch, and StartSession passes that dictionary directly under the W3C request's
capabilities key; however, line 630 adds se:remoteUrl as a sibling of the matching sections.
Grid constructs desired session capabilities only by merging alwaysMatch and firstMatch, then
SessionRequest forwards those resulting capabilities and LocalNode resolves the public URL from
them, so the sibling value is unavailable; by contrast, the ordinary .NET path places the value in
firstMatch, as required for consistent shared protocol serialization.

AGENTS.md: Maintain Cross-Binding Consistency for User-Visible Behavior
dotnet/src/webdriver/WebDriver.cs[622-633]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[229-250]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[214-221]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[270-300]
java/src/org/openqa/selenium/grid/data/SessionRequest.java[63-69]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1216-1227]

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 `RemoteSessionSettings` path adds `se:remoteUrl` at the top level of the W3C `capabilities` envelope beside `alwaysMatch` and `firstMatch`. Move it into a valid matched capability set so Grid forwards it to `LocalNode`.

## Issue Context

`RemoteSessionSettings.ToDictionary()` returns the contents of the `capabilities` envelope, including the `alwaysMatch`/`firstMatch` structure. Ensure the advertised URL is included in the effective matched capabilities while preserving caller ownership and caller-provided values, and avoid introducing overlap between `alwaysMatch` and `firstMatch`.

## Fix Focus Areas

- dotnet/src/webdriver/WebDriver.cs[622-633]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[229-250]

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

7. gridUrlSpecified() missing Javadoc ✗ Dismissed 📘 Rule violation ✧ Quality
Description 
The new public method LocalNode.Builder.gridUrlSpecified(boolean configured) lacks a Javadoc block
with @param/@return, violating the requirement for complete Javadoc on public API methods. This
reduces API clarity and makes the builder contract harder to maintain.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1577-1580]

+    public Builder gridUrlSpecified(boolean configured) {
+      this.gridUrlSpecified = configured;
+      return this;
+    }
Evidence 
PR Compliance ID 330201 requires complete Javadoc for changed/added public methods. The added public
builder method gridUrlSpecified(boolean configured) has no Javadoc directly above it.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1577-1580]

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 new public method was added without the required complete Javadoc (description + `@param` + `@return`).

## Issue Context
`LocalNode.Builder.gridUrlSpecified(boolean configured)` is public and should be documented per the project’s Javadoc requirements.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1577-1580]

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


Remediation recommended

8. Mixed alternatives omit URL 🐞 Bug ≡ Correctness ⭐ New
Description 
ContainsMatchCapability suppresses automatic injection when any firstMatch candidate defines
se:remoteUrl, leaving every other candidate without the executor URL. If Grid selects one of those
candidates behind a proxy or NAT, LocalNode falls back to its potentially unreachable internal
gridUri for BiDi/CDP/VNC URLs.
Code

dotnet/src/webdriver/WebDriver.cs[R682-685]

+                if (candidate is IDictionary<string, object> firstMatchCapabilities
+                    && firstMatchCapabilities.ContainsKey(capabilityName))
+                {
+                    return true;
Evidence 
RemoteSessionSettings supports and serializes multiple firstMatch alternatives, while the new helper
returns true after finding the capability in only one candidate and consequently skips global
injection. Grid expands each firstMatch candidate independently, and LocalNode falls back to gridUri
when the selected candidate lacks se:remoteUrl.

dotnet/src/webdriver/WebDriver.cs[633-647]
dotnet/src/webdriver/WebDriver.cs[677-687]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[141-154]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[243-247]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[276-297]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1322-1345]

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 any `firstMatch` candidate contains `se:remoteUrl`, `ContainsMatchCapability` suppresses injection for the entire payload. Other candidates can therefore be selected without a client-reachable URL.

## Issue Context
Preserve explicitly supplied values, but add the executor URL to each `firstMatch` candidate that lacks one. Avoid creating an `alwaysMatch`/`firstMatch` overlap, and add coverage for mixed alternatives.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[622-650]
- dotnet/src/webdriver/WebDriver.cs[668-692]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[141-154]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[243-247]

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

9. Mixed-case HTTPS downgrades websocket ✓ Resolved 🐞 Bug ≡ Correctness
Description 
resolvePublicGridUri accepts HTTP(S) schemes case-insensitively, but rewrite recognizes HTTPS
case-sensitively. An accepted URL such as HTTPS://grid.example is consequently rewritten as
ws:// instead of wss://, causing insecure or failed BiDi, CDP, and VNC connections.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1294]

+      String scheme = "https".equals(baseUri.getScheme()) ? "wss" : "ws";
Evidence 
The resolver accepts https through equalsIgnoreCase, allowing mixed-case schemes to reach
rewrite; rewrite then selects wss only when the scheme exactly equals lowercase https.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1292-1297]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1305-1318]

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

## Issue description
Mixed-case HTTPS remote URLs pass validation but are rewritten using the insecure `ws` scheme.

## Issue Context
URI schemes are case-insensitive. Validation already uses `equalsIgnoreCase`, so rewriting must apply the same semantics.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1292-1297]
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1305-1318]
- java/test/org/openqa/selenium/grid/node/NodeTest.java[212-250]

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

10. Reused settings retain stale URL ✓ Resolved 🐞 Bug ≡ Correctness
Description 
StartSession permanently adds the executor URL to the caller-owned RemoteSessionSettings, and
HasCapability prevents recomputing it when that settings instance is reused. A subsequent driver
targeting another Grid therefore advertises the first Grid URL, causing returned BiDi/CDP/VNC URLs
to target the wrong endpoint.
Code

dotnet/src/webdriver/WebDriver.cs[R623-625]

+                && !remoteSettings.HasCapability("se:remoteUrl"))
+            {
+                remoteSettings.AddMetadataSetting("se:remoteUrl", remoteHttpExecutor.RemoteServerUri.AbsoluteUri);
Evidence 
The new branch writes into the supplied remoteSettings only when the key is absent.
RemoteSessionSettings.AddMetadataSetting stores values in an instance dictionary, while
HasCapability continues returning true for stored metadata, so the generated URL survives and
blocks recomputation on later use.

dotnet/src/webdriver/WebDriver.cs[622-628]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[36-36]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[121-134]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[189-197]

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

## Issue description
`WebDriver.StartSession` stores its automatically generated `se:remoteUrl` directly in the caller-owned `RemoteSessionSettings`. Reusing that settings instance with another executor retains the original URL because `HasCapability` suppresses subsequent injection.

## Issue Context
Preserve an explicitly caller-provided `se:remoteUrl`, but do not persist request-specific generated metadata on the reusable settings object. Build a per-request dictionary or copy before injecting the executor URL, and add coverage for reuse across two different remote endpoints.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[622-628]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[121-134]
- dotnet/test/remote/RemoteSessionCreationTests.cs[76-135]

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

View more (13)
11. Ruby http_client NoMethodError ✓ Resolved 🐞 Bug ≡ Correctness
Description 
Ruby Remote::Driver now unconditionally reads http_client.client_config.server_url when setting
se:remoteUrl. If callers provide a custom http_client (supported by Remote::Bridge) that doesn’t
expose client_config, driver construction will raise NoMethodError before a session is created.
Code

rb/lib/selenium/webdriver/remote/driver.rb[41]

+          caps['se:remoteUrl'] = http_client.client_config.server_url.to_s
Evidence 
The new code path dereferences http_client.client_config unconditionally, while the Remote::Bridge
contract explicitly allows any HTTP client implementing the Default protocol; such clients are not
guaranteed to expose client_config, so initialization can fail with NoMethodError.

rb/lib/selenium/webdriver/remote/driver.rb[34-47]
rb/lib/selenium/webdriver/remote/bridge.rb[53-60]

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

### Issue description
`Remote::Driver#initialize` sets `caps['se:remoteUrl']` by dereferencing `http_client.client_config.server_url`. This assumes every caller-supplied HTTP client exposes `client_config`, but the public contract allows custom clients implementing the `Http::Default` protocol (not necessarily inheriting from `Http::Common`). This can raise `NoMethodError` at construction time.

### Issue Context
The code already has the authoritative remote URL available as `url || client_config&.server_url || default`, and it already assigns that into `http_client.server_url = ...`. The capability should be derived from that known value (or guarded behind `respond_to?(:client_config)`), rather than requiring `client_config` on the HTTP client.

### Fix Focus Areas
- rb/lib/selenium/webdriver/remote/driver.rb[34-47]
- rb/lib/selenium/webdriver/remote/bridge.rb[53-60]

### Suggested fix
1. Compute a `remote_url` local variable from `url || client_config&.server_url || default`.
2. Set both:
  - `http_client.server_url = remote_url`
  - `caps['se:remoteUrl'] = remote_url.to_s` (ensure trailing `/` if you want consistency with `ClientConfig#server_url` normalization)
3. Do **not** dereference `http_client.client_config` unless you first check `http_client.respond_to?(:client_config)`.
4. Add a regression test that passes a minimal custom http_client implementing `server_url=`/`call` but **not** `client_config`, and asserts initialization/session creation does not crash and that `se:remoteUrl` is present.

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

12. se:remoteUrl lacks assertion test 📘 Rule violation ☼ Reliability
Description 
Python WebDriver.start_session now injects se:remoteUrl, but the updated unit test explicitly
removes the capability instead of asserting it is present (and correctly omitted for local drivers).
This leaves the new behavior unprotected against regressions.
Code

py/test/unit/selenium/webdriver/remote/new_session_tests.py[R37-42]

+    command, params = mock.call_args[0]
+    assert command == Command.NEW_SESSION
+    always_match = params["capabilities"]["alwaysMatch"]
+    always_match.pop("se:remoteUrl", None)
+    assert params["capabilities"]["firstMatch"] == [{}]
+    assert always_match == w3c_caps
Evidence 
Rule 5 requires behavior changes to include corresponding tests. The production code adds
se:remoteUrl in start_session, while the only updated unit test removes se:remoteUrl from the
payload instead of asserting its expected presence/value, so the new behavior is not actually
covered.

AGENTS.md: Add or Update Tests; Prefer Small Unit Tests Over Browser Tests
py/selenium/webdriver/remote/webdriver.py[388-413]
py/test/unit/selenium/webdriver/remote/new_session_tests.py[30-43]

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 now adds `se:remoteUrl` during remote session creation, but there is no positive test asserting it is sent (and no test asserting it is not sent for local-driver/service sessions).

## Issue Context
The existing test was adjusted to `pop("se:remoteUrl", None)` from `alwaysMatch`, which prevents failures but does not validate the new capability injection behavior.

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

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

13. se:remoteUrl rejection unlogged ✓ Resolved 📘 Rule violation ◔ Observability
Description 
LocalNode.resolvePublicGridUri silently ignores invalid/unsupported se:remoteUrl values and
falls back to gridUri. This makes proxy/Docker reachability issues harder to diagnose because
users get no operational signal about why their advertised URL was ignored.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1305-1323]

+  private URI resolvePublicGridUri(Capabilities caps) {
+    if (gridUrlSpecified) {
+      return gridUri;
+    }
+    Object raw = caps.getCapability("se:remoteUrl");
+    if (raw instanceof String && !((String) raw).isEmpty()) {
+      try {
+        URI uri = new URI((String) raw);
+        String scheme = uri.getScheme();
+        if (uri.getHost() != null
+            && ("http".equalsIgnoreCase(scheme) || "https".equalsIgnoreCase(scheme))) {
+          return uri;
+        }
+      } catch (URISyntaxException e) {
+        // Fall back to gridUri.
+      }
+    }
+    return gridUri;
+  }
Evidence 
Rule 7 requires adding logging where users need operational insight. The new resolvePublicGridUri
behavior introduces multiple fallback paths (configured grid-url, accepted se:remoteUrl, or
fallback to gridUri) but provides no log message when se:remoteUrl is rejected, leaving users
without actionable diagnostics.

AGENTS.md: Add Logging Where Users Need Operational Insight
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1303-1323]

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 `se:remoteUrl` is present but invalid (bad URI syntax, missing host, non-http(s) scheme), the Node falls back to `gridUri` without any logging, which reduces diagnosability.

## Issue Context
This PR introduces `se:remoteUrl` specifically to fix unreachable returned BiDi/CDP/VNC URLs behind proxies/NAT; silent fallback makes it difficult for operators/users to understand why the feature is not taking effect.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1303-1323]

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

14. JS se:remoteUrl lacks test 📘 Rule violation ▣ Testability
Description 
The JavaScript binding now injects se:remoteUrl into capabilities when building a remote session,
but this PR does not add a regression test to verify the payload. Without coverage, the behavior
could silently regress and reintroduce unreachable BiDi/CDP/VNC URLs for JS clients.
Code

javascript/selenium-webdriver/index.js[R654-656]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence 
Rule 389273 requires automated tests for bug fixes/new functionality. The PR introduces new behavior
in the JS builder by setting se:remoteUrl from the remote server URL, but no corresponding JS test
change is present to validate this behavior.

Rule 389273: Require tests for all new functionality and bug fixes
javascript/selenium-webdriver/index.js[651-656]
javascript/selenium-webdriver/test/builder_test.js[1-110]

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 JS binding injects `se:remoteUrl` for remote sessions, but no test asserts that the new-session capabilities include the expected value.

## Issue Context
This is user-visible behavior for remote/Grid usage. A lightweight unit test should ensure the remote server URL string is propagated into capabilities as `se:remoteUrl`.

## Fix Focus Areas
- javascript/selenium-webdriver/index.js[651-660]
- javascript/selenium-webdriver/test/builder_test.js[1-110]

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

15. .NET se:remoteUrl lacks test 📘 Rule violation ▣ Testability
Description 
The .NET binding now injects se:remoteUrl into the W3C capabilities it sends when creating a
session, but this PR does not add a regression test to confirm the request payload. This makes it
easy for future refactors to break the behavior without detection.
Code

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

+            if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
+            {
+                matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
+            }
Evidence 
Rule 389273 requires tests for bug fixes/new functionality. The PR changes how the .NET binding
constructs the new-session request (injecting se:remoteUrl derived from
HttpCommandExecutor.RemoteServerUri), but no .NET test is added/updated here to assert that
behavior.

Rule 389273: Require tests for all new functionality and bug fixes
dotnet/src/webdriver/WebDriver.cs[603-610]
dotnet/src/webdriver/Remote/HttpCommandExecutor.cs[114-118]
dotnet/test/remote/RemoteSessionCreationTests.cs[1-135]

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 .NET binding adds `se:remoteUrl` to `alwaysMatch` capabilities for remote sessions, but there is no automated test asserting this field is present/accurate.

## Issue Context
This is a behavioral change intended to fix reachable BiDi/CDP/VNC URLs when behind proxies/Docker port-mapping. A test should validate `se:remoteUrl` is sent when `CommandExecutor` is an `HttpCommandExecutor`.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[603-619]
- dotnet/src/webdriver/Remote/HttpCommandExecutor.cs[114-118]
- dotnet/test/remote/RemoteSessionCreationTests.cs[1-135]

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

16. RemoteSessionSettings omits remoteUrl ✓ Resolved 🐞 Bug ≡ Correctness
Description 
In .NET WebDriver.StartSession, se:remoteUrl is injected only for non-RemoteSessionSettings
capabilities; when callers use RemoteSessionSettings, the payload is sent via
remoteSettings.ToDictionary() with no injection. This makes se:remoteUrl inconsistent across
.NET session creation APIs and can break reachable BiDi/CDP/VNC URL construction behind proxies/NAT
unless users manually add the metadata setting.
Code

dotnet/src/webdriver/WebDriver.cs[R604-614]

        {
            Dictionary<string, object> matchCapabilities = this.GetCapabilitiesDictionary(capabilities);

+            if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
+            {
+                matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
+            }
+
            List<object> firstMatchCapabilitiesList = new List<object>();
            firstMatchCapabilitiesList.Add(matchCapabilities);
Evidence 
The injection is performed only inside the capabilities is not RemoteSessionSettings branch; the
RemoteSessionSettings branch sends remoteSettings.ToDictionary() directly.
RemoteSessionSettings explicitly supports metadata settings and includes them when producing its
dictionary, so the missing injection is an observable omission for that API path.

dotnet/src/webdriver/WebDriver.cs[588-623]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[111-134]
dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[225-251]

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 added only in the non-`RemoteSessionSettings` branch. When `RemoteSessionSettings` is used, the request bypasses the injection and omits `se:remoteUrl`.

### Issue Context
`RemoteSessionSettings` supports adding metadata settings via `AddMetadataSetting` and serializes them in `ToDictionary()`. The injection should be applied in this branch too (ideally only if the user hasn’t already supplied `se:remoteUrl`).

### Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[603-623]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[111-134]
- dotnet/src/webdriver/Remote/RemoteSessionSettings.cs[225-251]

### Suggested change
In the `else` branch:
- if `CommandExecutor` is `Remote.HttpCommandExecutor` and `remoteSettings["se:remoteUrl"]` is null (or `GetCapability("se:remoteUrl")` is null), call `remoteSettings.AddMetadataSetting("se:remoteUrl", httpExecutor.RemoteServerUri.AbsoluteUri)` before `ToDictionary()`.
- avoid overwriting an explicitly supplied `se:remoteUrl`.

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

17. Promise URL skips remoteUrl 🐞 Bug ≡ Correctness
Description 
In JS Builder, se:remoteUrl is only set when the server address is a string, so sessions created
with a Promise-backed URL (e.g., SELENIUM_SERVER_JAR) or a URL object won’t advertise
se:remoteUrl. In proxied/Docker Grid setups this makes Nodes fall back to their own (often
unreachable) address when building returned BiDi/CDP/VNC URLs.
Code

javascript/selenium-webdriver/index.js[R654-656]

+      if (typeof url === 'string') {
+        capabilities.set('se:remoteUrl', url)
+      }
Evidence 
startSeleniumServer returns a Promise, and Builder.build() assigns that Promise to url when
SELENIUM_SERVER_JAR is set. The new typeof url === 'string' guard therefore skips setting
se:remoteUrl exactly in the async URL case, even though the executor later resolves the URL via
Promise.resolve(url).

javascript/selenium-webdriver/index.js[57-68]
javascript/selenium-webdriver/index.js[639-660]

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 only added when `url` is a string, but `url` can be a Promise (from `startSeleniumServer`) or potentially a `URL` object. In those cases the capability is never sent.

### Issue Context
`Builder.build()` already uses `Promise.resolve(url)` to construct the HTTP client, so you can reuse that same resolved value to set `se:remoteUrl` deterministically.

### Fix Focus Areas
- javascript/selenium-webdriver/index.js[651-660]

### Suggested change
Create a `resolvedUrl` promise (stringified) and:
- in the `.then(...)` handler, set `capabilities.set('se:remoteUrl', resolvedString)`
- use the same promise to build the `HttpClient`
This ensures the capability is set for string, Promise, and URL inputs without racing session creation.

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

18. LocalNode constructor signature changed ✗ Dismissed 📘 Rule violation ≡ Correctness
Description 
The protected LocalNode(...) constructor now requires an additional boolean gridUrlSpecified
parameter, which is a potentially breaking ABI/API change for any external subclasses calling
super(...). Consider adding a backward-compatible overload delegating to the new constructor to
preserve compatibility.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R176-180]

      EventBus bus,
      URI uri,
      URI gridUri,
+      boolean gridUrlSpecified,
      @Nullable HealthCheck healthCheck,
Evidence 
PR Compliance ID 389266 requires maintaining backward-compatible public API/ABI surfaces. The
constructor signature was modified by adding the gridUrlSpecified parameter, which changes the
callable signature for subclasses.

Rule 389266: Maintain backward-compatible public API and ABI
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[174-205]

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 new required parameter was added to a `protected` constructor, which can break downstream subclasses compiled against the previous signature.

## Issue Context
Even though call sites in-repo were updated, `protected` constructors are part of the subclassing surface and can impact external users.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[174-205]

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

19. RemoteUrl scheme not validated ✓ Resolved 🐞 Bug ≡ Correctness
Description 
LocalNode.resolvePublicGridUri will accept any se:remoteUrl URI that merely has a non-null host,
even if it’s not an absolute http/https URL. This can produce incorrect ws/wss URL construction (and
can break path handling) when the accepted URI is later consumed by rewrite().
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1303-1321]

+  // A configured grid-url always wins; only when the node falls back to its auto-detected address
+  // (which may be unreachable behind Docker/proxy) do we use the client-advertised se:remoteUrl.
+  private URI resolvePublicGridUri(Capabilities caps) {
+    if (gridUrlSpecified) {
+      return gridUri;
+    }
+    Object raw = caps.getCapability("se:remoteUrl");
+    if (raw instanceof String && !((String) raw).isEmpty()) {
+      try {
+        URI uri = new URI((String) raw);
+        if (uri.getHost() != null) {
+          return uri;
+        }
+      } catch (URISyntaxException e) {
+        // Fall back to gridUri.
+      }
+    }
+    return gridUri;
+  }
Evidence 
The new resolver returns any URI with a host, but rewrite() assumes an http/https-like base and
uses its scheme/path to generate websocket URLs. Separately, NodeOptions.normalizeSubPath calls
trim() on its input, which will throw if a null path ever reaches it.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1292-1321]
java/src/org/openqa/selenium/grid/node/config/NodeOptions.java[171-184]

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

### Issue description
`resolvePublicGridUri` currently accepts any `se:remoteUrl` string that parses as a `URI` with a non-null host. Because this URI is later used as the base for websocket URL generation, accepting scheme-less or non-http(s) URIs can lead to incorrect ws/wss selection and/or invalid path normalization.

### Issue Context
Downstream, `rewrite()` derives websocket scheme from `baseUri.getScheme()` and normalizes `baseUri.getPath()`.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1303-1321]

### Suggested fix
- In `resolvePublicGridUri`, require an absolute URI with an explicit scheme of `http` or `https` (and a non-null host) before returning it.
- Optionally harden `rewrite()` to treat a null path as empty (defensive coding), but the key is to avoid returning invalid bases from `resolvePublicGridUri`.

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

20. gridUrlSpecified footgun 🐞 Bug ⚙ Maintainability
Description 
LocalNode.Builder defaults gridUrlSpecified to false and requires callers to set it separately
from the gridUri they pass. Any caller that supplies a configured/public gridUri but forgets to
set gridUrlSpecified(true) will silently allow client se:remoteUrl to override what the caller
intended to be authoritative.
Code

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[R1577-1581]

+    public Builder gridUrlSpecified(boolean configured) {
+      this.gridUrlSpecified = configured;
+      return this;
+    }
+
Evidence 
The builder stores gridUri and gridUrlSpecified independently, with the flag defaulting to
false, so callers can accidentally create contradictory state. There are existing call sites that
pass a publicUri distinct from the node URI without setting the new flag, demonstrating the ease
of omission.

java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1476-1505]
[java/src/org/ope

[Comment truncated to fit github's 65,536-char limit.]

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 22, 2026
View reviewed changes
Comment thread py/test/unit/selenium/webdriver/remote/new_session_tests.py
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Comment thread rb/lib/selenium/webdriver/remote/driver.rb Outdated
qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 28, 2026
View reviewed changes
Comment thread dotnet/src/webdriver/WebDriver.cs Outdated
@qodo-code-review

qodo-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 47e3b18

@titusfortner
titusfortner requested a review from Copilot July 28, 2026 16:37
Copilot AI reviewed Jul 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 28, 2026
View reviewed changes
Comment thread dotnet/src/webdriver/WebDriver.cs Outdated
@qodo-code-review

qodo-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 52195cf

@qodo-code-review

qodo-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 5db3310

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 28, 2026
View reviewed changes
Comment thread dotnet/src/webdriver/WebDriver.cs
@qodo-code-review

qodo-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 17503eb

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 28, 2026
View reviewed changes
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java Outdated
@qodo-code-review

qodo-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 56d5343

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 28, 2026
View reviewed changes
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java
@qodo-code-review

qodo-code-review Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

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

qodo-code-review[bot]
qodo-code-review Bot reviewed Jul 29, 2026
View reviewed changes
Comment thread java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Comment thread dotnet/src/webdriver/WebDriver.cs
@qodo-code-review

qodo-code-review Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 780f91f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qodo-code-review qodo-code-review[bot] qodo-code-review[bot] left review comments
Copilot code review Copilot Copilot left review comments
@diemol diemol diemol approved these changes

Assignees

No one assigned

Labels

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Do not establish BiDiConnection on startup [🐛 Bug]: BiDi URL is not translated

4 participants

@titusfortner @diemol @selenium-ci