[rb] always reject a missing required inbound BiDi field - #17936

• Raise SerializationError when required inbound BiDi fields are missing.
• Remove SE_BIDI_STRICT toggle and strict-mode branching from serialization.
• Update Ruby unit/integration tests to reflect strict inbound behavior.

graph TD
  A["Inbound BiDi JSON"] --> B["Record.from_json"] --> C["wire_value"] --> E{"Required field missing?"}
  E -- "no" --> D["read() / cast"]
  E -- "yes" --> F["raise SerializationError"]

Loading

The following are alternative approaches to this PR:

1. Keep SE_BIDI_STRICT as a deprecated transition flag

• ➕ Reduces immediate breakage for consumers interacting with lagging browser implementations
• ➕ Allows phased rollout while updating generated schemas/overrides
• ➖ Perpetuates two runtime behaviors and increases long-term maintenance and test matrix
• ➖ Contradicts the stated ADR direction to reject tolerance at runtime

2. Collect missing-required errors and return a partially-populated object

• ➕ Can improve resilience for non-critical fields without hard failure
• ➕ Enables callers to decide whether missing fields are acceptable
• ➖ Produces invalid typed objects and pushes correctness burden to callers
• ➖ Complicates generated protocol layer and invariants around required/nullable fields

Recommendation: Given the ADR decision, the PR’s approach (always raising on missing required inbound fields and removing the toggle) is the cleanest and most predictable behavior for a typed protocol layer. If compatibility issues arise with lagging remote ends, prefer schema-level overrides (as noted in the PR) rather than reintroducing runtime tolerance.

Bug fix (1) +8 / -17

record.rbAlways raise on missing required inbound fields +8/-17

Always raise on missing required inbound fields

• Updates inbound deserialization semantics so required fields missing from the payload raise Error::SerializationError unconditionally. Removes the missing_required helper and folds the raise directly into wire_value.

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

Tests (2) +7 / -17

test_environment.rbStop forcing BiDi strict mode in integration test environment +0/-3

Stop forcing BiDi strict mode in integration test environment

• Removes the test harness logic that set SE_BIDI_STRICT=true when WEBDRIVER_BIDI is enabled. This reflects that strictness is no longer configurable via environment variable.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb

serialization_spec.rbUpdate unit tests to require errors for missing required inbound fields +7/-14

Update unit tests to require errors for missing required inbound fields

• Replaces the prior tolerance/warn behavior test with assertions that missing required fields raise SerializationError. Adds/keeps coverage that explicit null is accepted for required-and-nullable fields.

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

Other (2) +1 / -17

serialization.rbRemove strict inbound toggle (SE_BIDI_STRICT) from serialization runtime +1/-13

Remove strict inbound toggle (SE_BIDI_STRICT) from serialization runtime

• Deletes Serialization.strict? and its documentation, eliminating the env-controlled strict/lenient inbound behavior split. Keeps the module focused on UNSET, enum validation, and shared serialization helpers.

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

serialization.rbsRemove strict? and missing_required signatures from RBS +0/-4

Remove strict? and missing_required signatures from RBS

• Deletes the RBS declarations for Serialization.strict? and Record.missing_required to match the runtime removals. Keeps type signatures aligned with the simplified strict inbound behavior.

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

Description

This PR removes the previously user-facing SE_BIDI_STRICT toggle and the prior tolerant/warn
behavior for missing required inbound BiDi fields without a deprecation period or migration warning,
which can abruptly break users relying on it. It also leaves rb/CHANGES documenting the old
behavior and an environment variable that no longer exists/works, making the release notes
inaccurate.

Code

rb/lib/selenium/webdriver/bidi/serialization.rb[L43-46]

-        def self.strict?
-          value = ENV.fetch('SE_BIDI_STRICT', '').strip.downcase
-          !value.empty? && value != '0' && value != 'false'
-        end

Evidence

PR Compliance ID 2 requires public functionality to be deprecated before removal, but the PR changes
inbound BiDi deserialization to unconditionally raise Error::SerializationError when a required
field is absent, eliminating the earlier tolerated/warn path and removing the strict-mode toggle
interface from the serialization module. At the same time, rb/CHANGES still describes tolerant
behavior and references SE_BIDI_STRICT as the mechanism to escalate, demonstrating a mismatch
between documented and actual behavior and confirming that a deprecation notice or temporary
compatibility shim with warning is missing.

AGENTS.md: Follow deprecation policy before removing public functionality
rb/lib/selenium/webdriver/bidi/serialization/record.rb[217-226]
rb/lib/selenium/webdriver/bidi/serialization.rb[20-51]
rb/CHANGES[14-16]
rb/CHANGES[1-16]

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 previously user-facing `SE_BIDI_STRICT` toggle (and the prior tolerant+warn behavior for missing required inbound BiDi fields) was removed outright, but the deprecation policy requires a deprecation notice and/or a temporary compatibility shim with warning before full removal. Additionally, `rb/CHANGES` still documents the old tolerant behavior and references `SE_BIDI_STRICT`, which is now inaccurate given the new default behavior of always raising.

## Issue Context
The Ruby BiDi serialization layer previously supported a runtime toggle via `SE_BIDI_STRICT` (documented in `rb/CHANGES`) to escalate missing required inbound field handling. The new implementation always raises on missing required fields and the strict-mode env var and associated warning path were removed, leaving no deprecation warning for users who still set/use `SE_BIDI_STRICT` and leaving the changelog describing behavior that no longer exists.

## Fix Focus Areas
- rb/lib/selenium/webdriver/bidi/serialization.rb[20-51]
- rb/lib/selenium/webdriver/bidi/serialization/record.rb[217-226]
- rb/CHANGES[1-20]

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

💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗