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

javascript/selenium-webdriver/project_bidi_schema.mjs

@@ -21,7 +21,7 @@
  * The normalizer has already removed the awkward CDDL shapes, so this is a
  * straight mapping into a small vocabulary:
  *
- *   type node:  { kind: 'record', fields: [field], map?, extensible?, preserveExtras?, specHref? }
+ *   type node:  { kind: 'record', fields: [field], map?, extensible?, specHref? }
  *             | { kind: 'enum',   values: [string], specHref? }
  *             | { kind: 'union',  variants: [ref], selector, objectOnly?, specHref? }
  *             | { kind: 'alias',  type, specHref? }
@@ -43,13 +43,10 @@
  * points at the editor's draft, so for an older generated artifact the target drifts
  * from the pinned source; synthetic types (and anything neither source covers) omit it.
  *
- * Three derived signals let a binding validate the wire boundary without re-deriving
+ * Two derived signals let a binding validate the wire boundary without re-deriving
  * anything itself:
  *   `objectOnly: true`   — a union all of whose arms are object (record) types, so a
  *                          non-object payload is a schema violation, not a scalar arm.
- *   `preserveExtras: true` — an `extensible` type that can also be *sent* (reachable
- *                          from a command's params), so unknown properties received on
- *                          the wire must be stored and echoed back rather than dropped.
  *   an inline `enum` ref carries the `primitive` its literals share, so even a scalar
  *   the normalizer did not hoist to a named enum is typed rather than opaque.
  *   `scalar` on an inline `union` ref marks a union with a bare-scalar arm (a map entry's
@@ -335,47 +332,6 @@ function variantIsObject(ref, types, seen = new Set()) {
   return false // enum
 }
 
-// The type-name refs a projected ref node points at, recursing through list / map /
-// inline union / inline record. (checkSchema has an equivalent local walk for its own
-// referential checks; this module-level one feeds the reachability closure below.)
-function refNames(node) {
-  if (!node) return []
-  if (node.ref) return [node.ref]
-  if (node.list) return refNames(node.list)
-  if (node.map) return refNames(node.map)
-  if (node.union) return node.union.flatMap(refNames)
-  if (node.record) return node.record.flatMap((f) => refNames(f.type))
-  return []
-}
-
-// The type-name refs a type *node* (record / union / alias) points at: a record's
-// field and map value types, a union's variants, an alias's target.
-function typeRefNames(node) {
-  if (node.kind === 'record') {
-    const refs = node.fields.flatMap((f) => refNames(f.type))
-    if (node.map) refs.push(...refNames(node.map))
-    return refs
-  }
-  if (node.kind === 'union') return node.variants
-  if (node.kind === 'alias') return refNames(node.type)
-  return []
-}
-
-// The set of types that can be *sent*: reachable from some command's params, through
-// fields, lists, unions, maps, and nested records/aliases. Results and events are not
-// roots — a type reached only through them is received-only. `preserveExtras` gates the
-// extras store on this, so only a type you can hand back keeps unknown wire properties.
-function reSendableTypes(commands, types) {
-  const reachable = new Set()
-  const visit = (name) => {
-    if (!name || reachable.has(name) || !types[name]) return
-    reachable.add(name)
-    for (const r of typeRefNames(types[name])) visit(r)
-  }
-  for (const c of commands) if (c.params?.ref) visit(c.params.ref)
-  return reachable
-}
-
 // The constant value a record pins on wire key `k`, as `{ value }` (a string or
 // `null`), or `{ open: true }` when the field exists but is not constant (a base
 // type acting as the catch-all, e.g. log.GenericLogEntry.type), or null when the
@@ -635,13 +591,6 @@ export function projectSchema(ast, model, links = {}) {
     }
   }
 
-  // An extensible type keeps unknown wire properties only when it is also re-sendable
-  // (reachable from a command's params) — a type you receive and can hand back, so its
-  // extras must round-trip. A received-only extensible type drops them.
-  const reSendable = reSendableTypes(commands, types)
-  for (const [name, node] of Object.entries(types))
-    if (node.extensible && reSendable.has(name)) node.preserveExtras = true
-
   // Per-domain module links, for a binding that emits one class/namespace per domain.
   const domains = {}
   for (const domain of Object.keys(model)) {

javascript/selenium-webdriver/project_bidi_schema_test.mjs

@@ -386,7 +386,7 @@ describe('unionSelector', () => {
   })
 })
 
-describe('schema signals (objectOnly / preserveExtras / enum primitive)', () => {
+describe('schema signals (objectOnly / extensible / enum primitive)', () => {
   const rec = (name, typeConst) => group(name, [field('type', [lit(typeConst)])])
   const union = (name, refs) => ({
     Type: 'variable',
@@ -430,7 +430,10 @@ describe('schema signals (objectOnly / preserveExtras / enum primitive)', () =>
     assert.equal(s.types['x.Origin'].objectOnly, undefined)
   })
 
-  it('marks an extensible type reachable from command params as preserveExtras, but not a result-only one', () => {
+  it('marks every extensible type extensible, regardless of send/receive reachability', () => {
+    // Extensibility is the whole signal: a type reachable only through a command's result
+    // keeps its extras store just as one reachable through params does. Send-reachability
+    // ("retain extras only where they can be sent back") is deliberately not a factor.
     const ast = [
       group('x.SetParams', [field('cfg', [ref('x.Config')])]),
       group('x.Config', [field('text', ['any'], { n: 0, m: null })]),
@@ -439,10 +442,8 @@ describe('schema signals (objectOnly / preserveExtras / enum primitive)', () =>
     ]
     const model = { x: { commands: [{ method: 'x.set', name: 'set', params: 'x.SetParams', result: 'x.GetResult' }] } }
     const s = projectSchema(ast, model)
-    assert.equal(s.types['x.Config'].extensible, true)
-    assert.equal(s.types['x.Config'].preserveExtras, true) // reachable through the command's params
-    assert.equal(s.types['x.Info'].extensible, true)
-    assert.equal(s.types['x.Info'].preserveExtras, undefined) // reachable only through the result
+    assert.equal(s.types['x.Config'].extensible, true) // reachable through the command's params
+    assert.equal(s.types['x.Info'].extensible, true) // reachable only through the result
     assert.deepEqual(checkSchema(s), [])
   })
 

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

@@ -150,7 +150,8 @@ class BytesValue < Serialization::Union
             http_only: {wire_key: 'httpOnly', primitive: 'boolean'},
             secure: {wire_key: 'secure', primitive: 'boolean'},
             same_site: {wire_key: 'sameSite', enum: 'Network::SAME_SITE'},
-            expiry: {wire_key: 'expiry', required: false, primitive: 'integer'}
+            expiry: {wire_key: 'expiry', required: false, primitive: 'integer'},
+            extensible: true
           )
 
           # @api private

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

@@ -204,7 +204,8 @@ class ProxyConfiguration < Serialization::Union
               required: false,
               ref: 'Session::UserPromptHandler'
             },
-            web_socket_url: {wire_key: 'webSocketUrl', required: false, primitive: 'string'}
+            web_socket_url: {wire_key: 'webSocketUrl', required: false, primitive: 'string'},
+            extensible: true
           )
 
           # @api private

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

@@ -33,7 +33,8 @@ class Storage < Domain
           # @see https://w3c.github.io/webdriver-bidi/#type-storage-PartitionKey
           PartitionKey = Serialization::Record.define(
             user_context: {wire_key: 'userContext', required: false, primitive: 'string'},
-            source_origin: {wire_key: 'sourceOrigin', required: false, primitive: 'string'}
+            source_origin: {wire_key: 'sourceOrigin', required: false, primitive: 'string'},
+            extensible: true
           )
 
           # @api private

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

@@ -81,8 +81,8 @@ def new(**kwargs)
 
             # Inbound: builds from the wire. A missing required field is omitted and warned (or
             # raised in strict mode, in +wire_value+); enum tokens are mapped back to symbols and an
-            # unrecognized one raises (in +read+); an undeclared property is warned, then captured
-            # (extensible) or dropped (closed) — strict on shape, lenient on extras.
+            # unrecognized one raises (in +read+); an undeclared property is captured silently
+            # (extensible) or warned and dropped (closed) — strict on shape, lenient on extras.
             def from_json(json_payload)
               unless json_payload.is_a?(::Hash)
                 raise Error::WebDriverError, "#{name} expected an object on the wire, got #{json_payload.inspect}"
@@ -92,8 +92,11 @@ def from_json(json_payload)
                 [f.name, wire_value(f, json_payload)]
               end
               undeclared = extra(json_payload)
-              warn_undeclared(undeclared) unless undeclared.empty?
-              attributes[:extensions] = undeclared if extensible?
+              if extensible?
+                attributes[:extensions] = undeclared # the spec sanctions these extras; preserve them silently
+              else
+                warn_undeclared(undeclared) unless undeclared.empty?
+              end
               construct(**attributes)
             end
 
@@ -268,9 +271,9 @@ def extra(json_payload)
               json_payload.except(*known)
             end
 
-            # Forward-compat signal: a property the type does not model is tolerated (retained on an
-            # extensible type, dropped on a closed one) and warned so schema drift is visible. Tagged
-            # +:bidi_undeclared_property+ so a caller can silence it via +logger.ignore+.
+            # Forward-compat signal: a property a closed type does not model is dropped and warned so
+            # schema drift is visible (an extensible type keeps its extras silently — the spec sanctions
+            # them). Tagged +:bidi_undeclared_property+ so a caller can silence it via +logger.ignore+.
             def warn_undeclared(undeclared)
               undeclared.each_key do |key|
                 WebDriver.logger.warn("#{name} received an undeclared property: #{key.inspect}",
@@ -301,9 +304,26 @@ def as_json(*)
                 value = Serialization.to_wire(value, Protocol.const_get(f.enum)) if f.enum
                 payload[f.wire_key] = Serializable.as_json(value)
               end
-              payload.merge!(extensions) if self.class.extensible? && !extensions.empty?
+              merge_extensions!(payload) if self.class.extensible? && !extensions.empty?
               payload
             end
+
+            private
+
+            # Merge the passthrough extras onto the wire, erroring rather than letting an extra whose key
+            # is a declared field's wire key silently clobber that typed value; an extra is by definition
+            # a field the spec does not declare. Keys are stringified first so a symbol key (e.g. `name:`)
+            # cannot slip past the guard and then reappear as a duplicate wire key once serialized. The
+            # single gate every outbound path funnels through: +new+, +with+, and in-place mutation.
+            def merge_extensions!(payload)
+              extras = extensions.transform_keys(&:to_s)
+              collisions = extras.keys & self.class.fields.map(&:wire_key)
+              unless collisions.empty?
+                raise ::ArgumentError, "#{self.class.name} extensions shadow declared fields: #{collisions.join(', ')}"
+              end
+
+              payload.merge!(extras)
+            end
           end
         end
       end

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

@@ -775,11 +775,11 @@ def record_class(name, type)
                                 wire: const['wire'], value: const['type']['const'],
                                 rbs: rbs_const(const['type']['const'])}
       fields = type['fields'].reject { |f| baked_discriminator?(f) }.map { |f| field_ir(f) }
-      # Gate the extensions store on `preserveExtras` (extensible AND re-sendable), not raw
-      # `extensible`: only a type you receive and can hand back keeps unknown wire keys. A
-      # received-only extensible type gets no store, so its unknown keys are silently ignored.
+      # Every extensible type gets the extensions store: an undeclared wire key is preserved
+      # and echoed back on any type the spec marks extensible, whether or not it is re-sendable.
+      # Extensibility alone is the signal; send-reachability does not enter into it.
       TypeClass.new(ruby_name: BiDiGenerate.type_class_name(name), fields: fields,
-                    discriminator: discriminator, extensible: type['preserveExtras'] ? true : false,
+                    discriminator: discriminator, extensible: type['extensible'] ? true : false,
                     schema_name: name, synthetic: type['synthetic'] ? true : false,
                     owner: type['owner'], label: type['label'], spec_href: type['specHref'])
     end

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

@@ -89,7 +89,8 @@ module Selenium
             attr_reader secure: bool
             attr_reader same_site: Symbol
             attr_reader expiry: untyped
-            def self.new: (name: String, value: ::Selenium::WebDriver::BiDi::Protocol::Network::BytesValue, domain: String, path: String, size: Integer, http_only: bool, secure: bool, same_site: Symbol, ?expiry: Integer) -> instance
+            attr_reader extensions: Hash[String, untyped]
+            def self.new: (name: String, value: ::Selenium::WebDriver::BiDi::Protocol::Network::BytesValue, domain: String, path: String, size: Integer, http_only: bool, secure: bool, same_site: Symbol, ?expiry: Integer, ?extensions: Hash[String, untyped]) -> instance
           end
 
           class CookieHeader < ::Selenium::WebDriver::BiDi::Serialization::Record

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

@@ -140,7 +140,8 @@ module Selenium
               attr_reader proxy: untyped
               attr_reader unhandled_prompt_behavior: untyped
               attr_reader web_socket_url: untyped
-              def self.new: (accept_insecure_certs: bool, browser_name: String, browser_version: String, platform_name: String, set_window_rect: bool, user_agent: String, ?proxy: ::Selenium::WebDriver::BiDi::Protocol::Session::ProxyConfiguration, ?unhandled_prompt_behavior: ::Selenium::WebDriver::BiDi::Protocol::Session::UserPromptHandler, ?web_socket_url: String) -> instance
+              attr_reader extensions: Hash[String, untyped]
+              def self.new: (accept_insecure_certs: bool, browser_name: String, browser_version: String, platform_name: String, set_window_rect: bool, user_agent: String, ?proxy: ::Selenium::WebDriver::BiDi::Protocol::Session::ProxyConfiguration, ?unhandled_prompt_behavior: ::Selenium::WebDriver::BiDi::Protocol::Session::UserPromptHandler, ?web_socket_url: String, ?extensions: Hash[String, untyped]) -> instance
             end
           end
 

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

@@ -26,7 +26,8 @@ module Selenium
           class PartitionKey < ::Selenium::WebDriver::BiDi::Serialization::Record
             attr_reader user_context: untyped
             attr_reader source_origin: untyped
-            def self.new: (?user_context: String, ?source_origin: String) -> instance
+            attr_reader extensions: Hash[String, untyped]
+            def self.new: (?user_context: String, ?source_origin: String, ?extensions: Hash[String, untyped]) -> instance
           end
 
           class CookieFilter < ::Selenium::WebDriver::BiDi::Serialization::Record

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

@@ -103,6 +103,10 @@ module Selenium
             def self.as_json: (untyped value) -> untyped
 
             def as_json: (*untyped) -> Hash[String, untyped]
+
+            private
+
+            def merge_extensions!: (Hash[String, untyped] payload) -> void
           end
         end
 

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

@@ -209,36 +209,56 @@ def valid_cookie_attrs
           end
 
           describe 'extensible records' do
-            it 'captures unknown keys and merges them back on serialization' do
+            # The spec sanctions extras on an extensible type, so they are captured silently — no
+            # undeclared-property warning, unlike a closed type.
+            it 'captures unknown keys silently and merges them back on serialization' do
               parsed = nil
               expect { parsed = Script::SharedReference.from_json('sharedId' => 's1', 'webdriverValue' => 42) }
-                .to have_warning(:bidi_undeclared_property)
+                .not_to have_warning(:bidi_undeclared_property)
 
               expect(parsed.shared_id).to eq('s1')
               expect(parsed.extensions).to eq('webdriverValue' => 42)
               expect(parsed.as_json).to eq('sharedId' => 's1', 'webdriverValue' => 42)
             end
 
-            # A re-sendable type (reachable from a command's params, e.g. a cookie filter) keeps
-            # unknown properties so a received-then-resent payload round-trips them.
-            it 'preserves an unknown key on a re-sendable type across a receive/re-send round trip' do
+            # An extensible type keeps unknown properties so a received-then-resent payload
+            # round-trips them. Extensibility alone is the trigger.
+            it 'preserves an unknown key on an extensible type across a receive/re-send round trip' do
               parsed = nil
               expect { parsed = Storage::CookieFilter.from_json('name' => 'sid', 'x-vendor' => 'keep-me') }
-                .to have_warning(:bidi_undeclared_property)
+                .not_to have_warning(:bidi_undeclared_property)
 
               expect(parsed.extensions).to eq('x-vendor' => 'keep-me')
               expect(parsed.as_json).to eq('name' => 'sid', 'x-vendor' => 'keep-me')
             end
 
             # network.Cookie is extensible but received-only (not reachable from any command's
-            # params), so preserveExtras is false: unknown keys are ignored, not stored/echoed.
-            it 'drops an unknown key on an extensible-but-received-only type on re-serialize' do
-              wire = Network::Cookie.new(**valid_cookie_attrs).as_json.merge('x-vendor' => 'drop-me')
+            # params); it still preserves and echoes an unknown key, because extensibility — not
+            # send-reachability — is what sanctions the extra field.
+            it 'preserves an unknown key on an extensible received-only type across re-serialize' do
+              wire = Network::Cookie.new(**valid_cookie_attrs).as_json.merge('x-vendor' => 'keep-me')
               parsed = nil
-              expect { parsed = Network::Cookie.from_json(wire) }.to have_warning(:bidi_undeclared_property)
+              expect { parsed = Network::Cookie.from_json(wire) }.not_to have_warning(:bidi_undeclared_property)
 
-              expect(parsed).not_to respond_to(:extensions)
-              expect(parsed.as_json).not_to include('x-vendor')
+              expect(parsed.extensions).to eq('x-vendor' => 'keep-me')
+              expect(parsed.as_json).to include('x-vendor' => 'keep-me')
+            end
+
+            # An extra is by definition a field the spec does not declare, so an extensions key that
+            # collides with a declared wire key would silently clobber a typed, validated value on the
+            # wire. Reject it at the merge instead — the single gate every outbound path funnels through.
+            it 'rejects an extension that shadows a declared field on serialize' do
+              cookie = Network::Cookie.new(**valid_cookie_attrs, extensions: {'name' => 'clobber'})
+
+              expect { cookie.as_json }.to raise_error(ArgumentError, /extensions shadow declared fields: name/)
+            end
+
+            # A symbol key stringifies to a declared wire key on serialization, so it must trip the same
+            # guard rather than ride onto the wire as a duplicate of the typed field.
+            it 'rejects a symbol-keyed extension that shadows a declared field on serialize' do
+              cookie = Network::Cookie.new(**valid_cookie_attrs, extensions: {name: 'clobber'})
+
+              expect { cookie.as_json }.to raise_error(ArgumentError, /extensions shadow declared fields: name/)
             end
 
             it 'warns on and drops an unknown key on a non-extensible type' do