[grid] Fix DefaultSlotMatcher matching requests differentiated only by undeclared automationName by diemol · Pull Request #17898 · SeleniumHQ/selenium

diemol · 2026-08-10T21:30:07Z

🔗 Related Issues

💥 What does this PR do?

DefaultSlotMatcher.extensionCapabilitiesMatch() only inspects extension capability names the stereotype declares. If a stereotype declares no extension (:-namespaced) capabilities at all — e.g. a plain browser node — the check short-circuits to true regardless of what the request asks for. This lets a request differentiated solely by an identity extension capability (appium:automationName) match a completely unrelated, non-Appium node, as long as browserName is absent from the request and platformName resolves to the same Platform family.

This adds a new automationNameMatch() check, called alongside the existing extension-capability checks in matches(). It only rejects a match when the stereotype shows no existing Appium-awareness at all (no relevant extension capability, and no non-W3C-compliant platformVersion, which the code already treats as an Appium signal) while the request specifies an automationName. Stereotypes that already show some Appium-awareness (e.g. relay nodes advertising appium:platformVersion but omitting automationName) are left untouched, preserving existing relay-node matching behavior.

🔧 Implementation Notes

The issue proposed an unconditional bidirectional automationName check. That approach was tested against the existing test suite and found to break two passing relay-node tests, where Appium relay nodes intentionally omit automationName while still being valid targets for varied automation sessions. The fix here gates the check on stereotype Appium-awareness instead, using the same platformVersion signal the codebase already relies on elsewhere in this file, so relay-node flexibility is preserved while the reported misroute is closed.

🤖 AI assistance

No substantial AI assistance used
AI assisted (complete below)
- Tool(s): Claude Code
- What was generated: Initial implementation of automationNameMatch() and the two regression tests, drafted from the issue's analysis and root-caused/iterated against the existing test suite to avoid regressing relay-node matching.
- I reviewed all AI output and can explain the change

💡 Additional Considerations

None.

🔄 Types of changes

Bug fix (backwards compatible)

…ionName extensionCapabilitiesMatch() only inspected extension capability names the stereotype declared, so a stereotype declaring none at all (e.g. a plain browser node) matched any requested automationName by default. This let a native-automation request differentiated solely by automationName match an unrelated browser-only node. Add automationNameMatch(), gated on the stereotype showing some existing Appium-awareness (a relevant extension capability, or a non-W3C-compliant platformVersion), so relay-node matching keeps working while a stereotype with no such awareness no longer matches on an undeclared automationName. Fixes #17845 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review · 2026-08-10T21:31:42Z

PR Summary by Qodo

Grid: Prevent slot matching on undeclared appium:automationName

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Prevent plain browser stereotypes matching requests differentiated only by
 appium:automationName.
• Add automationNameMatch() gating based on whether the stereotype is Appium-aware.
• Add regression tests for #17845 and preserve relay-node matching behavior.

Diagram

graph TD
  A["Stereotype + request caps"] --> B["DefaultSlotMatcher.matches()"] --> C{"Extension caps match?"} -->|"pass"| D{"AutomationName allowed?"} -->|"pass"| E{"platformVersion match?"} -->|"pass"| F["Final browser/platform match"] --> G["Match"]
  C -->|"fail"| H["No match"]
  D -->|"fail"| H
  E -->|"fail"| H

High-Level Assessment

The following are alternative approaches to this PR:

1. Unconditional bidirectional automationName match

➕ Very clear rule: stereotype and request must agree on automationName presence/value
➖ Breaks existing relay-node behavior where nodes intentionally omit automationName to stay flexible
➖ Would require either relaxing relay behavior or expanding stereotype declarations across deployments

2. Fold automationName into extensionCapabilitiesMatch()

➕ Keeps all extension-capability matching rules in one place
➕ Avoids adding another top-level check in matches()
➖ Harder to express the required nuance (only enforce when stereotype is not Appium-aware) without complicating extension-cap matching semantics
➖ Blurs the intent between 'match declared extensions' vs 'guard against Appium-only differentiation'

Recommendation: Keep the current gated automationNameMatch() approach: it closes the misroute when a stereotype is clearly non-Appium, while preserving existing relay-node flexibility by treating other Appium signals (e.g., platformVersion or relevant extension caps) as sufficient awareness.

Files changed (2) +65 / -0

Bug fix (1) +24 / -0

DefaultSlotMatcher.javaAdd Appium automationName gating to slot matching +24/-0

Add Appium automationName gating to slot matching

• Adds 'automationNameMatch()' and invokes it during 'matches()' to prevent non-Appium stereotypes from matching requests differentiated only by 'automationName'. The check is intentionally gated so Appium-aware stereotypes (e.g., those signaling via 'platformVersion' or relevant extensions) continue matching even if they omit 'automationName'.

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java

Tests (1) +41 / -0

DefaultSlotMatcherTest.javaRegression tests for automationName-only request matching +41/-0

Regression tests for automationName-only request matching

• Adds a regression test ensuring a plain browser stereotype does not match a request containing only 'appium:automationName'. Adds a companion test verifying that an Appium-aware stereotype (via 'appium:platformVersion') still matches requests that include 'appium:automationName' even if the stereotype omits it.

java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java

qodo-code-review · 2026-08-10T21:34:10Z

Code Review by Qodo

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

1. ~~Appium-aware heuristic too broad~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

automationNameMatch() treats any non-vendor extension capability key (any : capability not in
goog:/moz:/ms:/safari:/se: and not *options*) as “Appium-aware”, so a non-Appium node
advertising an unrelated custom extension capability will bypass the automationName gate. This can
re-allow Appium requests differentiated only by automationName to be considered eligible for the
wrong slot whenever other non-extension checks match.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R203-207]

+                name ->
+                    name.contains("platformVersion")
+                        || (name.contains(":")
+                            && !name.toLowerCase().contains("options")
+                            && EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains)));

Evidence
The new logic marks a stereotype as Appium-aware based on the presence of any : capability not in
a short vendor-prefix ignore list; this is not equivalent to Appium-awareness. The test suite
demonstrates generic non-vendor extension keys like prefixed:cheese are used for non-Appium
extension matching, so treating them as Appium signals will bypass the new automationName protection
for unrelated nodes.
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[48-55]
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[165-178]
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[195-213]
java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java[526-571]

Agent prompt

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

### Issue description
`automationNameMatch()` currently infers “Appium-aware” from *any* non-vendor extension capability key. This is overly broad and causes unrelated custom extension caps to disable the new automationName protection.

### Issue Context
- `EXTENSION_CAPABILITIES_PREFIXES` only lists vendor browser prefixes (`goog:`, `moz:`, `ms:`, `safari:`, `se:`). Many non-Appium nodes can still advertise other extension keys (e.g. `prefixed:cheese` in tests), which should not imply Appium support.
- The intent of this PR is to treat *Appium signals* as Appium-awareness (e.g., `appium:*` keys and the existing non-W3C `platformVersion` signal), not arbitrary extension caps.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[48-55]
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[195-213]
- java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java[526-571]

### Suggested change
Constrain `stereotypeIsAppiumAware` to true only when the stereotype declares Appium-specific keys (e.g. `name.startsWith("appium:")`) and/or the existing `platformVersion` signal (preferably exact key match rather than substring). This preserves relay flexibility while preventing unrelated extension caps from bypassing the new automationName gate.

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

2. Locale-dependent options matching 🐞 Bug ≡ Correctness ⭐ New

Description

requestsAutomationName() uses name.toLowerCase() with the JVM default locale, so in locales like
Turkish the lowercased result may not contain the literal "options" substring for mixed/upper-case
keys. This can miss nested automationName (e.g., in APPiUM:OPTIONS), allowing a non-Appium-aware
stereotype to match when it should be rejected.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R225-227]

+    return name.toLowerCase().contains("options")
+        && value instanceof Map
+        && ((Map<?, ?>) value).containsKey("automationName");

Evidence

The new nested-options detection relies on name.toLowerCase().contains("options"); because the
lowercasing is locale-dependent, the substring check can fail under some default locales, preventing
recognition of options maps that carry automationName.

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[219-227]

Agent prompt

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

## Issue description
`requestsAutomationName` performs a case-insensitive check via `name.toLowerCase().contains("options")`, but `String#toLowerCase()` without an explicit locale is locale-sensitive. This can cause the `"options"` detection to fail under certain default locales (e.g., Turkish), which defeats the intended nested-automationName guard.

## Issue Context
This logic is part of the new automationName matching gate and is meant to reliably detect `automationName` nested inside an `*:options` capability.

## Fix
Change the lowercase conversion to use a locale-stable form (`Locale.ROOT`) (or use a locale-independent case-insensitive check).

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[24-24]
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[223-227]

## (Optional) Regression test
Add a unit test that temporarily sets `Locale.setDefault(new Locale("tr", "TR"))` and verifies `APPiUM:OPTIONS` (or similar mixed-case containing `I`) is still detected.

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

3. ~~Options automationName ignored~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

automationNameMatch only inspects top-level capability keys, so a request that specifies
automationName inside an *:options map (e.g. appium:options) will bypass the new guard and may
still match a non-Appium-aware stereotype. This leaves the reported misrouting hole open for a
capability format that this matcher explicitly treats as “handled by the remote endpoint”.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R206-208]

+    return capabilities.getCapabilityNames().stream()
+        .noneMatch(name -> name.equals("automationName") || name.endsWith(":automationName"));
+  }

Evidence

The new guard only checks top-level capability names; capability names are the keyset of asMap(),
so nested maps (like appium:options) are invisible to this check. The Grid session payload parser
constructs ImmutableCapabilities directly from merged JSON maps, preserving nested maps as values,
and the matcher explicitly filters out *options* extension capabilities elsewhere, so
automationName nested under options bypasses both extension matching and the new automation-name
matching.

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[165-208]
java/src/org/openqa/selenium/Capabilities.java[98-108]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[214-223]

Agent prompt

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

### Issue description
`DefaultSlotMatcher.automationNameMatch()` only checks `capabilities.getCapabilityNames()` (top-level keys) for `automationName` / `*:automationName`. If a client sends automationName nested under an `*:options` capability (e.g. `appium:options: { automationName: ... }`), the key never appears in `getCapabilityNames()`, so the new gate does not block matching against non-Appium-aware stereotypes.

### Issue Context
- This matcher already intentionally skips matching extension capabilities whose name contains `options`, implying these payload shapes are expected to appear and be forwarded.
- `NewSessionPayload` builds `ImmutableCapabilities` directly from merged JSON maps, without flattening nested maps.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[165-208]
- java/src/org/openqa/selenium/Capabilities.java[98-108]
- java/src/org/openqa/selenium/remote/NewSessionPayload.java[214-223]

### Suggested fix
Extend `automationNameMatch()` to also detect automationName when it is provided inside an options map (at least `appium:options`, and ideally any `*:options` where the value is a map). If options contains `automationName` (or `appium:automationName`), treat it as “request specifies automationName” for the purpose of rejecting non-Appium-aware stereotypes.

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

4. ~~automationName key substring match~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

automationNameMatch() rejects requests by checking whether any requested capability key contains
the substring automationName, which can accidentally match unrelated custom keys. This can cause
false negatives (unexpected no-match) for non-Appium requests that happen to use a different
capability name containing that substring.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R211-213]
+    return capabilities.getCapabilityNames().stream()
+        .noneMatch(name -> name.contains("automationName"));
+  }

Evidence

The new check uses substring matching on capability *names* rather than matching a specific
capability key, so any custom capability containing that substring will trigger the Appium-specific
rejection logic.

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[211-213]

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 request-side check uses `name.contains("automationName")`, which is overly broad and can treat unrelated custom capability names as an Appium automationName request.

### Issue Context
This method is intended to special-case Appium’s `automationName` differentiation. Substring matching risks blocking unrelated capabilities like `my:automationNameOverride`.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[211-213]

### Suggested change
Replace substring matching with exact key matching against the intended keys (e.g. `"appium:automationName"` and possibly legacy/non-namespaced `"automationName"` if supported), using `equals` (or `equalsIgnoreCase` only if the project treats capability names case-insensitively elsewhere).

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

View medium (1)

5. matches missing Javadoc 📘 Rule violation ✧ Quality

Description

The public method matches(Capabilities, Capabilities) was modified but still has no Javadoc block
documenting parameters/return value, which violates the requirement for complete Javadoc on changed
public API methods. This reduces API clarity and makes behavior changes harder to audit and consume
correctly.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R78-81]
+    if (!automationNameMatch(stereotype, capabilities)) {
+      return false;
+    }
+

Evidence

PR Compliance ID 330201 requires a Javadoc block for each changed public method, including @param
tags for all parameters and a @return tag when non-void. In the updated source, `public boolean
matches(Capabilities stereotype, Capabilities capabilities) has no /** ... */` Javadoc immediately
above it.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[59-61]

Agent prompt

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

## Issue description
A changed public method (`matches`) lacks a Javadoc block, but the checklist requires complete Javadoc (including `@param` and `@return`) for all changed public API methods.

## Issue Context
`DefaultSlotMatcher.matches(...)` is `public` and was modified in this PR (new `automationNameMatch(...)` gating). The method should be documented so callers and maintainers can understand matching semantics and expectations for the two `Capabilities` parameters.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[59-103]

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

Context

✅ Compliance rules (platform): 18 rules

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous review results

Review updated until commit ae0e1bc ⚖️ Balanced

Results up to commit 8e4d708 ⚖️ Balanced

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

1. ~~Appium-aware heuristic too broad~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

automationNameMatch() treats any non-vendor extension capability key (any : capability not in
goog:/moz:/ms:/safari:/se: and not *options*) as “Appium-aware”, so a non-Appium node
advertising an unrelated custom extension capability will bypass the automationName gate. This can
re-allow Appium requests differentiated only by automationName to be considered eligible for the
wrong slot whenever other non-extension checks match.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R203-207]

+                name ->
+                    name.contains("platformVersion")
+                        || (name.contains(":")
+                            && !name.toLowerCase().contains("options")
+                            && EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains)));

Evidence
The new logic marks a stereotype as Appium-aware based on the presence of any : capability not in
a short vendor-prefix ignore list; this is not equivalent to Appium-awareness. The test suite
demonstrates generic non-vendor extension keys like prefixed:cheese are used for non-Appium
extension matching, so treating them as Appium signals will bypass the new automationName protection
for unrelated nodes.
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[48-55]
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[165-178]
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[195-213]
java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java[526-571]

Agent prompt

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

### Issue description
`automationNameMatch()` currently infers “Appium-aware” from *any* non-vendor extension capability key. This is overly broad and causes unrelated custom extension caps to disable the new automationName protection.

### Issue Context
- `EXTENSION_CAPABILITIES_PREFIXES` only lists vendor browser prefixes (`goog:`, `moz:`, `ms:`, `safari:`, `se:`). Many non-Appium nodes can still advertise other extension keys (e.g. `prefixed:cheese` in tests), which should not imply Appium support.
- The intent of this PR is to treat *Appium signals* as Appium-awareness (e.g., `appium:*` keys and the existing non-W3C `platformVersion` signal), not arbitrary extension caps.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[48-55]
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[195-213]
- java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java[526-571]

### Suggested change
Constrain `stereotypeIsAppiumAware` to true only when the stereotype declares Appium-specific keys (e.g. `name.startsWith("appium:")`) and/or the existing `platformVersion` signal (preferably exact key match rather than substring). This preserves relay flexibility while preventing unrelated extension caps from bypassing the new automationName gate.

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

2. matches missing Javadoc 📘 Rule violation ✧ Quality

Description

The public method matches(Capabilities, Capabilities) was modified but still has no Javadoc block
documenting parameters/return value, which violates the requirement for complete Javadoc on changed
public API methods. This reduces API clarity and makes behavior changes harder to audit and consume
correctly.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R78-81]
+    if (!automationNameMatch(stereotype, capabilities)) {
+      return false;
+    }
+

Evidence

PR Compliance ID 330201 requires a Javadoc block for each changed public method, including @param
tags for all parameters and a @return tag when non-void. In the updated source, `public boolean
matches(Capabilities stereotype, Capabilities capabilities) has no /** ... */` Javadoc immediately
above it.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[59-61]

Agent prompt

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

## Issue description
A changed public method (`matches`) lacks a Javadoc block, but the checklist requires complete Javadoc (including `@param` and `@return`) for all changed public API methods.

## Issue Context
`DefaultSlotMatcher.matches(...)` is `public` and was modified in this PR (new `automationNameMatch(...)` gating). The method should be documented so callers and maintainers can understand matching semantics and expectations for the two `Capabilities` parameters.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[59-103]

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

3. ~~automationName key substring match~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

automationNameMatch() rejects requests by checking whether any requested capability key contains
the substring automationName, which can accidentally match unrelated custom keys. This can cause
false negatives (unexpected no-match) for non-Appium requests that happen to use a different
capability name containing that substring.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R211-213]
+    return capabilities.getCapabilityNames().stream()
+        .noneMatch(name -> name.contains("automationName"));
+  }

Evidence

The new check uses substring matching on capability *names* rather than matching a specific
capability key, so any custom capability containing that substring will trigger the Appium-specific
rejection logic.

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[211-213]

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 request-side check uses `name.contains("automationName")`, which is overly broad and can treat unrelated custom capability names as an Appium automationName request.

### Issue Context
This method is intended to special-case Appium’s `automationName` differentiation. Substring matching risks blocking unrelated capabilities like `my:automationNameOverride`.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[211-213]

### Suggested change
Replace substring matching with exact key matching against the intended keys (e.g. `"appium:automationName"` and possibly legacy/non-namespaced `"automationName"` if supported), using `equals` (or `equalsIgnoreCase` only if the project treats capability names case-insensitively elsewhere).

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

Results up to commit e62d621 ⚖️ Balanced

No changes from previous review

Results up to commit 97ccbef ⚖️ Balanced

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

1. ~~Options automationName ignored~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

automationNameMatch only inspects top-level capability keys, so a request that specifies
automationName inside an *:options map (e.g. appium:options) will bypass the new guard and may
still match a non-Appium-aware stereotype. This leaves the reported misrouting hole open for a
capability format that this matcher explicitly treats as “handled by the remote endpoint”.

Code

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[R206-208]

+    return capabilities.getCapabilityNames().stream()
+        .noneMatch(name -> name.equals("automationName") || name.endsWith(":automationName"));
+  }

Evidence

The new guard only checks top-level capability names; capability names are the keyset of asMap(),
so nested maps (like appium:options) are invisible to this check. The Grid session payload parser
constructs ImmutableCapabilities directly from merged JSON maps, preserving nested maps as values,
and the matcher explicitly filters out *options* extension capabilities elsewhere, so
automationName nested under options bypasses both extension matching and the new automation-name
matching.

java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[165-208]
java/src/org/openqa/selenium/Capabilities.java[98-108]
java/src/org/openqa/selenium/remote/NewSessionPayload.java[214-223]

Agent prompt

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

### Issue description
`DefaultSlotMatcher.automationNameMatch()` only checks `capabilities.getCapabilityNames()` (top-level keys) for `automationName` / `*:automationName`. If a client sends automationName nested under an `*:options` capability (e.g. `appium:options: { automationName: ... }`), the key never appears in `getCapabilityNames()`, so the new gate does not block matching against non-Appium-aware stereotypes.

### Issue Context
- This matcher already intentionally skips matching extension capabilities whose name contains `options`, implying these payload shapes are expected to appear and be forwarded.
- `NewSessionPayload` builds `ImmutableCapabilities` directly from merged JSON maps, without flattening nested maps.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[165-208]
- java/src/org/openqa/selenium/Capabilities.java[98-108]
- java/src/org/openqa/selenium/remote/NewSessionPayload.java[214-223]

### Suggested fix
Extend `automationNameMatch()` to also detect automationName when it is provided inside an options map (at least `appium:options`, and ideally any `*:options` where the value is a map). If options contains `automationName` (or `appium:automationName`), treat it as “request specifies automationName” for the purpose of rejecting non-Appium-aware stereotypes.

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

Results up to commit 0d2c0c8 ⚖️ Balanced

No changes from previous review

Fixes two issues found in CI/review of #17898: - Format / Check Format was failing on ImmutableCapabilities argument wrapping in the new tests; re-ran ./go format to match. - Qodo correctly flagged that treating any non-vendor extension capability as "Appium-aware" was too broad -- a node advertising an unrelated custom extension capability (e.g. prefixed:cheese) would incorrectly bypass the automationName gate. Narrow the signal to appium:-prefixed capabilities and platformVersion specifically, and tighten the automationName check to an exact/suffix match instead of a bare substring. Added a regression test for the narrowed case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review · 2026-08-12T15:03:50Z

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

google-java-format wants the noneMatch lambda collapsed onto one line; the previous manual edit split it across two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review · 2026-08-12T15:16:49Z

Code review by qodo was updated up to the latest commit 97ccbef

Per Qodo review feedback on #17898: document the public matches() method's parameters and return value, since its behavior changed with the new automationName gating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review · 2026-08-12T15:54:05Z

Code review by qodo was updated up to the latest commit 0d2c0c8

…ction

Fixes a Qodo-flagged gap in #17898: automationNameMatch() only inspected top-level capability names, so a request nesting automationName inside an options map (e.g. appium:options) bypassed the new gate entirely, since Capabilities.getCapabilityNames() never flattens nested maps and this matcher already treats *options* capabilities as opaque elsewhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review · 2026-08-13T03:13:41Z

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

selenium-ci added B-grid Everything grid and server related C-java Java Bindings labels Aug 10, 2026

qodo-code-review Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java

Comment thread java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java Outdated

Comment thread java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java

[grid] Fix formatting in DefaultSlotMatcher

97ccbef

google-java-format wants the noneMatch lambda collapsed onto one line; the previous manual edit split it across two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qodo-code-review Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java

[grid] Add Javadoc to DefaultSlotMatcher.matches()

0d2c0c8

Per Qodo review feedback on #17898: document the public matches() method's parameters and return value, since its behavior changed with the new automationName gating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

diemol mentioned this pull request Aug 12, 2026

[🐛 Bug]: DefaultSlotMatcher.extensionCapabilitiesMatch() only checks capabilities the stereotype declares, so a request differentiated solely by an identity extension capability can match an unrelated node #17845

Open

diemol and others added 2 commits August 12, 2026 18:23

Merge branch 'trunk' into fix/17845-slot-matcher-automation-name-dire…

a5511ed

…ction

qodo-code-review Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java

Navigation Menu

Uh oh!

Uh oh!

[grid] Fix DefaultSlotMatcher matching requests differentiated only by undeclared automationName - #17898

[grid] Fix DefaultSlotMatcher matching requests differentiated only by undeclared automationName#17898
diemol wants to merge 6 commits into
trunkfrom
fix/17845-slot-matcher-automation-name-direction

diemol commented Aug 10, 2026

Uh oh!

qodo-code-review Bot commented Aug 10, 2026

Uh oh!

qodo-code-review Bot commented Aug 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

diemol commented Aug 10, 2026

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

🤖 AI assistance

💡 Additional Considerations

🔄 Types of changes

Uh oh!

qodo-code-review Bot commented Aug 10, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

qodo-code-review Bot commented Aug 12, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-code-review Bot commented Aug 10, 2026 •

edited

Loading