You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Related to docs/decisions/17786-bidi-low-level-behavioral-contract.md
(#17786).
💥 What does this PR do?
Replaces the BiDi TypeScript generator's per-domain output with classes
that extend the shared Domain base (bidi/domain.js) instead of standalone
modules with hand-rolled connection glue.
Commands validate outbound
params and parse inbound results through the runtime schema layer
(defineRecord/defineEnum/defineUnion).
Events are exposed as static EventDescriptor constants consumed via the generic Domain#addCallback(),
replacing one hand-generated on() method per event.
• Generates schema-backed domain classes extending the shared low-level Domain base.
• Validates command parameters, results, and event payloads through runtime schemas.
• Replaces generated event methods with reusable static descriptors and generic callbacks.
Diagram
graph TD
A["CDDL AST"] --> B["Schema Projector"] --> C["BiDi Generator"] --> D["Domain Classes"] --> E["Domain Base"] --> F["BiDi Connection"]
D --> G["Schema Runtime"]
H["Bazel Build"] --> C
H --> E
Loading
High-Level Assessment
The following are alternative approaches to this PR:
1. Retain generator enhancement manifest
➕ Preserves deprecated convenience methods and compatibility aliases
➕ Reduces immediate migration work for existing generated API consumers
➖ Mixes high-level conveniences into the low-level protocol binding
➖ Continues maintaining hand-written code fragments outside the schema
➖ Weakens consistency with the documented cross-language contract
2. Keep cddl2ts type generation
➕ Reuses the previous TypeScript conversion dependency
➕ Minimizes custom type-emission logic
➖ Requires textual post-processing and prefix-based domain inference
➖ Separates compile-time types from runtime validation metadata
➖ Makes schema-faithful cross-domain references harder to maintain
3. Separate compatibility facade
➕ Keeps generated domains contract-faithful
➕ Allows deprecated convenience APIs to migrate independently
➕ Provides a clearer boundary between low-level and ergonomic APIs
➖ Introduces another maintained API layer
➖ Requires explicit packaging and migration planning
Recommendation: Use the PR's schema-driven Domain-based generator for the low-level binding. If deprecated conveniences must remain, preserve them in a separate handwritten compatibility facade rather than restoring manifest-injected generator fragments.
• Replaces cddl2ts and manifest-driven customization with binding-neutral schema projection and direct TypeScript emission. Generated classes extend Domain, validate command and event payloads through runtime schemas, expose static event and enum descriptors, and include specification-linked documentation.
log_test.jsTest log events through the generic descriptor API+24/-22
Test log events through the generic descriptor API
• Replaces specialized log listener helpers with Log.ENTRY_ADDED subscriptions. Tests now filter console and JavaScript entries explicitly from the low-level event payload.
BUILD.bazelWire schema projection modules into BiDi generation+2/-2
Wire schema projection modules into BiDi generation
• Adds the AST normalization and schema projection modules to the generator's runtime data. Removes the cddl2ts dependency and obsolete enhancements manifest input.
generate_bidi.bzlStage BiDi runtimes for generated TypeScript compilation+36/-6
Stage BiDi runtimes for generated TypeScript compilation
• Removes enhancement-manifest support from the generation rule. Copies Domain and serialization runtime files into Bazel's output tree so NodeNext can resolve generated relative imports during compilation.
1. defineEnum primitives lack tests 📘 Rule violation☼ Reliability⭐ New
Description
The widened enum contract now accepts numeric and boolean values, but the serialization tests
exercise only string enums. Numeric and boolean membership behavior—and the corresponding TypeScript
declarations—can regress without detection.
+export function defineEnum<T extends string | number | boolean>(name: string, values: readonly T[]): EnumEntry<T>
Evidence
PR Compliance ID 5 requires focused tests for behavioral changes. The declaration now accepts
string, number, and boolean enum values, while the existing serialization fixture invokes
defineEnum only with strings.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The enum serialization contract was expanded from string-only values to include numbers and booleans, but focused tests cover only a string enum.
## Issue Context
Add runtime membership tests for numeric and boolean enums and a TypeScript compilation check covering the widened `defineEnum` generic and `TypeNode.enum` declaration.
## Fix Focus Areas
- javascript/selenium-webdriver/bidi/serialization/enum.d.ts[18-33]
- javascript/selenium-webdriver/bidi/serialization/record.d.ts[23-27]
- javascript/selenium-webdriver/test/bidi/serialization/record_test.js[24-30]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Aliased results bypass validation 🐞 Bug≡ Correctness⭐ New
Description
Commands whose result type is an alias fall through to the unchecked cast because aliases have no
entry in runtimeByTsName. For example, browser.createUserContext accepts a malformed result
missing required userContext instead of parsing it through the aliased browser.UserContextInfo
record.
+ lines.push(` return (await this.send('${methodStr}', ${sendArg})) as ${resultTypeName}`)
Evidence
The schema declares browser.createUserContext with result ref browser.CreateUserContextResult,
which is an alias to browser.UserContextInfo; that record requires userContext. The generator
gives aliases no runtime binding, so resultRuntime is undefined and focused line 1061 returns the
wire payload through an unchecked TypeScript cast rather than invoking the record's fromWire().
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Generated command methods do not validate results whose direct schema type is an alias. Resolve aliases to their underlying runtime record or union so these results use `fromWire()` rather than the unchecked cast path.
## Issue Context
`browser.createUserContext` returns `browser.CreateUserContextResult`, an alias to the required-field record `browser.UserContextInfo`. Because aliases are excluded from `runtimeByTsName`, malformed responses are currently returned as if valid.
## Fix Focus Areas
- javascript/selenium-webdriver/generate_bidi.mjs[758-763]
- javascript/selenium-webdriver/generate_bidi.mjs[1026-1061]
- common/bidi/schema.json[14-24]
- common/bidi/schema.json[2606-2620]
- common/bidi/schema.json[2709-2714]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Generated commands validate a typed JS-facing object but discard the constructed record and send the
original object, while defineRecord expects wire-keyed constructor input. Consequently,
setMediaFeaturesOverride({features: {prefersColorScheme: 'dark'}}) fails validation, and JS-facing
renamed fields cannot be correctly transmitted.
+ if (paramsRuntime?.kind === 'record') {+ lines.push(` new ${paramsRuntime.runtimeName}(params)`)
Evidence
The projector deliberately creates distinct JS and wire names, and the emitted interface exposes
field.name. The runtime constructor instead checks and reads field.wire, while its toJSON() is
the mechanism that converts record instances back to wire names; generated methods construct such an
instance only for side effects and then send paramsCast. The real schema demonstrates this on
emulation.MediaFeatures, where prefersColorScheme maps to prefers-color-scheme and is nested
under setMediaFeaturesOverride parameters.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Generated commands discard the record instance used for validation and send the original JS-facing parameter object. Align record construction with JS-facing field names and retain/send the validated record so `toJSON()` converts fields to their declared wire names, including nested records.
## Issue Context
The schema intentionally maps keys such as `prefers-color-scheme` to `prefersColorScheme`, but `defineRecord` currently reads constructor data through `field.wire`. The generator then discards that instance and sends the original object.
## Fix Focus Areas
- javascript/selenium-webdriver/generate_bidi.mjs[1017-1038]
- javascript/selenium-webdriver/bidi/serialization/record.js[214-325]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Cross-domain schema dependencies are emitted only as import type, so their modules never execute
and register runtime types when consumers import a single generated domain. For example,
session.subscribe consequently accepts invalid contexts values without validating them as
browsing-context identifiers.
+ imports.push(`import type { ${[...names].sort().join(', ')} } from './${sourceFile}'`)
Evidence
The generator discovers nested cross-domain references but emits them exclusively with `import
type`, which is erased from generated JavaScript. Runtime records resolve referenced types through
the registry and return the supplied value unchanged when no registration exists;
session.SubscribeParameters.contexts concretely references browsingContext.BrowsingContext, so
importing only session skips element validation.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Cross-domain imports are erased during TypeScript compilation, leaving referenced schemas absent from the shared runtime registry. Emit side-effect runtime imports or establish an equivalent deterministic registration mechanism while retaining type imports as needed.
## Issue Context
Runtime reference validation resolves types through a shared registry and explicitly skips validation when a type has not registered. Importing only the generated `session` module therefore does not load the `browsingContext` module needed to validate `session.SubscribeParameters.contexts`.
## Fix Focus Areas
- javascript/selenium-webdriver/generate_bidi.mjs[700-717]
- javascript/selenium-webdriver/bidi/serialization/registry.js[18-32]
- javascript/selenium-webdriver/bidi/serialization/record.js[88-112]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The schema projector supports boolean literal enums, but defineEnum and TypeNode.enum now allow
only strings and numbers. A generated boolean enum or alias containing one will fail TypeScript
compilation despite being valid projector output.
The declarations restrict enum values to string | number, while literalPrimitive() classifies
all-boolean literals as boolean; enumNode() stores those values in enum, and projectType()
can emit them as first-class enum values. Therefore the declared runtime schema vocabulary is
narrower than the projector output it is intended to represent.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The schema projector can emit boolean literal enums, while the serialization declarations only accept string or number enum values. This makes valid projected schemas containing boolean choices fail TypeScript type checking.
## Issue Context
`literalPrimitive()` explicitly recognizes boolean literals, and both inline and named enum projection retain those boolean values. Runtime membership checking already works for booleans through `Set.has()` and `Array.includes()`.
## Fix Focus Areas
- javascript/selenium-webdriver/bidi/serialization/enum.d.ts[18-32]
- javascript/selenium-webdriver/bidi/serialization/enum.js[21-26]
- javascript/selenium-webdriver/bidi/serialization/record.d.ts[23-26]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Context sources
Review mode: ⚖️ Balanced
Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history
Generated commands validate a typed JS-facing object but discard the constructed record and send the
original object, while defineRecord expects wire-keyed constructor input. Consequently,
setMediaFeaturesOverride({features: {prefersColorScheme: 'dark'}}) fails validation, and JS-facing
renamed fields cannot be correctly transmitted.
+ if (paramsRuntime?.kind === 'record') {+ lines.push(` new ${paramsRuntime.runtimeName}(params)`)
Evidence
The projector deliberately creates distinct JS and wire names, and the emitted interface exposes
field.name. The runtime constructor instead checks and reads field.wire, while its toJSON() is
the mechanism that converts record instances back to wire names; generated methods construct such an
instance only for side effects and then send paramsCast. The real schema demonstrates this on
emulation.MediaFeatures, where prefersColorScheme maps to prefers-color-scheme and is nested
under setMediaFeaturesOverride parameters.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Generated commands discard the record instance used for validation and send the original JS-facing parameter object. Align record construction with JS-facing field names and retain/send the validated record so `toJSON()` converts fields to their declared wire names, including nested records.
## Issue Context
The schema intentionally maps keys such as `prefers-color-scheme` to `prefersColorScheme`, but `defineRecord` currently reads constructor data through `field.wire`. The generator then discards that instance and sends the original object.
## Fix Focus Areas
- javascript/selenium-webdriver/generate_bidi.mjs[1017-1038]
- javascript/selenium-webdriver/bidi/serialization/record.js[214-325]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Cross-domain validation is skipped✓ Resolved🐞 Bug≡ Correctness
Description
Cross-domain schema dependencies are emitted only as import type, so their modules never execute
and register runtime types when consumers import a single generated domain. For example,
session.subscribe consequently accepts invalid contexts values without validating them as
browsing-context identifiers.
+ imports.push(`import type { ${[...names].sort().join(', ')} } from './${sourceFile}'`)
Evidence
The generator discovers nested cross-domain references but emits them exclusively with `import
type`, which is erased from generated JavaScript. Runtime records resolve referenced types through
the registry and return the supplied value unchanged when no registration exists;
session.SubscribeParameters.contexts concretely references browsingContext.BrowsingContext, so
importing only session skips element validation.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Cross-domain imports are erased during TypeScript compilation, leaving referenced schemas absent from the shared runtime registry. Emit side-effect runtime imports or establish an equivalent deterministic registration mechanism while retaining type imports as needed.
## Issue Context
Runtime reference validation resolves types through a shared registry and explicitly skips validation when a type has not registered. Importing only the generated `session` module therefore does not load the `browsingContext` module needed to validate `session.SubscribeParameters.contexts`.
## Fix Focus Areas
- javascript/selenium-webdriver/generate_bidi.mjs[700-717]
- javascript/selenium-webdriver/bidi/serialization/registry.js[18-32]
- javascript/selenium-webdriver/bidi/serialization/record.js[88-112]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The schema projector supports boolean literal enums, but defineEnum and TypeNode.enum now allow
only strings and numbers. A generated boolean enum or alias containing one will fail TypeScript
compilation despite being valid projector output.
The declarations restrict enum values to string | number, while literalPrimitive() classifies
all-boolean literals as boolean; enumNode() stores those values in enum, and projectType()
can emit them as first-class enum values. Therefore the declared runtime schema vocabulary is
narrower than the projector output it is intended to represent.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The schema projector can emit boolean literal enums, while the serialization declarations only accept string or number enum values. This makes valid projected schemas containing boolean choices fail TypeScript type checking.
## Issue Context
`literalPrimitive()` explicitly recognizes boolean literals, and both inline and named enum projection retain those boolean values. Runtime membership checking already works for booleans through `Set.has()` and `Array.includes()`.
## Fix Focus Areas
- javascript/selenium-webdriver/bidi/serialization/enum.d.ts[18-32]
- javascript/selenium-webdriver/bidi/serialization/enum.js[21-26]
- javascript/selenium-webdriver/bidi/serialization/record.d.ts[23-26]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
B-buildIncludes scripting, bazel and CI integrationsB-devtoolsIncludes everything BiDi or Chrome DevTools relatedC-nodejsJavaScript Bindings
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Related Issues
Related to docs/decisions/17786-bidi-low-level-behavioral-contract.md
(#17786).
💥 What does this PR do?
Replaces the BiDi TypeScript generator's per-domain output with classes
that extend the shared Domain base (bidi/domain.js) instead of standalone
modules with hand-rolled connection glue.
Commands validate outbound
params and parse inbound results through the runtime schema layer
(defineRecord/defineEnum/defineUnion).
Events are exposed as static EventDescriptor constants consumed via the generic Domain#addCallback(),
replacing one hand-generated on() method per event.
Brings the generated output in line with the low-level BiDi behavioral
contract in docs/decisions/17786-bidi-low-level-behavioral-contract.md
([adr] Behavioral contract for the low-level WebDriver BiDi layer #17786).
🔧 Implementation Notes
🤖 AI assistance
🔄 Types of changes