Skip to content

[js] Ensure BiDi is not exposed on Driver - #17926

Merged
pujagani merged 14 commits into
SeleniumHQ:trunkfrom
pujagani:bidi-access-js
Aug 18, 2026
Merged

[js] Ensure BiDi is not exposed on Driver#17926
pujagani merged 14 commits into
SeleniumHQ:trunkfrom
pujagani:bidi-access-js

Conversation

@pujagani

Copy link
Copy Markdown
Contributor

🔗 Related Issues

Related to #17814

💥 What does this PR do?

Deprecate old methods, creates relevant methods and classes for creating BiDi connection.

🔧 Implementation Notes

🤖 AI assistance

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

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • New feature (non-breaking change which adds functionality and tests!)

@selenium-ci selenium-ci added the C-nodejs JavaScript Bindings label Aug 18, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Externalize and deprecate JavaScript BiDi driver access

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Moves per-driver BiDi transport state into an internal WeakMap-backed connection registry.
• Deprecates raw WebDriver#getBidi access while preserving backward compatibility.
• Prevents concurrent connection races and safely handles shutdown after failed initialization.
Diagram

sequenceDiagram
  participant Module as BiDi Module
  participant Driver as WebDriver
  participant Registry as Connection Registry
  participant Caps as Capabilities
  participant Transport as BiDi Transport
  Module->>Driver: Request connection
  Driver->>Registry: Get shared promise
  Registry->>Driver: Read capabilities
  Driver-->>Registry: WebSocket URL
  Registry->>Transport: Create once
  Transport-->>Module: Shared connection
  Driver->>Registry: Close on quit
  Registry->>Transport: Close if opened
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Migrate composed modules directly
  • ➕ Avoids deprecation warnings when users follow the recommended composed-module API.
  • ➕ Completes the intended boundary by reserving WebDriver#getBidi for legacy callers only.
  • ➖ Requires coordinated changes across every existing JavaScript BiDi module.
  • ➖ Expands the PR scope and regression surface.
2. Store connection on a private driver field
  • ➕ Keeps connection lifecycle logic localized within WebDriver.
  • ➕ Avoids a module-level registry.
  • ➖ Continues coupling raw BiDi transport state to WebDriver.
  • ➖ Does not align as clearly with the accepted composition boundary.
  • ➖ Requires careful handling of concurrent initialization.

Recommendation: Keep the WeakMap-backed registry and shared in-flight promise; they provide appropriate ownership, garbage-collection behavior, and concurrency safety. However, composed BiDi modules should call the internal registry directly—either here or in an immediate follow-up—so supported usage does not trigger the deprecated WebDriver accessor.

Files changed (3) +198 / -18

Enhancement (1) +82 / -0
bidi_connection.jsAdd an internal per-driver BiDi connection registry +82/-0

Add an internal per-driver BiDi connection registry

• Introduces lazy BiDi transport creation backed by a WeakMap of in-flight promises, ensuring concurrent callers share one connection. Adds capability validation and non-rejecting cleanup that avoids creating unopened connections.

javascript/selenium-webdriver/lib/bidi_connection.js

Refactor (1) +20 / -18
webdriver.jsExternalize BiDi state and deprecate raw driver access +20/-18

Externalize BiDi state and deprecate raw driver access

• Removes BiDi connection state and creation logic from the WebDriver class, delegating creation and shutdown to the internal registry. Retains 'getBidi()' as a deprecated prototype compatibility shim directing users toward composed BiDi modules.

javascript/selenium-webdriver/lib/webdriver.js

Tests (1) +96 / -0
bidi_connection_test.jsTest BiDi connection concurrency and safe cleanup +96/-0

Test BiDi connection concurrency and safe cleanup

• Adds WebSocket-backed tests proving concurrent first access creates exactly one connection. Verifies cleanup is harmless when initialization never occurred or previously failed.

javascript/selenium-webdriver/test/lib/bidi_connection_test.js

@qodo-code-review

qodo-code-review Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Deprecations vanish by default ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New
Description
Logger#deprecate() records an ID before attempting to log, but Selenium's default root level is
OFF, so the first getBidi() warning is discarded while becoming permanently suppressed. Enabling
warning logging or adding a handler afterward cannot surface the notice on later calls.
Code

javascript/selenium-webdriver/lib/logging.js[R416-417]

+    this.deprecated_.add(id)
+    this.warning(`[${id}] ${message}`)
Evidence
The global LogManager initializes its root logger at Level.OFF, and Logger.log() immediately
drops non-loggable entries. The new method nevertheless adds the ID before calling warning(),
while logger instances are cached and reused; therefore later calls remain suppressed even if
logging is subsequently enabled. The added tests avoid this behavior by explicitly setting the
logger to WARNING.

javascript/selenium-webdriver/lib/logging.js[356-368]
javascript/selenium-webdriver/lib/logging.js[462-467]
javascript/selenium-webdriver/lib/logging.js[497-504]
javascript/selenium-webdriver/lib/webdriver.js[1799-1805]
javascript/selenium-webdriver/test/lib/logging_test.js[219-230]

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

## Issue description
`Logger#deprecate()` marks a deprecation as reported even when the warning is discarded by the default `OFF` logging configuration. Ensure a deprecation remains observable under normal defaults and is not permanently claimed before actual emission.

## Issue Context
The global root logger starts at `Level.OFF`, while `WebDriver#getBidi()` uses the shared logger's new deprecation method. The current tests explicitly enable `WARNING`, so they do not cover default or delayed logging configuration.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/logging.js[398-419]
- javascript/selenium-webdriver/lib/webdriver.js[1799-1806]
- javascript/selenium-webdriver/test/lib/logging_test.js[217-262]

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


2. Replacement emits deprecation warning ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Every composed BiDi module still calls driver.getBidi(), so wrapping that method with
util.deprecate makes the recommended replacement APIs emit the warning intended only for direct
legacy access. This affects both handwritten modules such as Network and every generated module
created by the BiDi generator.
Code

javascript/selenium-webdriver/lib/webdriver.js[R1790-1792]

+WebDriver.prototype.getBidi = util.deprecate(function () {
+  return getBidiConnection(this)
+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
Evidence
The changed method is wrapped by util.deprecate, while Network initialization invokes that exact
driver method. The generator emits the same invocation for every generated module, even though
Network.create(driver) is explicitly named as the replacement.

javascript/selenium-webdriver/lib/webdriver.js[1778-1792]
javascript/selenium-webdriver/bidi/network.js[93-95]
javascript/selenium-webdriver/generate_bidi.mjs[1013-1021]
javascript/selenium-webdriver/test/bidi/generated/network_test.js[23-36]

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

## Issue description
Composed BiDi modules still obtain their transport through the newly deprecated `WebDriver#getBidi()`. Consequently, using the APIs recommended by the deprecation message itself emits a deprecation warning.

Update internal handwritten and generated BiDi modules to call the shared connection helper directly, while retaining `getBidi()` only as the deprecated compatibility entry point.

## Issue Context
`getBidiConnection(driver)` is now the internal connection owner. Both existing module initialization and generated `create(driver)` methods must use it without traversing the deprecated driver member.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1790-1792]
- javascript/selenium-webdriver/lib/bidi_connection.js[38-43]
- javascript/selenium-webdriver/bidi/network.js[93-95]
- javascript/selenium-webdriver/generate_bidi.mjs[1013-1021]

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



Remediation recommended

3. getBidi warning untested 📘 Rule violation ▣ Testability ⭐ New
Description
The new getBidi() deprecation call is not exercised by the added tests, which only test
Logger.deprecate() directly. Removing the warning from getBidi() would therefore leave the new
test suite passing.
Code

javascript/selenium-webdriver/lib/webdriver.js[R1800-1803]

+  WebDriver.logger.deprecate(
+    'webdriver-getBidi',
+    'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' +
+      "require('selenium-webdriver/bidi/network').",
Evidence
Rule 389273 requires tests that exercise changed behavior and fail if it is reverted. The
implementation adds the warning to getBidi(), while the added tests invoke only
log.deprecate(...) and never call WebDriver#getBidi().

Rule 389273: Require tests for all new functionality and bug fixes
javascript/selenium-webdriver/lib/webdriver.js[1799-1805]
javascript/selenium-webdriver/test/lib/logging_test.js[217-262]

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

## Issue description
No automated test verifies that calling `WebDriver#getBidi()` emits the new warning with replacement guidance.

## Issue Context
The logging tests validate only the generic `Logger.deprecate()` helper, not its integration into the deprecated public API. Add a test that would fail if the call at this integration point were removed.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1799-1805]
- javascript/selenium-webdriver/test/lib/logging_test.js[217-262]

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


4. WebDriver.logger namespace nonstandard ✓ Resolved 📘 Rule violation ◔ Observability ⭐ New
Description
The new logger uses webdriver.WebDriver, which does not follow the required
selenium.webdriver.<modulename> namespace for the webdriver module. This prevents standardized
logger filtering and identification.
Code

javascript/selenium-webdriver/lib/webdriver.js[701]

+    return logging.getLogger('webdriver.WebDriver')
Evidence
The code correctly uses the project logging module, but its literal logger name is
webdriver.WebDriver rather than the namespace required by rule 389259.

Rule 389259: Use project logging module with standardized Selenium logger namespace
javascript/selenium-webdriver/lib/webdriver.js[700-702]

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.logger` obtains a project logger with the nonstandard name `webdriver.WebDriver`.

## Issue Context
Compliance rule 389259 requires a literal `selenium.webdriver.<modulename>` namespace; this file's module name is `webdriver`.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[700-702]

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


5. logger return description missing ✓ Resolved 📘 Rule violation ✧ Quality ⭐ New
Description
The new public static WebDriver.logger accessor has a typed @return tag but no description of
the returned logger.
Code

javascript/selenium-webdriver/lib/webdriver.js[698]

+   * @return {!./logging.Logger}
Evidence
Rule 389257 requires a typed return tag with a brief description. The accessor's `@return
{!./logging.Logger}` supplies only the type.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/webdriver.js[694-702]

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 JSDoc for the public `WebDriver.logger` accessor omits descriptive text from its `@return` tag.

## Issue Context
Compliance rule 389257 requires every non-void public function or method return tag to include both a type and a brief description.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[694-702]

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


View medium (8)
6. deprecate omits @throws ✓ Resolved 📘 Rule violation ✧ Quality ⭐ New
Description
The public Logger.deprecate() method synchronously throws TypeError for an empty identifier, but
its JSDoc has no @throws tag describing that condition.
Code

javascript/selenium-webdriver/lib/logging.js[R410-411]

+    if (!id) {
+      throw new TypeError('Logger#deprecate() requires a non-empty id')
Evidence
Rule 389257 requires public methods to document synchronous errors with typed @throws tags. The
new method throws TypeError at lines 410–411, while its immediately preceding JSDoc only contains
@param tags.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/logging.js[398-412]

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

## Issue description
`Logger.deprecate()` can synchronously throw a `TypeError`, but its public API JSDoc does not document that exception.

## Issue Context
Compliance rule 389257 requires a typed `@throws` tag with a description for every synchronous throw path.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/logging.js[398-412]

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


7. Deprecation assignment exceeds formatter width ✓ Resolved 📘 Rule violation ✧ Quality
Description
The new util.deprecate assignment is written as a single line substantially longer than the
configured 120-character Prettier width and would be reformatted by the enforced formatter.
Code

javascript/selenium-webdriver/lib/webdriver.js[1792]

+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
Evidence
PR Compliance ID 389272 treats code that would clearly be corrected by the configured formatter as a
violation. The package's .prettierrc and ESLint rule enforce printWidth: 120, whereas the added
line 1792 substantially exceeds that width.

Rule 389272: Run code formatter via scripts/format.sh or configured git hooks before pushing
javascript/selenium-webdriver/lib/webdriver.js[1790-1792]
javascript/selenium-webdriver/.prettierrc[1-7]
javascript/selenium-webdriver/eslint.config.js[95-103]

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 `util.deprecate` assignment does not conform to the JavaScript package's configured Prettier line width.

## Issue Context
Run the configured formatter or manually apply its wrapping while preserving the warning message.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1790-1792]

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


8. closeBidiConnection JSDoc incomplete ✓ Resolved 📘 Rule violation ✧ Quality
Description
The exported closeBidiConnection function documents neither the driver parameter nor its return
value with the required descriptive text.
Code

javascript/selenium-webdriver/lib/bidi_connection.js[R65-66]

+ * @param {object} driver
+ * @returns {Promise<void>}
Evidence
PR Compliance ID 389257 requires complete JSDoc for functions exported through module.exports. The
tags at lines 65-66 omit descriptions, while line 82 exports the function.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/bidi_connection.js[58-68]
javascript/selenium-webdriver/lib/bidi_connection.js[82-82]

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 exported `closeBidiConnection` function has incomplete `@param` and `@returns` tags.

## Issue Context
Each tag must include its type, matching parameter name where applicable, and a brief description.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/bidi_connection.js[58-68]

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


9. BiDi close comment restates call ✓ Resolved 📘 Rule violation ✧ Quality
Description
The new comment merely restates that the immediately following closeBidiConnection call closes an
opened BiDi connection, without explaining a non-obvious rationale or constraint.
Code

javascript/selenium-webdriver/lib/webdriver.js[798]

+      // Close the BiDi websocket connection, if one was ever opened
Evidence
PR Compliance ID 389275 requires comments to explain rationale rather than translate immediate code
behavior. The comment at line 798 describes exactly what the clearly named call on line 799 does.

Rule 389275: Prefer rationale-focused comments over restating code behavior
javascript/selenium-webdriver/lib/webdriver.js[798-799]

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 comment above `closeBidiConnection(this)` repeats behavior already conveyed by the function name.

## Issue Context
Remove the comment or replace it with rationale that is not apparent from the code, such as why the close remains fire-and-forget if that distinction is necessary.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[798-799]

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


10. getBidiConnection JSDoc incomplete ✓ Resolved 📘 Rule violation ✧ Quality
Description
The exported getBidiConnection function documents neither the driver parameter nor its return
value with the required descriptive text.
Code

javascript/selenium-webdriver/lib/bidi_connection.js[R35-36]

+ * @param {object} driver
+ * @returns {Promise<BiDi>}
Evidence
PR Compliance ID 389257 requires complete JSDoc for functions exported through module.exports. The
tags at lines 35-36 contain types and a parameter name but no descriptions, while line 82 exports
the function.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/bidi_connection.js[33-38]
javascript/selenium-webdriver/lib/bidi_connection.js[82-82]

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 exported `getBidiConnection` function has incomplete `@param` and `@returns` tags.

## Issue Context
Each tag must include its type, matching parameter name where applicable, and a brief description.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/bidi_connection.js[33-38]

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


11. quit() delegation lacks regression test ✓ Resolved 📘 Rule violation ▣ Testability
Description
The tests invoke closeBidiConnection directly but never verify that WebDriver.quit() delegates
to it, so reverting the new shutdown integration would not fail the added suite. This leaves the
connection-leak fix without the required regression coverage.
Code

javascript/selenium-webdriver/lib/webdriver.js[799]

+      closeBidiConnection(this)
Evidence
PR Compliance ID 389273 requires an assertion that fails when changed behavior is reverted. The
production change delegates shutdown to closeBidiConnection(this), but the added test suite only
calls the helper directly and therefore does not cover the quit() integration.

Rule 389273: Require tests for all new functionality and bug fixes
javascript/selenium-webdriver/lib/webdriver.js[798-799]
javascript/selenium-webdriver/test/lib/bidi_connection_test.js[39-95]

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

## Issue description
No regression test verifies that `WebDriver.quit()` closes a BiDi connection now stored outside the driver instance.

## Issue Context
The existing tests call `closeBidiConnection` directly. Add a test exercising `quit()` whose assertion would fail if the delegation at line 799 were reverted or removed.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[798-799]
- javascript/selenium-webdriver/test/lib/bidi_connection_test.js[39-95]

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


12. getBidi return description missing ✓ Resolved 📘 Rule violation ✧ Quality
Description
The public WebDriver#getBidi JSDoc declares a return type but omits the required description of
the returned promise.
Code

javascript/selenium-webdriver/lib/webdriver.js[1788]

+ * @returns {Promise<import('../bidi')>}
Evidence
PR Compliance ID 389257 requires an exported method's non-void @returns tag to include both a type
and a brief description. The new public prototype method provides only
{Promise<import('../bidi')>}.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/webdriver.js[1778-1790]

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 `WebDriver#getBidi` JSDoc `@returns` tag lacks descriptive text.

## Issue Context
The return tag must retain its type and briefly describe the promise and connection it resolves to.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1778-1790]

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


13. Accessor becomes enumerable ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
Assigning getBidi directly to WebDriver.prototype creates an enumerable property, unlike its
previous class-method definition. It now appears during for...in traversal of driver instances and
becomes more discoverable despite this change's stated implementation boundary.
Code

javascript/selenium-webdriver/lib/webdriver.js[R1790-1792]

+WebDriver.prototype.getBidi = util.deprecate(function () {
+  return getBidiConnection(this)
+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
Evidence
The PR replaces a non-enumerable class method with direct prototype assignment, whose resulting
property is enumerable. The referenced architectural decision specifically identifies
discoverability from the driver as the boundary this work is intended to eliminate.

javascript/selenium-webdriver/lib/webdriver.js[660-671]
javascript/selenium-webdriver/lib/webdriver.js[1778-1792]
docs/decisions/17670-bidi-implementation-boundaries.md[26-40]

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

## Issue description
Moving `getBidi` from the class body to direct prototype assignment changes its property descriptor from non-enumerable to enumerable. Preserve the normal class-method descriptor while applying the deprecation wrapper, for example with `Object.defineProperty`.

## Issue Context
The accepted implementation-boundary decision says low-level BiDi access should not be discoverable as a driver member. The deprecated compatibility method may remain temporarily, but it should not become more visible than it was before.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1790-1792]
- docs/decisions/17670-bidi-implementation-boundaries.md[26-40]

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


Grey Divider

Context
✅ Compliance rules (platform): 14 rules

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous review results

Review updated until commit e78fa56

Results up to commit 6288b65 ⚖️ Balanced


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


Action required
1. Replacement emits deprecation warning ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Every composed BiDi module still calls driver.getBidi(), so wrapping that method with
util.deprecate makes the recommended replacement APIs emit the warning intended only for direct
legacy access. This affects both handwritten modules such as Network and every generated module
created by the BiDi generator.
Code

javascript/selenium-webdriver/lib/webdriver.js[R1790-1792]

+WebDriver.prototype.getBidi = util.deprecate(function () {
+  return getBidiConnection(this)
+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
Evidence
The changed method is wrapped by util.deprecate, while Network initialization invokes that exact
driver method. The generator emits the same invocation for every generated module, even though
Network.create(driver) is explicitly named as the replacement.

javascript/selenium-webdriver/lib/webdriver.js[1778-1792]
javascript/selenium-webdriver/bidi/network.js[93-95]
javascript/selenium-webdriver/generate_bidi.mjs[1013-1021]
javascript/selenium-webdriver/test/bidi/generated/network_test.js[23-36]

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

## Issue description
Composed BiDi modules still obtain their transport through the newly deprecated `WebDriver#getBidi()`. Consequently, using the APIs recommended by the deprecation message itself emits a deprecation warning.

Update internal handwritten and generated BiDi modules to call the shared connection helper directly, while retaining `getBidi()` only as the deprecated compatibility entry point.

## Issue Context
`getBidiConnection(driver)` is now the internal connection owner. Both existing module initialization and generated `create(driver)` methods must use it without traversing the deprecated driver member.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1790-1792]
- javascript/selenium-webdriver/lib/bidi_connection.js[38-43]
- javascript/selenium-webdriver/bidi/network.js[93-95]
- javascript/selenium-webdriver/generate_bidi.mjs[1013-1021]

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



Remediation recommended
2. getBidi return description missing ✓ Resolved 📘 Rule violation ✧ Quality
Description
The public WebDriver#getBidi JSDoc declares a return type but omits the required description of
the returned promise.
Code

javascript/selenium-webdriver/lib/webdriver.js[1788]

+ * @returns {Promise<import('../bidi')>}
Evidence
PR Compliance ID 389257 requires an exported method's non-void @returns tag to include both a type
and a brief description. The new public prototype method provides only
{Promise<import('../bidi')>}.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/webdriver.js[1778-1790]

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 `WebDriver#getBidi` JSDoc `@returns` tag lacks descriptive text.

## Issue Context
The return tag must retain its type and briefly describe the promise and connection it resolves to.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1778-1790]

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


3. closeBidiConnection JSDoc incomplete ✓ Resolved 📘 Rule violation ✧ Quality
Description
The exported closeBidiConnection function documents neither the driver parameter nor its return
value with the required descriptive text.
Code

javascript/selenium-webdriver/lib/bidi_connection.js[R65-66]

+ * @param {object} driver
+ * @returns {Promise<void>}
Evidence
PR Compliance ID 389257 requires complete JSDoc for functions exported through module.exports. The
tags at lines 65-66 omit descriptions, while line 82 exports the function.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/bidi_connection.js[58-68]
javascript/selenium-webdriver/lib/bidi_connection.js[82-82]

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 exported `closeBidiConnection` function has incomplete `@param` and `@returns` tags.

## Issue Context
Each tag must include its type, matching parameter name where applicable, and a brief description.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/bidi_connection.js[58-68]

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


4. BiDi close comment restates call ✓ Resolved 📘 Rule violation ✧ Quality
Description
The new comment merely restates that the immediately following closeBidiConnection call closes an
opened BiDi connection, without explaining a non-obvious rationale or constraint.
Code

javascript/selenium-webdriver/lib/webdriver.js[798]

+      // Close the BiDi websocket connection, if one was ever opened
Evidence
PR Compliance ID 389275 requires comments to explain rationale rather than translate immediate code
behavior. The comment at line 798 describes exactly what the clearly named call on line 799 does.

Rule 389275: Prefer rationale-focused comments over restating code behavior
javascript/selenium-webdriver/lib/webdriver.js[798-799]

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 comment above `closeBidiConnection(this)` repeats behavior already conveyed by the function name.

## Issue Context
Remove the comment or replace it with rationale that is not apparent from the code, such as why the close remains fire-and-forget if that distinction is necessary.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[798-799]

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


View medium (4)
5. Accessor becomes enumerable ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
Assigning getBidi directly to WebDriver.prototype creates an enumerable property, unlike its
previous class-method definition. It now appears during for...in traversal of driver instances and
becomes more discoverable despite this change's stated implementation boundary.
Code

javascript/selenium-webdriver/lib/webdriver.js[R1790-1792]

+WebDriver.prototype.getBidi = util.deprecate(function () {
+  return getBidiConnection(this)
+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
Evidence
The PR replaces a non-enumerable class method with direct prototype assignment, whose resulting
property is enumerable. The referenced architectural decision specifically identifies
discoverability from the driver as the boundary this work is intended to eliminate.

javascript/selenium-webdriver/lib/webdriver.js[660-671]
javascript/selenium-webdriver/lib/webdriver.js[1778-1792]
docs/decisions/17670-bidi-implementation-boundaries.md[26-40]

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

## Issue description
Moving `getBidi` from the class body to direct prototype assignment changes its property descriptor from non-enumerable to enumerable. Preserve the normal class-method descriptor while applying the deprecation wrapper, for example with `Object.defineProperty`.

## Issue Context
The accepted implementation-boundary decision says low-level BiDi access should not be discoverable as a driver member. The deprecated compatibility method may remain temporarily, but it should not become more visible than it was before.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1790-1792]
- docs/decisions/17670-bidi-implementation-boundaries.md[26-40]

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


6. getBidiConnection JSDoc incomplete ✓ Resolved 📘 Rule violation ✧ Quality
Description
The exported getBidiConnection function documents neither the driver parameter nor its return
value with the required descriptive text.
Code

javascript/selenium-webdriver/lib/bidi_connection.js[R35-36]

+ * @param {object} driver
+ * @returns {Promise<BiDi>}
Evidence
PR Compliance ID 389257 requires complete JSDoc for functions exported through module.exports. The
tags at lines 35-36 contain types and a parameter name but no descriptions, while line 82 exports
the function.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/lib/bidi_connection.js[33-38]
javascript/selenium-webdriver/lib/bidi_connection.js[82-82]

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 exported `getBidiConnection` function has incomplete `@param` and `@returns` tags.

## Issue Context
Each tag must include its type, matching parameter name where applicable, and a brief description.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/bidi_connection.js[33-38]

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


7. Deprecation assignment exceeds formatter width ✓ Resolved 📘 Rule violation ✧ Quality
Description
The new util.deprecate assignment is written as a single line substantially longer than the
configured 120-character Prettier width and would be reformatted by the enforced formatter.
Code

javascript/selenium-webdriver/lib/webdriver.js[1792]

+}, 'WebDriver#getBidi() is deprecated. Use a composed BiDi module instead, e.g. Network.create(driver) or ' + "require('selenium-webdriver/bidi/network'). See docs/decisions/17670-bidi-implementation-boundaries.md.")
Evidence
PR Compliance ID 389272 treats code that would clearly be corrected by the configured formatter as a
violation. The package's .prettierrc and ESLint rule enforce printWidth: 120, whereas the added
line 1792 substantially exceeds that width.

Rule 389272: Run code formatter via scripts/format.sh or configured git hooks before pushing
javascript/selenium-webdriver/lib/webdriver.js[1790-1792]
javascript/selenium-webdriver/.prettierrc[1-7]
javascript/selenium-webdriver/eslint.config.js[95-103]

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 `util.deprecate` assignment does not conform to the JavaScript package's configured Prettier line width.

## Issue Context
Run the configured formatter or manually apply its wrapping while preserving the warning message.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[1790-1792]

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


8. quit() delegation lacks regression test ✓ Resolved 📘 Rule violation ▣ Testability
Description
The tests invoke closeBidiConnection directly but never verify that WebDriver.quit() delegates
to it, so reverting the new shutdown integration would not fail the added suite. This leaves the
connection-leak fix without the required regression coverage.
Code

javascript/selenium-webdriver/lib/webdriver.js[799]

+      closeBidiConnection(this)
Evidence
PR Compliance ID 389273 requires an assertion that fails when changed behavior is reverted. The
production change delegates shutdown to closeBidiConnection(this), but the added test suite only
calls the helper directly and therefore does not cover the quit() integration.

Rule 389273: Require tests for all new functionality and bug fixes
javascript/selenium-webdriver/lib/webdriver.js[798-799]
javascript/selenium-webdriver/test/lib/bidi_connection_test.js[39-95]

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

## Issue description
No regression test verifies that `WebDriver.quit()` closes a BiDi connection now stored outside the driver instance.

## Issue Context
The existing tests call `closeBidiConnection` directly. Add a test exercising `quit()` whose assertion would fail if the delegation at line 799 were reverted or removed.

## Fix Focus Areas
- javascript/selenium-webdriver/lib/webdriver.js[798-799]
- javascript/selenium-webdriver/test/lib/bidi_connection_test.js[39-95]

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


Qodo Logo

Comment thread javascript/selenium-webdriver/lib/bidi_connection.js Outdated
Comment thread javascript/selenium-webdriver/lib/bidi_connection.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/logging.js
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/webdriver.js Outdated
Comment thread javascript/selenium-webdriver/lib/logging.js
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 330507e

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 20c8e70

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@pujagani

Copy link
Copy Markdown
Contributor Author

Failing test is of dotnet and not related to the changes made here.

@pujagani
pujagani merged commit e059591 into SeleniumHQ:trunk Aug 18, 2026
7 checks passed
@qodo-code-review

qodo-code-review Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

No code changes since the last review — review skipped

Qodo Logo

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

Labels

C-nodejs JavaScript Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants