#17274: Apply filtering only on merged capabilities

[bhecquet]

🔗 Related Issues

Fixes #17274 where a browserName in stereotype of mobile slot is not filtered

💥 What does this PR do?

This PR restore filtering after capabilities merging, so that session capabilities contains browserName only if no app related capability is present

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

Review Summary by Qodo

Apply capability filtering after merge operation

🐞 Bug fix

Walkthroughs

Description

• Apply filtering after merging capabilities instead of before
• Ensures browserName is removed when app-related capabilities present
• Fixes issue where mobile slot stereotype browserName was not filtered
• Adds test case for Appium app capability precedence over browserName

File Changes

1. java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java 🐞 Bug fix +4/-1

Move filtering to post-merge stage

• Changed merge operation to use unfiltered stereotype instead of pre-filtered
• Moved filtering to occur after merge operation completes
• Added debug print statements to trace capability filtering

java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java

---

2. java/test/org/openqa/selenium/grid/node/relay/RelaySessionFactoryTest.java 🧪 Tests +65/-0

Add test for app capability precedence

• Added new test case requestedAppiumApplicationWinsOverBrowserName
• Tests that appium:app capability from request takes precedence
• Verifies browserName is null when app capability is present
• Validates filtering behavior with mobile slot stereotype

java/test/org/openqa/selenium/grid/node/relay/RelaySessionFactoryTest.java

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. System.out.println capability dumps ✗ 📘 Rule violation ✧ Quality

Description

The PR adds unconditional System.out.println(capabilities) statements in
RelaySessionFactory.apply, which bypass the project logger and can create noisy/uncontrolled
output (including potentially sensitive capability values). This conflicts with the requirement to
add only user-helpful, appropriately scoped logging.

Code

java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[R167-170]

+    capabilities = capabilities.merge(stereotype);
+    System.out.println(capabilities);
+    capabilities = filterRelayCapabilities(capabilities);
+    System.out.println(capabilities);

Evidence

PR Compliance ID 6 requires logging that improves user insight without excessive noise; the added
lines log full capabilities via System.out.println, which is always-on, unstructured, and not
governed by log levels/handlers.

AGENTS.md
java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[167-170]

Agent prompt

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

## Issue description
`RelaySessionFactory.apply` includes `System.out.println(capabilities)` statements that will always print to stdout, bypassing configured logging and potentially leaking/noisily printing capability data.

## Issue Context
This class already uses `LOG.info(...)` and should rely on the configured logging framework with appropriate level (e.g., `LOG.fine`/`LOG.debug`) and redaction if needed.

## Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[167-170]

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

---

2. Response merge restores browserName 🐞 Bug ≡ Correctness

Description

The final session capabilities (mergedCapabilities) are built by merging the downstream response
on top of the already-filtered capabilities, which can reintroduce browserName when the downstream
returns it even for native-app sessions. This contradicts the PR intent of omitting browserName
whenever app-related capabilities are present, because no filtering is applied after the response
merge.

Code

java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[R167-170]

+    capabilities = capabilities.merge(stereotype);
+    System.out.println(capabilities);
+    capabilities = filterRelayCapabilities(capabilities);
+    System.out.println(capabilities);

Evidence

Capabilities.merge is explicitly defined so that capabilities from other override those in
this, and RelaySessionFactory.apply builds mergedCapabilities as
capabilities.merge(responseCaps) before storing it in the ActiveSession. Since filtering is
applied only before sending the NEW_SESSION command, any browserName provided in responseCaps
will override the filtered value, even when app-related capabilities (e.g., appium:app,
appium:appPackage, appium:bundleId) are present.

java/src/org/openqa/selenium/Capabilities.java[98-104]
java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[156-170]
java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[204-219]
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[54-56]
java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java[191-204]

Agent prompt

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

### Issue description
`mergedCapabilities` stored in the created `ActiveSession` can reintroduce `browserName` if the downstream response includes it, because filtering is not re-applied after `capabilities.merge(responseCaps)`.

### Issue Context
- `Capabilities.merge(other)` is defined so that `other` overrides `this`.
- App-related capabilities are detected via `DefaultSlotMatcher.matchConditionToRemoveCapability`.

### Fix Focus Areas
- java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java[204-219]

### Suggested fix
Apply `filterRelayCapabilities` to the final merged capabilities before constructing the session, e.g.:
```java
Capabilities responseCaps = new ImmutableCapabilities((Map<?, ?>) response.getValue());
Capabilities mergedCapabilities = filterRelayCapabilities(capabilities.merge(responseCaps));
```
Optionally add/extend a unit test where the downstream response includes a non-null `browserName` while the request includes `appium:app`, and assert the stored session capabilities omit `browserName`.

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

---

ⓘ The new review experience is currently in Beta. Learn more