fix(config): SE_NODE_MAX_SESSIONS written to wrong section

[pplulee]

User description

Description

Environment variable SE_NODE_MAX_SESSIONS is not working in the latest version. This was caused by #2942, which has modified config generation but set to write SE_NODE_MAX_SESSIONS to [[node.driver-configuration]] section. However, this variable should be put under [node].

Motivation and Context

To fix the functionality of parameter SE_NODE_MAX_SESSIONS.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Fix SE_NODE_MAX_SESSIONS configuration placement in wrong section

• Move max-sessions setting from driver-configuration to node section

• Add validation for positive integer values

---

Diagram Walkthrough

flowchart LR
  A["SE_NODE_MAX_SESSIONS"] --> B["Validation Check"]
  B --> C["[node] Section"]
  D["[[node.driver-configuration]]"] -.-> E["Removed from here"]
  C --> F["Config File Generated"]

Loading

File Walkthrough

	Relevant files
Bug fix	generate_config Fix max-sessions configuration section placement                  NodeBase/generate_config Move SE_NODE_MAX_SESSIONS from [[node.driver-configuration]] to [node] section Add validation to ensure positive integer values Remove duplicate max-sessions configuration from driver section +4/-4

---

[CLAassistant]

All committers have signed the CLA.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Validation Behavior When SE_NODE_MAX_SESSIONS is unset, empty, zero, or non-numeric, no 'max-sessions' line will be written, relying on defaults. Confirm this is the intended fallback and that downstream components handle absence correctly. # Validate SE_NODE_MAX_SESSIONS is a positive integer if [[ "${SE_NODE_MAX_SESSIONS}" =~ ^[0-9]+$ ]] && [[ "${SE_NODE_MAX_SESSIONS}" -gt 0 ]]; then echo "max-sessions = ${SE_NODE_MAX_SESSIONS}" >>"$FILENAME" fi Quoting Consistency Other numeric values are echoed without quotes. Ensure TOML expects bare integers here and that environment expansion cannot inject whitespace; consider trimming or quoting consistently. echo "max-sessions = ${SE_NODE_MAX_SESSIONS}" >>"$FILENAME" fi

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Warn user about invalid configuration Add a warning message to be displayed when the SE_NODE_MAX_SESSIONS variable is set to an invalid value, instead of silently ignoring it. NodeBase/generate_config [125-128] # Validate SE_NODE_MAX_SESSIONS is a positive integer if [[ "${SE_NODE_MAX_SESSIONS}" =~ ^[0-9]+$ ]] && [[ "${SE_NODE_MAX_SESSIONS}" -gt 0 ]]; then echo "max-sessions = ${SE_NODE_MAX_SESSIONS}" >>"$FILENAME" +elif [[ -n "${SE_NODE_MAX_SESSIONS}" ]]; then + echo "Warning: Invalid value for SE_NODE_MAX_SESSIONS ('${SE_NODE_MAX_SESSIONS}'). It must be a positive integer. Ignoring." >&2 fi Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that silently ignoring invalid configuration is poor user experience, and proposing to add a warning message is a valuable improvement for usability and debugging.	Low
More

[VietND96]

I think you are passing config --detect-drivers true to override the default config, https://www.selenium.dev/documentation/grid/configuration/toml_options/#setting-custom-capabilities-for-matching-specific-nodes

[VietND96]

I just checked, we enforce it always false

echo "detect-drivers = false" >>"$FILENAME"

So, it does not require max-sessions must be under [node]

[pplulee]

I'm using standalone and node, using parameters described here. The variable under driver-configuration is not overriding the session number setting, but putting it under node works as before.

If there's an issue with how I'm using it, please let me know. I've always used it this way before, but problems started occurring after this update.

[VietND96]

Ah ok, I just discovered the issue now. The number of max-sessions enforced is less than or equal cpu cores. Probably at that time, I tested around 2~8 so I assumed it worked, but behavior is not correct

[[node.driver-configuration]]

display-name = "chrome"

stereotype = '{"browserName":"chrome","browserVersion":"140.0","platformName":"Linux","se:containerName":"b1c65e68640d","container:hostname":"b1c65e68640d","goog:chromeOptions":{"binary":"/usr/bin/chromium"}}'

max-sessions = 15


Starting Selenium Grid Standalone...

Appending Selenium option: --tracing false

Tracing is disabled

Using JAVA_OPTS:  -Dwebdriver.remote.enableTracing=false -Dwebdriver.httpclient.version=HTTP_1_1

05:32:10.875 INFO [LoggingOptions.configureLogEncoding] - Using the system default encoding

05:32:10.880 INFO [LoggingOptions.getTracer] - Using null tracer

05:32:11.282 INFO [LoggingOptions.getTracer] - Using null tracer

05:32:11.294 INFO [NodeOptions.getSessionFactories] - Detected 8 available processors

05:32:11.329 INFO [NodeOptions.report] - Adding chrome for {"browserName": "chrome","browserVersion": "140.0","container:hostname": "b1c65e68640d","goog:chromeOptions": {"binary": "\u002fusr\u002fbin\u002fchromium"},"platformName": "linux","se:containerName": "b1c65e68640d","se:deleteSessionOnUi": true,"se:downloadsEnabled": true,"se:noVncPort": 7900,"se:vncEnabled": true} 8 times

[VietND96]

FYI, I am trying to unify the logic in core, so that max-sessions is accepted both under [node] and specific driver config under [[node.driver-configuration]]. Hopefully, it can go with this release, otherwise I will merge this change for workaround and change back in next release.
SeleniumHQ/selenium#16341