[js] Add binding-neutral BiDi schema with cddl2ts-gated fidelity

[titusfortner]

🔗 Related Issues

Builds on #17657 (shared BiDi CDDL ast/model artifacts).

💥 What does this PR do?

Generates a single, binding-neutral BiDi schema (commands + events + types) from the shared CDDL artifacts, so the non-JS bindings can create client generators that consume one explicit, normalized artifact instead of each re-deriving the native CDDL shapes from the raw AST.

That re-derivation is where the bindings currently diverge and silently drop data — inline string-literal enums, variant-union params, composed-in base-type fields, optional/nullable fields. The schema normalizes all of these once (hoists inline enums to named types, canonicalizes variant unions into self-contained records, flattens group composition, preserves wire names and nullability verbatim) and the generation step fails the build if anything is dropped or dangling.

It lands the artifact and its fidelity gate only; no binding consumes it yet.

🔧 Implementation Notes

• Variant unions are flattened into self-contained variant records rather than cddl2ts's (A | B) & common intersection form, because the non-TS bindings can't express TS-style intersections cleanly. This duplicates common fields across variants but keeps each variant complete and the discriminator-conditional rule structural.
• Test added to compare output with cddl2ts — to ensure nothing was dropped in the implementation
• The schema becomes the only binding-facing artifact: bidi-ast.json / bidi-model.json are made package-private (dropping the cross-binding visibility added in [js] Expose BiDi CDDL ast and model as shared artifacts #17657), since the schema folds in the model and supersedes both for bindings. They remain internal inputs to the schema and to the existing JS generation. Nothing consumed the old visibility yet, so this changes a not-yet-used contract.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: the AST normalizer, schema projector, the cddl2ts differential test, and the Bazel wiring
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

• No binding consumes the schema yet — will need to re-evaluate whether it is missing functionality needed by the bindings when it is actually used
• Location — consider moving this tooling from in javascript/selenium-webdriver to javascript/bidi-support, the additional wiring seemed out of scope for this PR

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

[qodo-code-review]

PR Summary by Qodo

Add binding-neutral BiDi schema generator with cddl2ts fidelity gate
✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Description

• Generate a normalized, binding-neutral BiDi schema artifact (commands/events/types) from shared
 AST/model.
• Normalize CDDL AST to hoist inline enums/records, canonicalize variants, and flatten composition
 safely.
• Gate schema fidelity with mocha tests that diff against cddl2ts and fail on drift.

Diagram

graph TD
  AST[("BiDi AST JSON")] --> Norm["normalize_bidi_ast.mjs"] --> Proj["project_bidi_schema.mjs"] --> Schema[("BiDi schema JSON")]
  Model[("BiDi model JSON")] --> Proj
  AST --> Diff["bidi_schema_diff_test.mjs"] --> Oracle["cddl2ts (TS oracle)"]
  Model --> Diff

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Have bindings consume the normalized AST directly

• ➕ Avoids introducing a new schema vocabulary to maintain
• ➕ Keeps representation closer to upstream CDDL constructs
• ➖ Still forces each binding to implement CDDL-shape interpretation and type mapping
• ➖ Harder to enforce completeness/fidelity guarantees across languages

2. Emit a standard JSON Schema / OpenAPI-like spec

• ➕ Leverages existing validators and potential codegen ecosystems
• ➕ More familiar artifact for non-JS ecosystems
• ➖ BiDi’s union/variant semantics and wire-level naming/nullability don’t map cleanly
• ➖ Significant additional design work; likely still needs custom extensions

3. Extend the existing model artifact instead of adding a new schema artifact

• ➕ Fewer artifacts to version and distribute
• ➕ Potentially smaller incremental change if model is already widely consumed
• ➖ Model is command/event-focused; it’s not a complete, normalized type system
• ➖ Risks re-introducing implicit derivation and divergence for complex types

Recommendation: Keep the current approach: a dedicated, binding-neutral schema projection with a fail-closed validation and an independent cddl2ts differential gate. It provides a single explicit artifact for non-JS bindings, removes per-binding re-derivation, and makes fidelity regressions visible in CI. Revisit standard-schema formats later if/when bindings need broader ecosystem tooling.

Files changed (7) +1468 / -0

Enhancement (2) +744 / -0

normalize_bidi_ast.mjsAdd BiDi AST normalizer to canonicalize enums, variants, and composition +465/-0

Add BiDi AST normalizer to canonicalize enums, variants, and composition

• Adds a pure normalizer pipeline that dedupes defs, hoists inline string-literal unions to named enums, hoists inline records to named defs, canonicalizes variant-union params into self-contained variant records, and flattens group composition (including Extensible wildcard) while avoiding dispatch-hierarchy flattening. Provides a CLI for emitting a normalized AST artifact.

javascript/selenium-webdriver/normalize_bidi_ast.mjs

project_bidi_schema.mjsProject normalized AST+model into a flat, binding-neutral schema with validators +279/-0

Project normalized AST+model into a flat, binding-neutral schema with validators

• Implements schema projection into a small, language-agnostic vocabulary (record/enum/union/alias and ref/primitive/const/list/map), preserving wire names and nullability. Adds referential-integrity validation (checkSchema) and a generator-independent completeness check that re-derives expected command/event methods from the raw AST, with a self-cleaning allowlist for known upstream omissions.

javascript/selenium-webdriver/project_bidi_schema.mjs

Tests (3) +664 / -0

bidi_schema_diff_test.mjsAdd oracle-style diff test comparing schema projection to cddl2ts +289/-0

Add oracle-style diff test comparing schema projection to cddl2ts

• Implements a mocha differential test that parses cddl2ts TypeScript output and compares record fields, optional/nullable/array fidelity, enum values, and union member field sets against the projected schema. Includes explicit allowlists for reviewed, intentional divergences and flags stale allowlist entries.

javascript/selenium-webdriver/bidi_schema_diff_test.mjs

normalize_bidi_ast_test.mjsAdd unit tests for AST normalization transforms +245/-0

Add unit tests for AST normalization transforms

• Adds mocha unit coverage for enum hoisting behavior, variant canonicalization (including name trimming and orphan cleanup), composition flattening rules, dedupe semantics, and immutability of normalizeAst. Tests target the edge cases that previously caused cross-binding divergence.

javascript/selenium-webdriver/normalize_bidi_ast_test.mjs

project_bidi_schema_test.mjsAdd tests for schema projection and fail-closed validation +130/-0

Add tests for schema projection and fail-closed validation

• Adds unit tests covering enum hoisting, inline-record hoisting, extensible map handling, referential-integrity failures, and completeness validation behavior (including allowlisted drops and stale allowlist detection). Uses small representative fixtures to exercise the schema vocabulary and validators.

javascript/selenium-webdriver/project_bidi_schema_test.mjs

Other (2) +60 / -0

BUILD.bazelAdd Bazel targets for schema projector and mocha schema tests +34/-0

Add Bazel targets for schema projector and mocha schema tests

• Introduces a new js_binary for the schema projector and a mocha_test target that runs unit tests plus a cddl2ts differential fidelity test. Wires in generated AST/model artifacts and required node modules as test data.

javascript/selenium-webdriver/BUILD.bazel

generate_bidi.bzlAdd schema generation step to generate_bidi_library macro +26/-0

Add schema generation step to generate_bidi_library macro

• Extends the Bazel macro to run a new schema projection step after AST/model generation, emitting a *_schema.json artifact. Adds a schema_generator parameter with a default and makes schema generation fail-closed via the script’s validations.

javascript/selenium-webdriver/private/generate_bidi.bzl

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (2) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 14 rules

1. Null-only types become unknown 🐞 Bug ≡ Correctness ⭐ New

Description

In projectRef(), null and prelude nil alternatives are filtered out before projecting the
remaining type; if the input type is *only* null/nil, the remaining list is empty and the schema
emits { primitive: 'unknown', nullable: true } instead of a real null type. This can silently
corrupt schema fidelity for any CDDL field/alias defined as exactly null/nil, and the cddl2ts
diff check likely won’t catch it because it only asserts presence of nullability, not the underlying
primitive.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R73-86]

+// A `null` keyword or a `nil` prelude ref in a union means the value may be null.
+const isNullAlt = (e) =>
+  e === 'null' || (e && typeof e === 'object' && e.Type === 'group' && PRELUDE[e.Value] === 'null')
+
+function projectRef(type) {
+  const all = typeList(type)
+  const entries = all.filter((e) => !isNullAlt(e))
+  const node =
+    entries.length > 1
+      ? entries.every(isLiteral)
+        ? { enum: entries.map((e) => e.Value) }
+        : { union: entries.map(projectEntry) }
+      : projectEntry(entries[0])
+  if (entries.length < all.length) node.nullable = true // a `null` alternative means the value may be null

Evidence

The code explicitly maps nil to a null primitive, but then classifies nil as a nullable-union
alternative and filters it out. When all contains only nil/null, entries becomes empty, so
projectEntry(entries[0]) is invoked with undefined and returns { primitive: 'unknown' }, after
which nullable is set because entries.length < all.length.

javascript/selenium-webdriver/project_bidi_schema.mjs[49-95]

Agent prompt

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

### Issue description
`projectRef()` removes `null`/`nil` alternatives via `isNullAlt()`, then projects `entries[0]`. When the type is exactly `null`/`nil` (no non-null alternatives), `entries` becomes empty and `projectEntry(undefined)` returns `{ primitive: 'unknown' }`, producing an incorrect schema node.

### Issue Context
- `PRELUDE.nil` is mapped to `'null'`, and `projectEntry()` can correctly project a `nil` group ref to `{ primitive: 'null' }`.
- But `isNullAlt()` strips it before projection, which is only correct when there is at least one non-null alternative.

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[73-87]

### Suggested fix
- In `projectRef(type)`, after computing `entries`, add a guard:
 - If `entries.length === 0` and `all.length > 0`, return `{ primitive: 'null' }` (and **do not** set `nullable`), because the type is exactly null.
 - Only set `node.nullable = true` when there is at least one non-null alternative (i.e., `entries.length > 0 && entries.length < all.length`).
- Optionally add/extend a unit test to cover `nil`-only and `null`-only projection cases.

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

---

2. Alias refs unchecked ✓ Resolved 🐞 Bug ≡ Correctness

Description

checkSchema() never traverses kind: 'alias' nodes, so an alias whose RHS contains a {ref: ...}
can point at a missing type and still pass validation. This breaks the “fail-closed” schema gate by
allowing dangling references to ship undetected.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R166-200]

+/** Fail-closed validation: every ref resolves, and every ONE_OF entry is real. */
+export function checkSchema(schema) {
+  const errors = []
+  const has = (name) => Object.hasOwn(schema.types, name)
+  const refsIn = (node) =>
+    !node
+      ? []
+      : node.ref
+        ? [node.ref]
+        : node.list
+          ? refsIn(node.list)
+          : node.map
+            ? refsIn(node.map)
+            : node.union
+              ? node.union.flatMap(refsIn)
+              : node.record
+                ? node.record.flatMap((f) => refsIn(f.type))
+                : []
+
+  for (const c of [...schema.commands, ...schema.events]) {
+    for (const r of [...refsIn(c.params), ...refsIn(c.result ?? null)])
+      if (!has(r)) errors.push(`${c.method}: unresolved type ${r}`)
+  }
+  for (const [name, node] of Object.entries(schema.types)) {
+    if (node.kind === 'record')
+      for (const f of node.fields)
+        for (const r of refsIn(f.type)) if (!has(r)) errors.push(`${name}.${f.name}: unresolved type ${r}`)
+    if (node.kind === 'union')
+      for (const v of node.variants) if (!has(v)) errors.push(`${name}: unresolved variant ${v}`)
+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)
+  }
+  for (const name of Object.keys(ONE_OF)) if (!has(name)) errors.push(`oneOf: unknown type ${name}`)
+  return errors

Evidence

The projector can emit kind: 'alias' types, but the validator loop only checks record and
union nodes; it never validates refs that appear inside an alias RHS.

javascript/selenium-webdriver/project_bidi_schema.mjs[106-116]
javascript/selenium-webdriver/project_bidi_schema.mjs[166-200]

Agent prompt

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

### Issue description
`checkSchema()` validates refs for command/event params/results, record fields, and union variants, but it does not validate references contained inside `kind: 'alias'` nodes (the alias RHS). This allows unresolved/dangling refs to pass schema validation.

### Issue Context
Aliases are produced by `projectType()` for `variable` defs that are not pure enums and not unions of refs.

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[106-116]
- javascript/selenium-webdriver/project_bidi_schema.mjs[166-200]

### Proposed fix
- In `checkSchema()`, add a branch for `node.kind === 'alias'` that runs `refsIn(node.type)` and errors on any ref that does not exist in `schema.types`.
- Ensure `refsIn` can traverse the alias RHS as-is (it already handles `ref/list/map/union/record` type-ref shapes).

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

---

3. Record map refs unchecked ✓ Resolved 🐞 Bug ≡ Correctness

Description

projectRecord() can emit typed maps via record.map (from * text => T), but checkSchema()
only validates record.fields and ignores record.map. As a result, unresolved refs inside the map
value type won’t be caught by schema validation.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R126-198]

+function projectRecord(def) {
+  const record = { kind: 'record', fields: [] }
+  for (const prop of (def.Properties ?? []).flat()) {
+    if (!prop || typeof prop !== 'object') continue
+    // `m === null` is overloaded in this parser: a key-typed entry is a map
+    // (`* text => value`); an anonymous entry is a structural spread; everything
+    // else is just an optional field (the `?` quantifier). Only the first two
+    // are not real fields.
+    if (prop.Occurrence?.m === null && (!prop.Name || prop.Name in PRIMITIVES || prop.Name in PRELUDE)) {
+      if (prop.Name in PRIMITIVES || prop.Name in PRELUDE) {
+        const value = projectRef(prop.Type)
+        if (value.primitive === 'any') record.extensible = true
+        else record.map = value
+      }
+      continue
+    }
+    if (prop.Name) record.fields.push(projectField(prop))
+  }
+  return record
+}
+
+const typeRef = (name) => (name ? { ref: name } : null)
+
+/** Build the flat schema from the raw AST and the existing command/event model. */
+export function projectSchema(ast, model) {
+  const types = {}
+  for (const def of normalizeAst(ast)) if (def?.Name) types[def.Name] = projectType(def)
+
+  const commands = []
+  const events = []
+  for (const [domain, entry] of Object.entries(model)) {
+    for (const c of entry.commands ?? [])
+      commands.push({ domain, method: c.method, name: c.name, params: typeRef(c.params), result: typeRef(c.result) })
+    for (const e of entry.events ?? [])
+      events.push({ domain, method: e.method, name: e.name, params: typeRef(e.params) })
+  }
+
+  return { schemaVersion: 1, commands, events, types }
+}
+
+/** Fail-closed validation: every ref resolves, and every ONE_OF entry is real. */
+export function checkSchema(schema) {
+  const errors = []
+  const has = (name) => Object.hasOwn(schema.types, name)
+  const refsIn = (node) =>
+    !node
+      ? []
+      : node.ref
+        ? [node.ref]
+        : node.list
+          ? refsIn(node.list)
+          : node.map
+            ? refsIn(node.map)
+            : node.union
+              ? node.union.flatMap(refsIn)
+              : node.record
+                ? node.record.flatMap((f) => refsIn(f.type))
+                : []
+
+  for (const c of [...schema.commands, ...schema.events]) {
+    for (const r of [...refsIn(c.params), ...refsIn(c.result ?? null)])
+      if (!has(r)) errors.push(`${c.method}: unresolved type ${r}`)
+  }
+  for (const [name, node] of Object.entries(schema.types)) {
+    if (node.kind === 'record')
+      for (const f of node.fields)
+        for (const r of refsIn(f.type)) if (!has(r)) errors.push(`${name}.${f.name}: unresolved type ${r}`)
+    if (node.kind === 'union')
+      for (const v of node.variants) if (!has(v)) errors.push(`${name}: unresolved variant ${v}`)
+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)
+  }

Evidence

The projector places the typed map’s value type into record.map, but the validator’s record branch
only iterates node.fields, so refs inside record.map are never checked.

javascript/selenium-webdriver/project_bidi_schema.mjs[126-145]
javascript/selenium-webdriver/project_bidi_schema.mjs[189-194]

Agent prompt

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

### Issue description
`projectRecord()` stores typed-map value types under `record.map`, but `checkSchema()` does not validate refs inside `record.map`. This allows dangling type references to slip through the schema gate for records that use `* text => SomeType`.

### Issue Context
The schema vocabulary explicitly supports maps on records, and `projectRecord()` sets `record.map = value` when the wildcard entry is not `any`.

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[126-145]
- javascript/selenium-webdriver/project_bidi_schema.mjs[166-200]

### Proposed fix
- In `checkSchema()`, within the `node.kind === 'record'` branch, also validate `node.map` if present:
 - `for (const r of refsIn(node.map)) if (!has(r)) errors.push(`${name}: unresolved map value type ${r}`)`
- Keep existing `fields` validation unchanged.

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

---

View more (2)

4. projectSchema() JSDoc incomplete 📘 Rule violation ✧ Quality

Description

Several newly exported schema-related functions are missing complete JSDoc blocks with required
@param and @returns tags (and @throws where applicable), including projectSchema,
checkSchema, checkCompleteness, and multiple exports in normalize_bidi_ast.mjs. This violates
the compliance requirement to fully document exported APIs and reduces contract fidelity and
cross-binding/tooling clarity.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R149-279]

+/** Build the flat schema from the raw AST and the existing command/event model. */
+export function projectSchema(ast, model) {
+  const types = {}
+  for (const def of normalizeAst(ast)) if (def?.Name) types[def.Name] = projectType(def)
+
+  const commands = []
+  const events = []
+  for (const [domain, entry] of Object.entries(model)) {
+    for (const c of entry.commands ?? [])
+      commands.push({ domain, method: c.method, name: c.name, params: typeRef(c.params), result: typeRef(c.result) })
+    for (const e of entry.events ?? [])
+      events.push({ domain, method: e.method, name: e.name, params: typeRef(e.params) })
+  }
+
+  return { schemaVersion: 1, commands, events, types }
+}
+
+/** Fail-closed validation: every ref resolves, and every ONE_OF entry is real. */
+export function checkSchema(schema) {
+  const errors = []
+  const has = (name) => Object.hasOwn(schema.types, name)
+  const refsIn = (node) =>
+    !node
+      ? []
+      : node.ref
+        ? [node.ref]
+        : node.list
+          ? refsIn(node.list)
+          : node.map
+            ? refsIn(node.map)
+            : node.union
+              ? node.union.flatMap(refsIn)
+              : node.record
+                ? node.record.flatMap((f) => refsIn(f.type))
+                : []
+
+  for (const c of [...schema.commands, ...schema.events]) {
+    for (const r of [...refsIn(c.params), ...refsIn(c.result ?? null)])
+      if (!has(r)) errors.push(`${c.method}: unresolved type ${r}`)
+  }
+  for (const [name, node] of Object.entries(schema.types)) {
+    if (node.kind === 'record')
+      for (const f of node.fields)
+        for (const r of refsIn(f.type)) if (!has(r)) errors.push(`${name}.${f.name}: unresolved type ${r}`)
+    if (node.kind === 'union')
+      for (const v of node.variants) if (!has(v)) errors.push(`${name}: unresolved variant ${v}`)
+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)
+  }
+  for (const name of Object.keys(ONE_OF)) if (!has(name)) errors.push(`oneOf: unknown type ${name}`)
+  return errors
+}
+
+// ============================================================
+// CLI: raw ast + model → flat schema (validated)
+//   node project_bidi_schema.mjs --ast <ast.json> --model <model.json> --dump-schema <out.json>
+// ============================================================
+
+async function main() {
+  const { parseArgs } = await import('node:util')
+  const { readFileSync, writeFileSync } = await import('node:fs')
+  const { resolve } = await import('node:path')
+
+  // Under Bazel the js_binary wrapper chdir's to BAZEL_BINDIR, but $(location)
+  // inputs are execroot-relative and already carry that prefix — strip it so the
+  // path is not doubled. Mirrors resolveInputPath() in generate_bidi.mjs.
+  const resolveInput = (p) => {
+    if (!process.env.BAZEL_BINDIR) return resolve(p)
+    const prefix = process.env.BAZEL_BINDIR.replaceAll('\\', '/') + '/'
+    const norm = p.replaceAll('\\', '/')
+    return resolve(norm.startsWith(prefix) ? norm.slice(prefix.length) : norm)
+  }
+
+  const { values: args } = parseArgs({
+    options: { ast: { type: 'string' }, model: { type: 'string' }, 'dump-schema': { type: 'string' } },
+  })
+  if (!args.ast || !args.model || !args['dump-schema']) {
+    console.error('Usage: project_bidi_schema.mjs --ast <ast.json> --model <model.json> --dump-schema <out.json>')
+    process.exit(1)
+  }
+
+  const ast = JSON.parse(readFileSync(resolveInput(args.ast), 'utf8'))
+  const model = JSON.parse(readFileSync(resolveInput(args.model), 'utf8'))
+  const schema = projectSchema(ast, model)
+
+  // Generation is the gate: a broken or incomplete schema fails the build.
+  const errors = [...checkSchema(schema), ...checkCompleteness(ast, schema)]
+  if (errors.length) {
+    console.error('BiDi schema validation failed:')
+    errors.forEach((e) => console.error(`  ${e}`))
+    process.exit(1)
+  }
+
+  writeFileSync(resolve(args['dump-schema']), JSON.stringify(schema, null, 2) + '\n', 'utf8')
+  console.log(
+    `  ${schema.commands.length} commands, ${schema.events.length} events, ${Object.keys(schema.types).length} types → ${args['dump-schema']}`,
+  )
+}
+
+if (import.meta.main) {
+  main().catch((err) => {
+    console.error(err)
+    process.exit(1)
+  })
+}
+
+/**
+ * Independent completeness check: re-derive every command/event method straight
+ * from the raw AST (a leaf def carries a literal `method` property) and assert it
+ * survived into the schema. This compares input to output without trusting the
+ * generator, so a dropped command/event fails the build even if generation and
+ * its own checkSchema agree. Run as a Bazel test over committed fixtures.
+ */
+export function checkCompleteness(rawAst, schema) {
+  const emitted = new Set([...schema.commands, ...schema.events].map((c) => c.method))
+  const errors = []
+  for (const def of rawAst) {
+    const methodProp = (def.Properties ?? []).flat().find((p) => p?.Name === 'method')
+    const literal = methodProp && (Array.isArray(methodProp.Type) ? methodProp.Type[0] : methodProp.Type)
+    if (literal?.Type !== 'literal') continue
+    if (!emitted.has(literal.Value) && !KNOWN_INCOMPLETE.has(literal.Value))
+      errors.push(`dropped from schema: ${literal.Value}`)
+  }
+  // Self-cleaning: if a known-incomplete method is now emitted, the entry is
+  // stale and must be removed — so the allowlist cannot silently rot.
+  for (const known of KNOWN_INCOMPLETE) {
+    if (emitted.has(known)) errors.push(`stale KNOWN_INCOMPLETE entry (now emitted, remove it): ${known}`)
+  }
+  return errors
+}

Evidence

The compliance rule mandates that every exported function has a complete JSDoc block. The cited
exports projectSchema(ast, model), checkSchema(schema), and checkCompleteness(rawAst, schema)
in project_bidi_schema.mjs are documented with prose but lack the required @param and @returns
tags, and normalize_bidi_ast.mjs contains several export function ... declarations (e.g.,
hoistInlineEnums, hoistInlineRecords, canonicalizeVariantParams, flattenGroupComposition,
dedupeDefs, normalizeAst) that similarly have only prose comments and do not include the
required JSDoc tags, demonstrating noncompliance for newly exported APIs.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/project_bidi_schema.mjs[149-201]
javascript/selenium-webdriver/project_bidi_schema.mjs[263-279]
javascript/selenium-webdriver/normalize_bidi_ast.mjs[152-432]

Agent prompt

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

## Issue description
Exported functions in `javascript/selenium-webdriver/project_bidi_schema.mjs` and `javascript/selenium-webdriver/normalize_bidi_ast.mjs` are missing complete JSDoc blocks. Update each affected exported function to include a full JSDoc comment immediately preceding the declaration with a description plus `@param` tags for all parameters (in order), `@returns` for any non-void return values, and `@throws` where synchronous throws are possible.

## Issue Context
These exports are part of a binding-neutral schema toolchain, so consumers, maintainers, and tooling rely on JSDoc for a clear, machine-readable contract. The compliance rule requires a complete JSDoc block for each exported function declaration, not just prose.

## Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[149-201]
- javascript/selenium-webdriver/project_bidi_schema.mjs[263-279]
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[152-432]

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

---

5. Schema generator never runs 🐞 Bug ≡ Correctness

Description

project_bidi_schema.mjs (and normalize_bidi_ast.mjs) only calls main() behind `if
(import.meta.main), but Selenium’s Bazel Node toolchain is 22.x where import.meta.main` is not
set, so the js_binary will do nothing and the js_run_binary action will fail to write its declared
output.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R249-254]

+if (import.meta.main) {
+  main().catch((err) => {
+    console.error(err)
+    process.exit(1)
+  })
+}

Evidence

The generator’s main() is only executed if import.meta.main is truthy, but Bazel pins Node
22.22.0, and the normalizer file itself documents import.meta.main as a Node >=24 feature, so on
Node 22 this guard will be falsy and the CLI entrypoint won’t run.

javascript/selenium-webdriver/project_bidi_schema.mjs[249-254]
javascript/selenium-webdriver/normalize_bidi_ast.mjs[458-465]
MODULE.bazel[78-80]

Agent prompt

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

### Issue description
`project_bidi_schema.mjs` and `normalize_bidi_ast.mjs` gate execution with `if (import.meta.main)`, but Bazel config pins Node 22, where `import.meta.main` is not available. As a result, these scripts won't run when invoked as `js_binary` tools, and schema generation will fail due to missing outputs.

### Issue Context
- Bazel Node toolchain is pinned to Node 22.
- The schema generator is executed via `js_run_binary`, and must always run when invoked as the entry module.

### Fix
Replace the `import.meta.main` check with a Node-22-compatible “is this the entry module?” test, for example:

```js
import { pathToFileURL, fileURLToPath } from 'node:url'
import { resolve } from 'node:path'

function isMainModule() {
 if (!process.argv[1]) return false
 return pathToFileURL(resolve(process.argv[1])).href === import.meta.url
}

if (isMainModule()) {
 main().catch((err) => {
   console.error(err)
   process.exit(1)
 })
}
```

(Any equivalent guard that works on Node 22 is fine.)

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[249-254]
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[458-465]

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

---

6. oneOf validation unreachable 🐞 Bug ⚙ Maintainability

Description

projectType() only attaches oneOf to union nodes, but checkSchema() explicitly skips
validating oneOf for unions, so any future ONE_OF[...] entries can silently become invalid
(typos/missing fields) without being caught by the schema gate.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R195-197]

+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)

Evidence

The code adds oneOf only for kind: 'union' (in projectType()), but the validator
short-circuits union oneOf checks (node.kind === 'union' ? false : ...), so the only place
oneOf can appear is the one place it is never validated.

javascript/selenium-webdriver/project_bidi_schema.mjs[106-114]
javascript/selenium-webdriver/project_bidi_schema.mjs[189-199]

Agent prompt

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

### Issue description
`ONE_OF` is wired inconsistently:
- `projectType()` attaches `node.oneOf` only when emitting a `kind: 'union'` type.
- `checkSchema()` iterates `node.oneOf` but intentionally disables validation when `node.kind === 'union'`.

This makes the comment "validated by checkSchema" inaccurate and leaves future `ONE_OF` entries effectively unchecked.

### Issue Context
`ONE_OF` is currently empty, but this bug will matter the first time it’s used.

### Fix options (pick one and make it consistent)
1) **If `oneOf` is meant for record types:** attach `oneOf` to record nodes (where `fields` exist) and keep validation as a fields-existence check.
2) **If `oneOf` is meant for union types:** update `checkSchema()` to validate `oneOf` against union variants (e.g., ensure each named field exists on every variant record, or whatever semantics you intend).

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[106-114]
- javascript/selenium-webdriver/project_bidi_schema.mjs[195-197]

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

---

7. CLI uses console.log/error 📘 Rule violation ◔ Observability

Description

The new CLI entrypoints log via console.log/console.error instead of a standardized project
logging module and namespace. If this tooling is expected to follow the project's logging policy,
this should be migrated to the approved logger to ensure consistent formatting and routing.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R226-246]

+  if (!args.ast || !args.model || !args['dump-schema']) {
+    console.error('Usage: project_bidi_schema.mjs --ast <ast.json> --model <model.json> --dump-schema <out.json>')
+    process.exit(1)
+  }
+
+  const ast = JSON.parse(readFileSync(resolveInput(args.ast), 'utf8'))
+  const model = JSON.parse(readFileSync(resolveInput(args.model), 'utf8'))
+  const schema = projectSchema(ast, model)
+
+  // Generation is the gate: a broken or incomplete schema fails the build.
+  const errors = [...checkSchema(schema), ...checkCompleteness(ast, schema)]
+  if (errors.length) {
+    console.error('BiDi schema validation failed:')
+    errors.forEach((e) => console.error(`  ${e}`))
+    process.exit(1)
+  }
+
+  writeFileSync(resolve(args['dump-schema']), JSON.stringify(schema, null, 2) + '\n', 'utf8')
+  console.log(
+    `  ${schema.commands.length} commands, ${schema.events.length} events, ${Object.keys(schema.types).length} types → ${args['dump-schema']}`,
+  )

Evidence

The logging compliance item requires use of the project logging module and flags direct print-style
diagnostics. The new CLI code emits diagnostic messages using console.error(...) and
console.log(...).

Rule 389259: Use project logging module with standardized Selenium logger namespace
javascript/selenium-webdriver/project_bidi_schema.mjs[226-246]
javascript/selenium-webdriver/normalize_bidi_ast.mjs[447-456]

Agent prompt

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

## Issue description
New CLI scripts emit diagnostics with `console.log`/`console.error`. If project policy requires standardized logging, these should use the project logging wrapper and the required `selenium.webdriver.<modulename>` namespace (or the JS-equivalent standard, if different).

## Issue Context
The compliance rule flags direct stdout/stderr diagnostic logging in favor of the project logging system.

## Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[226-246]
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[447-456]

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

---

Previous review results

Review updated until commit 7c315df

Results up to commit 5a7fca3

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

1. projectSchema() JSDoc incomplete 📘 Rule violation ✧ Quality

Description

Several newly exported schema-related functions are missing complete JSDoc blocks with required
@param and @returns tags (and @throws where applicable), including projectSchema,
checkSchema, checkCompleteness, and multiple exports in normalize_bidi_ast.mjs. This violates
the compliance requirement to fully document exported APIs and reduces contract fidelity and
cross-binding/tooling clarity.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R149-279]

+/** Build the flat schema from the raw AST and the existing command/event model. */
+export function projectSchema(ast, model) {
+  const types = {}
+  for (const def of normalizeAst(ast)) if (def?.Name) types[def.Name] = projectType(def)
+
+  const commands = []
+  const events = []
+  for (const [domain, entry] of Object.entries(model)) {
+    for (const c of entry.commands ?? [])
+      commands.push({ domain, method: c.method, name: c.name, params: typeRef(c.params), result: typeRef(c.result) })
+    for (const e of entry.events ?? [])
+      events.push({ domain, method: e.method, name: e.name, params: typeRef(e.params) })
+  }
+
+  return { schemaVersion: 1, commands, events, types }
+}
+
+/** Fail-closed validation: every ref resolves, and every ONE_OF entry is real. */
+export function checkSchema(schema) {
+  const errors = []
+  const has = (name) => Object.hasOwn(schema.types, name)
+  const refsIn = (node) =>
+    !node
+      ? []
+      : node.ref
+        ? [node.ref]
+        : node.list
+          ? refsIn(node.list)
+          : node.map
+            ? refsIn(node.map)
+            : node.union
+              ? node.union.flatMap(refsIn)
+              : node.record
+                ? node.record.flatMap((f) => refsIn(f.type))
+                : []
+
+  for (const c of [...schema.commands, ...schema.events]) {
+    for (const r of [...refsIn(c.params), ...refsIn(c.result ?? null)])
+      if (!has(r)) errors.push(`${c.method}: unresolved type ${r}`)
+  }
+  for (const [name, node] of Object.entries(schema.types)) {
+    if (node.kind === 'record')
+      for (const f of node.fields)
+        for (const r of refsIn(f.type)) if (!has(r)) errors.push(`${name}.${f.name}: unresolved type ${r}`)
+    if (node.kind === 'union')
+      for (const v of node.variants) if (!has(v)) errors.push(`${name}: unresolved variant ${v}`)
+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)
+  }
+  for (const name of Object.keys(ONE_OF)) if (!has(name)) errors.push(`oneOf: unknown type ${name}`)
+  return errors
+}
+
+// ============================================================
+// CLI: raw ast + model → flat schema (validated)
+//   node project_bidi_schema.mjs --ast <ast.json> --model <model.json> --dump-schema <out.json>
+// ============================================================
+
+async function main() {
+  const { parseArgs } = await import('node:util')
+  const { readFileSync, writeFileSync } = await import('node:fs')
+  const { resolve } = await import('node:path')
+
+  // Under Bazel the js_binary wrapper chdir's to BAZEL_BINDIR, but $(location)
+  // inputs are execroot-relative and already carry that prefix — strip it so the
+  // path is not doubled. Mirrors resolveInputPath() in generate_bidi.mjs.
+  const resolveInput = (p) => {
+    if (!process.env.BAZEL_BINDIR) return resolve(p)
+    const prefix = process.env.BAZEL_BINDIR.replaceAll('\\', '/') + '/'
+    const norm = p.replaceAll('\\', '/')
+    return resolve(norm.startsWith(prefix) ? norm.slice(prefix.length) : norm)
+  }
+
+  const { values: args } = parseArgs({
+    options: { ast: { type: 'string' }, model: { type: 'string' }, 'dump-schema': { type: 'string' } },
+  })
+  if (!args.ast || !args.model || !args['dump-schema']) {
+    console.error('Usage: project_bidi_schema.mjs --ast <ast.json> --model <model.json> --dump-schema <out.json>')
+    process.exit(1)
+  }
+
+  const ast = JSON.parse(readFileSync(resolveInput(args.ast), 'utf8'))
+  const model = JSON.parse(readFileSync(resolveInput(args.model), 'utf8'))
+  const schema = projectSchema(ast, model)
+
+  // Generation is the gate: a broken or incomplete schema fails the build.
+  const errors = [...checkSchema(schema), ...checkCompleteness(ast, schema)]
+  if (errors.length) {
+    console.error('BiDi schema validation failed:')
+    errors.forEach((e) => console.error(`  ${e}`))
+    process.exit(1)
+  }
+
+  writeFileSync(resolve(args['dump-schema']), JSON.stringify(schema, null, 2) + '\n', 'utf8')
+  console.log(
+    `  ${schema.commands.length} commands, ${schema.events.length} events, ${Object.keys(schema.types).length} types → ${args['dump-schema']}`,
+  )
+}
+
+if (import.meta.main) {
+  main().catch((err) => {
+    console.error(err)
+    process.exit(1)
+  })
+}
+
+/**
+ * Independent completeness check: re-derive every command/event method straight
+ * from the raw AST (a leaf def carries a literal `method` property) and assert it
+ * survived into the schema. This compares input to output without trusting the
+ * generator, so a dropped command/event fails the build even if generation and
+ * its own checkSchema agree. Run as a Bazel test over committed fixtures.
+ */
+export function checkCompleteness(rawAst, schema) {
+  const emitted = new Set([...schema.commands, ...schema.events].map((c) => c.method))
+  const errors = []
+  for (const def of rawAst) {
+    const methodProp = (def.Properties ?? []).flat().find((p) => p?.Name === 'method')
+    const literal = methodProp && (Array.isArray(methodProp.Type) ? methodProp.Type[0] : methodProp.Type)
+    if (literal?.Type !== 'literal') continue
+    if (!emitted.has(literal.Value) && !KNOWN_INCOMPLETE.has(literal.Value))
+      errors.push(`dropped from schema: ${literal.Value}`)
+  }
+  // Self-cleaning: if a known-incomplete method is now emitted, the entry is
+  // stale and must be removed — so the allowlist cannot silently rot.
+  for (const known of KNOWN_INCOMPLETE) {
+    if (emitted.has(known)) errors.push(`stale KNOWN_INCOMPLETE entry (now emitted, remove it): ${known}`)
+  }
+  return errors
+}

Evidence

The compliance rule mandates that every exported function has a complete JSDoc block. The cited
exports projectSchema(ast, model), checkSchema(schema), and checkCompleteness(rawAst, schema)
in project_bidi_schema.mjs are documented with prose but lack the required @param and @returns
tags, and normalize_bidi_ast.mjs contains several export function ... declarations (e.g.,
hoistInlineEnums, hoistInlineRecords, canonicalizeVariantParams, flattenGroupComposition,
dedupeDefs, normalizeAst) that similarly have only prose comments and do not include the
required JSDoc tags, demonstrating noncompliance for newly exported APIs.

Rule 389257: Document public API functions with complete JSDoc blocks
javascript/selenium-webdriver/project_bidi_schema.mjs[149-201]
javascript/selenium-webdriver/project_bidi_schema.mjs[263-279]
javascript/selenium-webdriver/normalize_bidi_ast.mjs[152-432]

Agent prompt

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

## Issue description
Exported functions in `javascript/selenium-webdriver/project_bidi_schema.mjs` and `javascript/selenium-webdriver/normalize_bidi_ast.mjs` are missing complete JSDoc blocks. Update each affected exported function to include a full JSDoc comment immediately preceding the declaration with a description plus `@param` tags for all parameters (in order), `@returns` for any non-void return values, and `@throws` where synchronous throws are possible.

## Issue Context
These exports are part of a binding-neutral schema toolchain, so consumers, maintainers, and tooling rely on JSDoc for a clear, machine-readable contract. The compliance rule requires a complete JSDoc block for each exported function declaration, not just prose.

## Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[149-201]
- javascript/selenium-webdriver/project_bidi_schema.mjs[263-279]
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[152-432]

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

---

2. Schema generator never runs 🐞 Bug ≡ Correctness

Description

project_bidi_schema.mjs (and normalize_bidi_ast.mjs) only calls main() behind `if
(import.meta.main), but Selenium’s Bazel Node toolchain is 22.x where import.meta.main` is not
set, so the js_binary will do nothing and the js_run_binary action will fail to write its declared
output.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R249-254]

+if (import.meta.main) {
+  main().catch((err) => {
+    console.error(err)
+    process.exit(1)
+  })
+}

Evidence

The generator’s main() is only executed if import.meta.main is truthy, but Bazel pins Node
22.22.0, and the normalizer file itself documents import.meta.main as a Node >=24 feature, so on
Node 22 this guard will be falsy and the CLI entrypoint won’t run.

javascript/selenium-webdriver/project_bidi_schema.mjs[249-254]
javascript/selenium-webdriver/normalize_bidi_ast.mjs[458-465]
MODULE.bazel[78-80]

Agent prompt

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

### Issue description
`project_bidi_schema.mjs` and `normalize_bidi_ast.mjs` gate execution with `if (import.meta.main)`, but Bazel config pins Node 22, where `import.meta.main` is not available. As a result, these scripts won't run when invoked as `js_binary` tools, and schema generation will fail due to missing outputs.

### Issue Context
- Bazel Node toolchain is pinned to Node 22.
- The schema generator is executed via `js_run_binary`, and must always run when invoked as the entry module.

### Fix
Replace the `import.meta.main` check with a Node-22-compatible “is this the entry module?” test, for example:

```js
import { pathToFileURL, fileURLToPath } from 'node:url'
import { resolve } from 'node:path'

function isMainModule() {
 if (!process.argv[1]) return false
 return pathToFileURL(resolve(process.argv[1])).href === import.meta.url
}

if (isMainModule()) {
 main().catch((err) => {
   console.error(err)
   process.exit(1)
 })
}
```

(Any equivalent guard that works on Node 22 is fine.)

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[249-254]
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[458-465]

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

---

3. CLI uses console.log/error 📘 Rule violation ◔ Observability

Description

The new CLI entrypoints log via console.log/console.error instead of a standardized project
logging module and namespace. If this tooling is expected to follow the project's logging policy,
this should be migrated to the approved logger to ensure consistent formatting and routing.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R226-246]

+  if (!args.ast || !args.model || !args['dump-schema']) {
+    console.error('Usage: project_bidi_schema.mjs --ast <ast.json> --model <model.json> --dump-schema <out.json>')
+    process.exit(1)
+  }
+
+  const ast = JSON.parse(readFileSync(resolveInput(args.ast), 'utf8'))
+  const model = JSON.parse(readFileSync(resolveInput(args.model), 'utf8'))
+  const schema = projectSchema(ast, model)
+
+  // Generation is the gate: a broken or incomplete schema fails the build.
+  const errors = [...checkSchema(schema), ...checkCompleteness(ast, schema)]
+  if (errors.length) {
+    console.error('BiDi schema validation failed:')
+    errors.forEach((e) => console.error(`  ${e}`))
+    process.exit(1)
+  }
+
+  writeFileSync(resolve(args['dump-schema']), JSON.stringify(schema, null, 2) + '\n', 'utf8')
+  console.log(
+    `  ${schema.commands.length} commands, ${schema.events.length} events, ${Object.keys(schema.types).length} types → ${args['dump-schema']}`,
+  )

Evidence

The logging compliance item requires use of the project logging module and flags direct print-style
diagnostics. The new CLI code emits diagnostic messages using console.error(...) and
console.log(...).

Rule 389259: Use project logging module with standardized Selenium logger namespace
javascript/selenium-webdriver/project_bidi_schema.mjs[226-246]
javascript/selenium-webdriver/normalize_bidi_ast.mjs[447-456]

Agent prompt

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

## Issue description
New CLI scripts emit diagnostics with `console.log`/`console.error`. If project policy requires standardized logging, these should use the project logging wrapper and the required `selenium.webdriver.<modulename>` namespace (or the JS-equivalent standard, if different).

## Issue Context
The compliance rule flags direct stdout/stderr diagnostic logging in favor of the project logging system.

## Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[226-246]
- javascript/selenium-webdriver/normalize_bidi_ast.mjs[447-456]

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

---

4. oneOf validation unreachable 🐞 Bug ⚙ Maintainability

Description

projectType() only attaches oneOf to union nodes, but checkSchema() explicitly skips
validating oneOf for unions, so any future ONE_OF[...] entries can silently become invalid
(typos/missing fields) without being caught by the schema gate.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R195-197]

+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)

Evidence

The code adds oneOf only for kind: 'union' (in projectType()), but the validator
short-circuits union oneOf checks (node.kind === 'union' ? false : ...), so the only place
oneOf can appear is the one place it is never validated.

javascript/selenium-webdriver/project_bidi_schema.mjs[106-114]
javascript/selenium-webdriver/project_bidi_schema.mjs[189-199]

Agent prompt

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

### Issue description
`ONE_OF` is wired inconsistently:
- `projectType()` attaches `node.oneOf` only when emitting a `kind: 'union'` type.
- `checkSchema()` iterates `node.oneOf` but intentionally disables validation when `node.kind === 'union'`.

This makes the comment "validated by checkSchema" inaccurate and leaves future `ONE_OF` entries effectively unchecked.

### Issue Context
`ONE_OF` is currently empty, but this bug will matter the first time it’s used.

### Fix options (pick one and make it consistent)
1) **If `oneOf` is meant for record types:** attach `oneOf` to record nodes (where `fields` exist) and keep validation as a fields-existence check.
2) **If `oneOf` is meant for union types:** update `checkSchema()` to validate `oneOf` against union variants (e.g., ensure each named field exists on every variant record, or whatever semantics you intend).

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[106-114]
- javascript/selenium-webdriver/project_bidi_schema.mjs[195-197]

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

---

Results up to commit 5ae00f7

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

1. Alias refs unchecked ✓ Resolved 🐞 Bug ≡ Correctness

Description

checkSchema() never traverses kind: 'alias' nodes, so an alias whose RHS contains a {ref: ...}
can point at a missing type and still pass validation. This breaks the “fail-closed” schema gate by
allowing dangling references to ship undetected.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R166-200]

+/** Fail-closed validation: every ref resolves, and every ONE_OF entry is real. */
+export function checkSchema(schema) {
+  const errors = []
+  const has = (name) => Object.hasOwn(schema.types, name)
+  const refsIn = (node) =>
+    !node
+      ? []
+      : node.ref
+        ? [node.ref]
+        : node.list
+          ? refsIn(node.list)
+          : node.map
+            ? refsIn(node.map)
+            : node.union
+              ? node.union.flatMap(refsIn)
+              : node.record
+                ? node.record.flatMap((f) => refsIn(f.type))
+                : []
+
+  for (const c of [...schema.commands, ...schema.events]) {
+    for (const r of [...refsIn(c.params), ...refsIn(c.result ?? null)])
+      if (!has(r)) errors.push(`${c.method}: unresolved type ${r}`)
+  }
+  for (const [name, node] of Object.entries(schema.types)) {
+    if (node.kind === 'record')
+      for (const f of node.fields)
+        for (const r of refsIn(f.type)) if (!has(r)) errors.push(`${name}.${f.name}: unresolved type ${r}`)
+    if (node.kind === 'union')
+      for (const v of node.variants) if (!has(v)) errors.push(`${name}: unresolved variant ${v}`)
+    for (const field of node.oneOf ?? [])
+      if (node.kind === 'union' ? false : !node.fields?.some((f) => f.name === field))
+        errors.push(`oneOf(${name}): no such field ${field}`)
+  }
+  for (const name of Object.keys(ONE_OF)) if (!has(name)) errors.push(`oneOf: unknown type ${name}`)
+  return errors

Evidence

The projector can emit kind: 'alias' types, but the validator loop only checks record and
union nodes; it never validates refs that appear inside an alias RHS.

javascript/selenium-webdriver/project_bidi_schema.mjs[106-116]
javascript/selenium-webdriver/project_bidi_schema.mjs[166-200]

Agent prompt

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

### Issue description
`checkSchema()` validates refs for command/event params/results, record fields, and union variants, but it does not validate references contained inside `kind: 'alias'` nodes (the alias RHS). This allows unresolved/dangling refs to pass schema validation.

### Issue Context
Aliases are produced by `projectType()` for `variable` defs that are not pure enums and not unions of refs.

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[106-116]
- javascript/selenium-webdriver/project_bidi_schema.mjs[166-200]

### Proposed fix
- In `checkSchema()`, add a branch for `node.kind === 'alias'` that runs `refsIn(node.type)` and errors on any ref that does not exist in `schema.types`.
- Ensure `refsIn` can traverse the alias RHS as-is (it already handles `ref/list/map/union/record` type-ref shapes).

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

---

2. Record map refs unchecked ✓ Resolved 🐞 Bug ≡ Correctness

Description

projectRecord() can emit typed maps via record.map (from * text => T), but checkSchema()
only validates record.fields and ignores record.map. As a result, unresolved refs inside the map
value type won’t be caught by schema validation.

Code

javascript/selenium-webdriver/project_bidi_schema.mjs[R126-198]

+function projectRecord(def) {
+  const record = { kind: 'record', fields: [] }
+  for (const prop of (def.Properties ?? []).flat()) {
+    if (!prop || typeof prop !== 'object') continue
+    // `m === null` is overloaded in this parser: a key-typed entry is a map
+    // (`* text => value`); an anonymous entry is a structural spread; everything
+    // else is just an optional field (the `?` quantifier). Only the first two
+    // are not real fields.
+    if (prop.Occurrence?.m === null && (!prop.Name || prop.Name in PRIMITIVES || prop.Name in PRELUDE)) {
+      if (prop.Name in PRIMITIVES || prop.Name in PRELUDE) {
+        const value = projectRef(prop.Type)
+        if (value.primitive === 'any') record.extensible = true
+        else record.map = value
+      }
+      continue
+    }
+    if (prop.Name) record.fields.push(projectField(prop))
+  }
+  return record
+}
+
+const typeRef = (name) => (name ? { ref: name } : null)
+
+/** Build the flat schema from the raw AST and the existing command/event model. */
+export function projectSchema(ast, model) {
+  const types = {}
+  for (const def of normalizeAst(ast)) if (def?.Name) types[def.Name] = projectType(def)
+
+  const commands = []
+  const events = []
+  for (const [domain, entry] of Object.entries(model)) {
+    for (const c of entry.commands ?? [])
+      commands.push({ domain, method: c.method, name: c.name, params: typeRef(c.params), result: typeRef(c.result) })
+    for (const e of entry.events ?? [])
+      events.push({ domain, method: e.method, name: e.name, params: typeRef(e.params) })
+  }
+
+  return { schemaVersion: 1, commands, events, types }
+}
+
+/** Fail-closed validation: every ref resolves, and every ONE_OF entry is real. */
+export function checkSchema(schema) {
+  const errors = []
+  const has = (name) => Object.hasOwn(schema.types, name)
+  const refsIn = (node) =>
+    !node
+      ? []
+      : node.ref
+        ? [node.ref]
+        : node.list
+          ? refsIn(node.list)
+          : node.map
+            ? refsIn(node.map)
+            : node.union
+              ? node.union.flatMap(refsIn)
+              : node.record
+                ? node.record.flatMap((f) => refsIn(f.type))
+                : []
+
+  for (const c of [...schema.commands, ...schema.events]) {
+    for (const r of [...refsIn(c.params), ...refsIn(c.result ?? null)])
+      if (!has(r)) errors.push(`${c.method}: unresolved type ${r}`)
+  }
+  for (const [name, node] of Object.entries(schema.types)) {
+    if (node.kind === 'record')
+      for (const f of node.fields)
+        for (const r of refsIn(f.type)) if (!has(r)) errors.push(`${name}.${f.name}: unresolved type ${r}`)
+    if (node.kind === 'union')
+      for (const v of node.variants) if (!has(...

[qodo-code-review]

Code review by qodo was updated up to the latest commit 5ae00f7

[qodo-code-review]

Code review by qodo was updated up to the latest commit 421fc38

[Copilot]

Pull request overview

Adds a binding-neutral WebDriver BiDi schema artifact (commands/events/types) generated from shared CDDL AST/model data, plus Bazel wiring and tests that gate schema fidelity against cddl2ts to prevent silent field/type loss across bindings.

Changes:

• Introduces a BiDi AST normalizer and schema projector that emit a flat schema with referential-integrity + completeness validation.
• Extends the Bazel BiDi generation pipeline to produce a shared *_schema.json artifact with cross-binding visibility.
• Adds mocha tests, including a differential “oracle” check comparing the generated schema to cddl2ts output.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
javascript/selenium-webdriver/project_bidi_schema.mjs	Projects normalized AST + model into a flat schema and validates it (integrity + completeness).
javascript/selenium-webdriver/project_bidi_schema_test.mjs	Unit tests for schema projection and validation helpers.
javascript/selenium-webdriver/private/generate_bidi.bzl	Adds schema-generation step to the Bazel BiDi pipeline and exposes schema to other bindings.
javascript/selenium-webdriver/normalize_bidi_ast.mjs	Normalizes raw CDDL AST into canonical forms (hoisted enums/records, flattened unions/composition).
javascript/selenium-webdriver/normalize_bidi_ast_test.mjs	Unit tests for AST normalizer transforms.
javascript/selenium-webdriver/BUILD.bazel	Adds project_bidi_schema_script and a mocha test target for schema tooling + fidelity gate.
javascript/selenium-webdriver/bidi_schema_diff_test.mjs	Differential test comparing generated schema to cddl2ts as an oracle.

[qodo-code-review]

1. Null-only types become unknown 🐞 Bug ≡ Correctness

In projectRef(), null and prelude nil alternatives are filtered out before projecting the
remaining type; if the input type is *only* null/nil, the remaining list is empty and the schema
emits { primitive: 'unknown', nullable: true } instead of a real null type. This can silently
corrupt schema fidelity for any CDDL field/alias defined as exactly null/nil, and the cddl2ts
diff check likely won’t catch it because it only asserts presence of nullability, not the underlying
primitive.

Agent Prompt

### Issue description
`projectRef()` removes `null`/`nil` alternatives via `isNullAlt()`, then projects `entries[0]`. When the type is exactly `null`/`nil` (no non-null alternatives), `entries` becomes empty and `projectEntry(undefined)` returns `{ primitive: 'unknown' }`, producing an incorrect schema node.

### Issue Context
- `PRELUDE.nil` is mapped to `'null'`, and `projectEntry()` can correctly project a `nil` group ref to `{ primitive: 'null' }`.
- But `isNullAlt()` strips it before projection, which is only correct when there is at least one non-null alternative.

### Fix Focus Areas
- javascript/selenium-webdriver/project_bidi_schema.mjs[73-87]

### Suggested fix
- In `projectRef(type)`, after computing `entries`, add a guard:
  - If `entries.length === 0` and `all.length > 0`, return `{ primitive: 'null' }` (and **do not** set `nullable`), because the type is exactly null.
  - Only set `node.nullable = true` when there is at least one non-null alternative (i.e., `entries.length > 0 && entries.length < all.length`).
- Optionally add/extend a unit test to cover `nil`-only and `null`-only projection cases.

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

[qodo-code-review]

Code review by qodo was updated up to the latest commit 7c315df

[diemol]

Shouldn't this be a draft until #17657 gets a 👍?

[titusfortner]

@diemol #17657 is merged. Do you mean #17701?
Even if that ADR is not accepted, this code will make bindings-specific code generation much easier, regardless of how we do it. That ADR is proposing to limit auto-generation to just what this schema provides; the alternative is to inject additional orchestration code into the language-specific generation on top of this schema.

[diemol]

Yes, sorry. I meant that one.

Thank you for clarifying. From that point of view, then we could move forward on this one.