[bidi] mark BiDi types extensible per spec and update Ruby handling

[titusfortner]

🔗 Related Issues

• Removes unnecessary addition from [rb] add objectOnly/preserveExtras/scalar-primitive BiDi schema signals #17784
• [adr] Behavioral contract for the low-level WebDriver BiDi layer #17786

💥 What does this PR do?

• Makes the shared schema mark a type extensible whenever the spec does, rather than only when it is also reachable from a command's params, so bindings can implement spec extensibility faithfully.
• Corrects Ruby's handling to match: all extensible types store undeclared fields, a closed type warns and drops them, and an outbound extra cannot shadow a declared field.

🔧 Implementation Notes

• Dropped preserveExtras (and its reSendableTypes reachability walk, plus the now-unused typeRefNames / refNames helpers) from the schema projector. The pre-existing extensible flag already carries the signal, so bindings key off it directly.
• Three received-only types now retain extras and gain an extensions accessor: Network::Cookie, Session::NewResult::Capabilities, Storage::PartitionKey.
• The Ruby generator gates its extensions store on extensible instead of preserveExtras. Regenerated protocol/*.rb and .rbs output is included.
• Ruby serialization (record.rb): the undeclared-field warning moved inside the non-extensible branch, and as_json rejects an extra whose key matches a declared field's wire key. That check sits at the single merge point covering new, with, and in-place extensions mutation.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: generator changes, test updates, regenerated protocol output, this description
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

• Naming: recommend implementations use Extensions as the name for the per-type passthrough map; and AdditionalData for envelope-level extras (not implemented here)

🔄 Types of changes

• Bug fix (backwards compatible)

[selenium-ci]

Thank you, @titusfortner for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

[qodo-code-review]

PR Summary by Qodo

Mark BiDi types extensible per spec and align Ruby extras passthrough

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Remove preserveExtras and treat extensible as the single wire-extras signal.
• Make Ruby records preserve undeclared fields for all extensible types, silently.
• Reject outbound extension keys that collide with declared wire keys; update specs.

Diagram

graph TD
  Spec{{"WebDriver BiDi spec"}} --> Projector["JS schema projector"] --> Schema["Shared schema JSON"] --> RbGen["Ruby BiDi generator"] --> RbProto["Generated protocol records"] --> RbRecord["Ruby Record serialization"] --> Wire["Wire JSON"]
  RbTests["Ruby serialization specs"] --> RbRecord

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep `preserveExtras` as a deprecated alias

• ➕ Avoids breaking any downstream consumers still checking preserveExtras
• ➕ Allows phased migration across bindings/releases
• ➖ Continues carrying redundant/ambiguous signaling in the shared schema
• ➖ Encourages bindings to reintroduce send-reachability heuristics that diverge from spec intent

2. Represent extras as a dedicated wire field (e.g., `extensions`)

• ➕ Avoids key-collision concerns by construction
• ➕ Simplifies merge logic
• ➖ Does not match BiDi’s model (extras are additional properties, not a nested field)
• ➖ Would be incompatible with existing wire payloads/spec semantics

Recommendation: Prefer the PR’s approach: treat extensible as the single, spec-authored signal and remove send-reachability heuristics. If there are known external schema consumers, consider temporarily emitting preserveExtras = extensible for one release as a compatibility bridge; otherwise, the simplification is the cleanest long-term contract.

Files changed (12) +79 / -89

Bug fix (10) +48 / -71

project_bidi_schema.mjsRemove 'preserveExtras' reachability analysis from schema projection +2/-53

Remove 'preserveExtras' reachability analysis from schema projection

• Deletes the helper walks ('refNames', 'typeRefNames') and the command-params reachability closure used to derive 'preserveExtras'. Updates documentation/comments so bindings use 'extensible' directly as the sole extras passthrough signal.

javascript/selenium-webdriver/project_bidi_schema.mjs

network.rbMark Network::Cookie as extensible in generated protocol +2/-1

Mark Network::Cookie as extensible in generated protocol

• Adds 'extensible: true' to 'Network::Cookie' so unknown wire properties are preserved and surfaced via 'extensions'. This aligns the generated Ruby protocol type with spec extensibility.

rb/lib/selenium/webdriver/bidi/protocol/network.rb

session.rbMark Session::NewResult::Capabilities as extensible in generated protocol +2/-1

Mark Session::NewResult::Capabilities as extensible in generated protocol

• Adds 'extensible: true' to 'Session::NewResult::Capabilities', enabling passthrough storage of undeclared wire keys on this received payload type.

rb/lib/selenium/webdriver/bidi/protocol/session.rb

storage.rbMark Storage::PartitionKey as extensible in generated protocol +2/-1

Mark Storage::PartitionKey as extensible in generated protocol

• Adds 'extensible: true' to 'Storage::PartitionKey', ensuring undeclared keys are captured and re-emitted for this spec-extensible record.

rb/lib/selenium/webdriver/bidi/protocol/storage.rb

record.rbSilently preserve extras for extensible records and prevent shadowing on serialize +26/-8

Silently preserve extras for extensible records and prevent shadowing on serialize

• Moves undeclared-property warnings into the non-extensible branch so extensible records capture extras without warnings. Replaces direct 'payload.merge!(extensions)' with 'merge_extensions!', which raises if an extension key collides with any declared field wire key before merging.

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

bidi_generate.rbGenerate Ruby record extensibility from schema 'extensible' +4/-4

Generate Ruby record extensibility from schema 'extensible'

• Updates the generator to gate the Ruby extensions store on the schema’s 'extensible' flag rather than 'preserveExtras'. This makes all spec-extensible types generated as extensible, independent of send/receive reachability.

rb/lib/selenium/webdriver/bidi/support/bidi_generate.rb

network.rbsExpose 'extensions' in RBS for Network::Cookie +2/-1

Expose 'extensions' in RBS for Network::Cookie

• Adds an 'extensions' reader and an optional 'extensions:' keyword parameter to the 'Network::Cookie' constructor signature to reflect the extensible record shape.

rb/sig/lib/selenium/webdriver/bidi/protocol/network.rbs

session.rbsExpose 'extensions' in RBS for Session::NewResult::Capabilities +2/-1

Expose 'extensions' in RBS for Session::NewResult::Capabilities

• Adds an 'extensions' reader and optional 'extensions:' keyword to the 'Session::NewResult::Capabilities' constructor signature for extensible passthrough data.

rb/sig/lib/selenium/webdriver/bidi/protocol/session.rbs

storage.rbsExpose 'extensions' in RBS for Storage::PartitionKey +2/-1

Expose 'extensions' in RBS for Storage::PartitionKey

• Adds an 'extensions' reader and optional 'extensions:' keyword parameter to the 'Storage::PartitionKey' signature to match generated extensible behavior.

rb/sig/lib/selenium/webdriver/bidi/protocol/storage.rbs

serialization.rbsAdd RBS signature for 'merge_extensions!' helper +4/-0

Add RBS signature for 'merge_extensions!' helper

• Declares the private 'merge_extensions!' method used by 'Record#as_json' to merge extras safely, aligning the signature with the new runtime behavior.

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

Tests (2) +31 / -18

project_bidi_schema_test.mjsUpdate schema-signal tests to assert 'extensible' is unconditional +7/-6

Update schema-signal tests to assert 'extensible' is unconditional

• Renames the schema-signal test suite and replaces the 'preserveExtras' reachability expectation with an assertion that all spec-extensible types remain marked extensible regardless of command param/result reachability.

javascript/selenium-webdriver/project_bidi_schema_test.mjs

serialization_spec.rbAdjust extensible-record specs and add collision rejection test +24/-12

Adjust extensible-record specs and add collision rejection test

• Updates expectations so extensible records no longer warn when capturing unknown keys, including for previously received-only extensible types. Adds a new test asserting serialization raises when 'extensions' attempts to shadow a declared wire key.

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

[qodo-code-review]

Code Review by Qodo

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

1. Symbol extras bypass collision ✓ Resolved 🐞 Bug ≡ Correctness

Description

Record#merge_extensions! only checks extension-key collisions using raw Hash keys, so an extension
like :name bypasses the guard against shadowing the declared wire key "name". When the payload is
later JSON-serialized, symbol keys are stringified, producing duplicate/ambiguous keys on the wire
and effectively reintroducing field clobbering.

Code

rb/lib/selenium/webdriver/bidi/serialization/record.rb[R317-324]

+            def merge_extensions!(payload)
+              collisions = extensions.keys & self.class.fields.map(&:wire_key)
+              unless collisions.empty?
+                raise ::ArgumentError, "#{self.class.name} extensions shadow declared fields: #{collisions.join(', ')}"
+              end
+
+              payload.merge!(extensions)
+            end

Evidence

Extensions are accepted from kwargs without key normalization, declared wire keys are stored as
strings, and the collision check compares keys as-is; JSON serialization later stringifies symbol
keys, so a symbol-keyed extension can still collide on the wire.

rb/lib/selenium/webdriver/bidi/serialization/record.rb[58-64]
rb/lib/selenium/webdriver/bidi/serialization/record.rb[72-80]
rb/lib/selenium/webdriver/bidi/serialization/record.rb[313-324]
rb/lib/selenium/webdriver/common/websocket_connection.rb[99-104]

Agent prompt

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

### Issue description
`merge_extensions!` checks collisions using `extensions.keys` vs declared `wire_key` strings, but does not normalize extension keys. Ruby callers can pass `extensions: {name: 'x'}` (symbol key), which bypasses the collision check because `:name != 'name'` in Ruby. Later, `JSON.generate(...)` stringifies symbol keys, producing duplicate/ambiguous JSON properties and allowing extension values to shadow typed fields.

### Issue Context
- Extensions can come from user code via `Record.new(..., extensions: ...)` and can be mutated in-place via `record.extensions[:foo] = ...`.
- Declared field wire keys are strings.
- WebSocket transport uses `JSON.generate`, which stringifies symbol keys.

### Fix Focus Areas
- rb/lib/selenium/webdriver/bidi/serialization/record.rb[72-80]
- rb/lib/selenium/webdriver/bidi/serialization/record.rb[296-324]

### Suggested fix approach
1. In `merge_extensions!(payload)`, build a normalized extensions hash with **string keys** (e.g., allow String/Symbol keys, coerce via `to_s`, reject others).
2. Run collision detection against declared wire keys using the normalized keys.
3. Merge the normalized hash into `payload`.
4. Add/extend a unit test asserting that `extensions: {name: 'clobber'}` (symbol key) raises the same shadowing error as `{'name' => 'clobber'}`.

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

---

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

[qodo-code-review]

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