[adr] script and logging async/event API on driver.script

[titusfortner]

📄 The decision, its rationale, considered options, and consequences are in the record file this
PR adds (docs/decisions/17776-bidi-script.md); read it there. The sections below are proposal
notes and review logistics.

🔗 Related

• [docs] Selenium 5 release charter #17717 — Selenium 5 release charter.
• [🚀 Feature]: Implement high level BiDi script commands #13992 — the summit-agreed surface this record consolidates (pin / unpin / execute, and
  add/remove for console-message, JavaScript-error, and DOM-mutation handlers under
  driver.script), and where the per-binding implementation history is tracked.
• [adr] BiDi implementation boundaries #17670 — BiDi is an internal implementation mechanism. driver.script is the protocol-neutral
  surface consistent with that boundary.

📝 Proposal notes

• Satisfies a charter requirement. This is the ADR the Selenium 5 charter ([docs] Selenium 5 release charter #17717) names for
  the script and logging async/event API; on acceptance the charter links back to it.
• Already agreed — this record consolidates. From the summit ([🚀 Feature]: Implement high level BiDi script commands #13992): the driver.script
  accessor; placing the log-domain console and error events under script; pin / unpin /
  execute; and separate add/remove handlers for console messages, JavaScript errors, and DOM
  mutations.
• New or revised here — needs fresh sign-off. Beyond the agreed surface, this record departs
  in two ways; the rest of its calls fill in details [🚀 Feature]: Implement high level BiDi script commands #13992 left unspecified and are reasoned in the
  record:
  • splits preload out of pin — the summit had a single pin; this separates run-by-handle
    (pin) from run-on-every-navigation (preload). Naming both lets the record classify each
    binding's current pin as one or the other;
  • returns a registration object from add_*, not an integer id.
• Left open on purpose. script.execute(source) overlaps the existing execute_script (and
  executeAsyncScript); whether script.execute supersedes and deprecates them is a larger
  migration decision this record does not make.
• Implementation is unspecified. The record fixes the surface and behavior; how each binding
  stores pinned scripts, routes log.entryAdded, and keys handler removal is left to the adopting
  PRs.

🗣 Discussion

Pending — not yet discussed.

📌 Tracking

Tracking issue: (linked on acceptance)

[qodo-code-review]

PR Summary by Qodo

Add ADR defining the driver.script async script & event API surface

📝 Documentation 🕐 10-20 Minutes

AI Description

• Add a proposed ADR standardizing the protocol-neutral driver.script surface across bindings.
• Define execute, pin, preload, and handler registration/removal for console/error/DOM
 mutations.
• Document key design choices: split pin vs preload, return registration objects, shaped
 payloads.

High-Level Assessment

The following are alternative approaches to this PR:

1. Expose console/error events under a separate `driver.log` accessor

• ➕ Mirrors BiDi protocol domains closely
• ➕ May feel more natural to spec-focused users
• ➖ Splits a single workflow across script and log (scripts + DOM mutation + console/error)
• ➖ Conflicts with existing Selenium meaning of “logs” (e.g., manage().logs())

2. Keep a single `pin` API for both run-on-demand and run-on-navigation behaviors

• ➕ Fewer commands to learn
• ➕ Matches some current binding implementations
• ➖ Encourages silent divergence across bindings (same name, different semantics)
• ➖ Makes migration and documentation less explicit

3. Return primitive integer/long ids from `add_*` handlers

• ➕ Simplest surface area
• ➕ Aligns with current behavior in multiple bindings
• ➖ Limits extensibility (harder to add metadata/behavior without new APIs)
• ➖ Pushes more handler bookkeeping details onto users

Recommendation: Proceed with the ADR’s approach: a single protocol-neutral driver.script accessor that owns script execution and related event subscriptions, with an explicit split between run-on-demand (pin) and run-on-navigation (preload) and registration objects for removals. This best reduces cross-binding semantic drift while keeping the API extensible and user-focused; the main alternatives (separate log, single pin, primitive ids) trade short-term simplicity for long-term inconsistency and migration friction.

Files changed (1) +145 / -0

Documentation (1) +145 / -0

bidi-script.mdProposed ADR for 'driver.script' script execution and event subscriptions +145/-0

Proposed ADR for 'driver.script' script execution and event subscriptions

• Adds a proposed architecture decision record defining a protocol-neutral 'driver.script' API for script execution ('execute', 'pin', 'preload') and subscriptions to console messages, JavaScript errors, and DOM mutations. Captures behavioral requirements (shaped payloads, handler exception logging) and documents considered alternatives and consequences for binding convergence.

docs/decisions/bidi-script.md

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 11 rules

1. Old/new values undefined 🐞 Bug ≡ Correctness ⭐ New

Description

Decision 6 requires DOM-mutation payloads to include “old and new” values for attributes and
character data, but does not define field presence/nullability when a value does not exist (e.g.,
attribute added/removed) or when an “old” value was not captured. This allows incompatible payload
shapes across bindings (null vs omitted vs sentinel).

Code

docs/decisions/17776-bidi-script.md[R104-105]

+   kind, and the fields for that kind — an attribute name with its old and new value, the old and new
+   character data, or the added and removed nodes).

Evidence

Lines 104–105 mandate old/new values for attribute and character-data mutations, but the ADR does
not define the nullability/presence rules needed to represent additions/removals or non-captured old
values consistently.

docs/decisions/17776-bidi-script.md[101-105]

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 ADR states mutation payloads include old/new values for attributes and character data, but it does not specify how to represent cases where one of those values is absent (e.g., attribute removal => new value does not exist) or unknown/not captured. Without a precise contract, bindings will diverge in nullability and field presence.

### Issue Context
Mutation payloads are a core part of the convergence goal; consumers need consistent semantics to interpret additions/removals/changes.

### Fix Focus Areas
- docs/decisions/17776-bidi-script.md[101-105]

### Suggested resolution
Define, for each mutation kind:
- whether `old_value` / `new_value` are always present vs nullable/optional
- how to represent “does not exist” (added/removed) vs “not captured/unknown”
- whether bindings must always capture old values, or if old values are best-effort (and how that is signaled)

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

---

2. Mutation types contract unclear ✓ Resolved 🐞 Bug ≡ Correctness

Description

The ADR says DOM-mutation handlers can observe attributes/child-list/character-data “or any
combination” but does not state whether mutation types are required or what the default is when not
provided. This can cause bindings to implement different defaults or validation behavior, breaking
portability.

Code

docs/decisions/17776-bidi-script.md[R74-75]

+   **DOM-mutation handlers select which changes to observe** — attribute, child-list,
+   character-data, or any combination.

Evidence

The ADR introduces selectable mutation kinds but provides no contract for whether selection is
required vs optional/defaulted.

docs/decisions/17776-bidi-script.md[73-75]

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 ADR doesn’t define the requiredness/default behavior for selecting DOM mutation types on handler registration.

## Issue Context
The text says handlers select mutation kinds “or any combination,” but it does not specify whether callers must supply at least one kind, what happens if they omit the selection, nor how empty/invalid selections are handled.

## Fix Focus Areas
- docs/decisions/17776-bidi-script.md[73-75]

## Suggested change
Clarify one of the following (explicitly):
1) `types` is mandatory (must include at least one kind), and specify the error/validation behavior for empty selections; or
2) `types` is optional, and specify the common default (e.g., observe all supported kinds vs attributes-only) and how to handle an empty list/set if provided.

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

---

3. Mutation payload incomplete ✓ Resolved 🐞 Bug ≡ Correctness

Description

The ADR supports character-data DOM mutations but the payload description only specifies
attribute/child-list oriented fields (attribute name/old+new values, added/removed nodes), leaving
the character-data payload shape undefined. This ambiguity can lead bindings to expose incompatible
fields/nullability for character-data mutations.

Code

docs/decisions/17776-bidi-script.md[R102-104]

+   source location and stack trace: console message (level, text, type, arguments, source, stack
+   trace), JavaScript error (message, source, stack trace), DOM mutation (target element, mutation
+   kind, attribute name with old and new value, added and removed nodes).

Evidence

The ADR explicitly allows observing character-data mutations, but the later payload definition
omits any character-data-specific fields and only lists attribute/child-list relevant ones.

docs/decisions/17776-bidi-script.md[74-75]
docs/decisions/17776-bidi-script.md[102-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
The ADR defines DOM-mutation handler types including `character-data`, but the payload section only describes fields that map to attributes and child-list mutations. Without specifying the payload contract for character-data (and the presence/nullability of fields that don't apply), bindings are likely to diverge.

## Issue Context
This ADR’s stated goal is cross-binding convergence on surface and behavior, including shaped event payloads.

## Fix Focus Areas
- docs/decisions/17776-bidi-script.md[74-75]
- docs/decisions/17776-bidi-script.md[102-104]

## Suggested change
Update the DOM-mutation payload description to be explicit per `mutation kind` (e.g., attributes vs child-list vs character-data), including what fields exist for each kind (or which are null/omitted) and add the missing character-data details (e.g., old/new text/value fields).

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

---

4. Console stacktrace ambiguity ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The ADR says console and JavaScript-error handlers are split so console subscribers “do not also
receive stack traces,” but later requires console-message payloads to include a stack trace; this
conflict can lead bindings to implement different payload shapes. Clarify whether the intent is “no
JavaScript-error stack traces in console events” vs “console messages never include any stack
trace.”

Code

docs/decisions/17776-bidi-script.md[R72-74]

+4. **Console messages and JavaScript errors are separate handlers**, even though one `log` event
+   feeds both, so a console subscriber does not also receive stack traces. **DOM-mutation handlers
+   select which changes to observe** — attribute, child-list, character-data, or any combination.

Evidence

Decision 4 states console subscribers should not receive stack traces, but Decision 6 and the
examples explicitly include/access a stack trace for console messages, creating an
implementer-facing contradiction.

docs/decisions/17776-bidi-script.md[72-74]
docs/decisions/17776-bidi-script.md[100-111]

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 ADR text conflicts on whether console-message handlers receive stack traces: Decision 4 implies console subscribers should not receive stack traces, while Decision 6 explicitly lists a stack trace in the console-message payload.

### Issue Context
This ADR is intended to drive cross-binding convergence; contradictory normative language will likely produce divergent implementations and payload types.

### Fix Focus Areas
- docs/decisions/17776-bidi-script.md[72-74]
- docs/decisions/17776-bidi-script.md[100-111]

### Suggested change
Adjust Decision 4 wording to explicitly distinguish *JavaScript-error stack traces* from *console-message stack traces* (or remove `stack` from Decision 6 if console messages are not meant to carry it), and make the examples consistent with the chosen rule.

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

---

View more (1)

5. Preload timing underspecified ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The ADR states preload(source) runs “before page scripts, on the current page and every
navigation,” but does not define behavior when preload() is called after the current document’s
scripts have already run. This ambiguity can cause bindings to diverge on whether the preload runs
immediately, only on the next navigation, or requires a reload to satisfy “before page scripts.”

Code

docs/decisions/17776-bidi-script.md[R57-59]

+3. **`preload(source)` registers a script to run automatically** — before page scripts, on the
+   current page and every navigation. It returns a handle that `remove_preload` discards; a preload
+   script runs only on its own, never by handle.

Evidence

The ADR asserts both 'before page scripts' and 'on the current page' but provides no definition for
how that should work when the current page has already executed scripts, leaving a key timing case
unspecified.

docs/decisions/17776-bidi-script.md[57-60]

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 ADR defines `preload(source)` as running "before page scripts" on the current page, but does not specify what happens if the user registers it after the page is already loaded (when "before page scripts" is no longer possible without a reload).

### Issue Context
Because this record is meant to standardize cross-binding semantics, leaving this lifecycle edge-case undefined will likely result in inconsistent user-visible behavior.

### Fix Focus Areas
- docs/decisions/17776-bidi-script.md[57-60]

### Suggested change
Add a normative statement describing the expected behavior for late registration (e.g., applies only to subsequently created documents; or executes immediately in the current document but without the "before page scripts" guarantee; or triggers a reload explicitly if that guarantee is required).

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

---

Previous review results

Review updated until commit 309ef00

Results up to commit c0cfe40

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

1. Console stacktrace ambiguity ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The ADR says console and JavaScript-error handlers are split so console subscribers “do not also
receive stack traces,” but later requires console-message payloads to include a stack trace; this
conflict can lead bindings to implement different payload shapes. Clarify whether the intent is “no
JavaScript-error stack traces in console events” vs “console messages never include any stack
trace.”

Code

docs/decisions/17776-bidi-script.md[R72-74]

+4. **Console messages and JavaScript errors are separate handlers**, even though one `log` event
+   feeds both, so a console subscriber does not also receive stack traces. **DOM-mutation handlers
+   select which changes to observe** — attribute, child-list, character-data, or any combination.

Evidence

Decision 4 states console subscribers should not receive stack traces, but Decision 6 and the
examples explicitly include/access a stack trace for console messages, creating an
implementer-facing contradiction.

docs/decisions/17776-bidi-script.md[72-74]
docs/decisions/17776-bidi-script.md[100-111]

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 ADR text conflicts on whether console-message handlers receive stack traces: Decision 4 implies console subscribers should not receive stack traces, while Decision 6 explicitly lists a stack trace in the console-message payload.

### Issue Context
This ADR is intended to drive cross-binding convergence; contradictory normative language will likely produce divergent implementations and payload types.

### Fix Focus Areas
- docs/decisions/17776-bidi-script.md[72-74]
- docs/decisions/17776-bidi-script.md[100-111]

### Suggested change
Adjust Decision 4 wording to explicitly distinguish *JavaScript-error stack traces* from *console-message stack traces* (or remove `stack` from Decision 6 if console messages are not meant to carry it), and make the examples consistent with the chosen rule.

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

---

2. Preload timing underspecified ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The ADR states preload(source) runs “before page scripts, on the current page and every
navigation,” but does not define behavior when preload() is called after the current document’s
scripts have already run. This ambiguity can cause bindings to diverge on whether the preload runs
immediately, only on the next navigation, or requires a reload to satisfy “before page scripts.”

Code

docs/decisions/17776-bidi-script.md[R57-59]

+3. **`preload(source)` registers a script to run automatically** — before page scripts, on the
+   current page and every navigation. It returns a handle that `remove_preload` discards; a preload
+   script runs only on its own, never by handle.

Evidence

The ADR asserts both 'before page scripts' and 'on the current page' but provides no definition for
how that should work when the current page has already executed scripts, leaving a key timing case
unspecified.

docs/decisions/17776-bidi-script.md[57-60]

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 ADR defines `preload(source)` as running "before page scripts" on the current page, but does not specify what happens if the user registers it after the page is already loaded (when "before page scripts" is no longer possible without a reload).

### Issue Context
Because this record is meant to standardize cross-binding semantics, leaving this lifecycle edge-case undefined will likely result in inconsistent user-visible behavior.

### Fix Focus Areas
- docs/decisions/17776-bidi-script.md[57-60]

### Suggested change
Add a normative statement describing the expected behavior for late registration (e.g., applies only to subsequently created documents; or executes immediately in the current document but without the "before page scripts" guarantee; or triggers a reload explicitly if that guarantee is required).

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

---

Results up to commit 5905691

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

1. Mutation types contract unclear ✓ Resolved 🐞 Bug ≡ Correctness

Description

The ADR says DOM-mutation handlers can observe attributes/child-list/character-data “or any
combination” but does not state whether mutation types are required or what the default is when not
provided. This can cause bindings to implement different defaults or validation behavior, breaking
portability.

Code

docs/decisions/17776-bidi-script.md[R74-75]

+   **DOM-mutation handlers select which changes to observe** — attribute, child-list,
+   character-data, or any combination.

Evidence

The ADR introduces selectable mutation kinds but provides no contract for whether selection is
required vs optional/defaulted.

docs/decisions/17776-bidi-script.md[73-75]

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 ADR doesn’t define the requiredness/default behavior for selecting DOM mutation types on handler registration.

## Issue Context
The text says handlers select mutation kinds “or any combination,” but it does not specify whether callers must supply at least one kind, what happens if they omit the selection, nor how empty/invalid selections are handled.

## Fix Focus Areas
- docs/decisions/17776-bidi-script.md[73-75]

## Suggested change
Clarify one of the following (explicitly):
1) `types` is mandatory (must include at least one kind), and specify the error/validation behavior for empty selections; or
2) `types` is optional, and specify the common default (e.g., observe all supported kinds vs attributes-only) and how to handle an empty list/set if provided.

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

---

2. Mutation payload incomplete ✓ Resolved 🐞 Bug ≡ Correctness

Description

The ADR supports character-data DOM mutations but the payload description only specifies
attribute/child-list oriented fields (attribute name/old+new values, added/removed nodes), leaving
the character-data payload shape undefined. This ambiguity can lead bindings to expose incompatible
fields/nullability for character-data mutations.

Code

docs/decisions/17776-bidi-script.md[R102-104]

+   source location and stack trace: console message (level, text, type, arguments, source, stack
+   trace), JavaScript error (message, source, stack trace), DOM mutation (target element, mutation
+   kind, attribute name with old and new value, added and removed nodes).

Evidence

The ADR explicitly allows observing character-data mutations, but the later payload definition
omits any character-data-specific fields and only lists attribute/child-list relevant ones.

docs/decisions/17776-bidi-script.md[74-75]
docs/decisions/17776-bidi-script.md[102-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
The ADR defines DOM-mutation handler types including `character-data`, but the payload section only describes fields that map to attributes and child-list mutations. Without specifying the payload contract for character-data (and the presence/nullability of fields that don't apply), bindings are likely to diverge.

## Issue Context
This ADR’s stated goal is cross-binding convergence on surface and behavior, including shaped event payloads.

## Fix Focus Areas
- docs/decisions/17776-bidi-script.md[74-75]
- docs/decisions/17776-bidi-script.md[102-104]

## Suggested change
Update the DOM-mutation payload description to be explicit per `mutation kind` (e.g., attributes vs child-list vs character-data), including what fields exist for each kind (or which are null/omitted) and add the missing character-data details (e.g., old/new text/value fields).

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

---

[Copilot]

Pull request overview

Adds a new Architecture/Design Record (ADR) that standardizes the cross-binding, protocol-neutral driver.script surface for script execution/pinning and for console/JS-error/DOM-mutation event subscriptions, intended to guide Selenium 5 convergence work.

Changes:

• Defines the driver.script API responsibilities and normative behavior for execute, pin/unpin, and preload/removal.
• Specifies handler registration/removal semantics (including returning registration objects) and the expected shaped payloads for console/error/mutation events.
• Documents current per-binding divergence and the intended convergence outcomes.

[qodo-code-review]

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

[qodo-code-review]

Code review by qodo was updated up to the latest commit 309ef00