[build] Split Rakefile into per-language task files

[titusfortner]

User description

With the CrazyFun code gone (#16972), and the removal of all the outdated Rake code (#16978), what is the point of Rake now?

The idea is to wrap the common features for each binding into its namespace (and an all namespace to run it for all bindings)
The build and release process is different for each language, and which bazel targets are needed for everything is different.
Rather than remembering and referencing each different bazel command in the CI and shell scripts to do file manipulation,
we're using bundled jruby to orchestrate this behavior outside the workflow files.

For our CI, that means being able to do all:release, and follow up with java:release, etc depending on what is needed.

To make it more clear and obvious, this PR is going to break up the Rakefile so that each binding gets its own file
to define the features, so it is easy to see which file/language you're looking at.
Another advantage to this is that we can load these with different namespaces so they are effectively aliased.
No more needing to javascript --> node conversions in workflows.

💥 What does this PR do?

Split the monolithic Rakefile (~1600 lines) into per-language task files.

New Files:

File	Purpose
rake_tasks/common.rb	Shared SeleniumRake module (version bumping, changelog generation, package verification)
rake_tasks/java.rake	Java tasks (build, client, package, release, verify, docs, changelogs, version, lint, install)
rake_tasks/python.rake	Python tasks (build, release, verify, docs, changelogs, version, lint, install)
rake_tasks/ruby.rake	Ruby tasks (build, local_dev, release, verify, docs, pin, update, changelogs, version, lint, install)
rake_tasks/node.rake	JavaScript/Node tasks (build, pin, update, release, verify, docs, changelogs, version, lint, install)
rake_tasks/dotnet.rake	.NET tasks (build, package, release, verify, docs, changelogs, version, lint, install)
rake_tasks/rust.rake	Rust/Selenium Manager tasks (build, update, pin, changelogs, version, lint)
rake_tasks/grid.rake	Grid tasks (build, package, release)
rake_tasks/appium.rake	iOS driver atoms task for Appium

Namespace Loading:

namespace(:java) { load 'rake_tasks/java.rake' }
namespace(:rb) { load 'rake_tasks/ruby.rake' }
namespace(:ruby) { load 'rake_tasks/ruby.rake' }    # alias
namespace(:py) { load 'rake_tasks/python.rake' }
namespace(:python) { load 'rake_tasks/python.rake' }  # alias
namespace(:node) { load 'rake_tasks/node.rake' }
namespace(:js) { load 'rake_tasks/node.rake' }        # alias
namespace(:javascript) { load 'rake_tasks/node.rake' } # alias
namespace(:dotnet) { load 'rake_tasks/dotnet.rake' }
namespace(:rust) { load 'rake_tasks/rust.rake' }
namespace(:grid) { load 'rake_tasks/grid.rake' }

🔧 Implementation Notes

• Each .rake file defines tasks without namespace wrapper (namespace applied when loaded)
• Aliases maintain backward compatibility (rb/ruby, py/python, node/js/javascript)
• Legacy task aliases preserved for CI (e.g., publish-maven → java:release)
• all:* aggregation tasks iterate over language namespaces
• New tasks added: *:lint, *:check_credentials, *:install

💡 Additional Considerations

We can greatly simplify the CI workflows in several places

Verification:

./go -T                    # All tasks listed
./go java:build            # Java builds
./go rb:build              # Ruby builds
./go python:build          # Alias works
./go all:build             # All languages
./go all:lint              # All linters
./go lint                  # Full lint (includes buildifier, actionlint)

🔄 Types of changes

• Cleanup (formatting, renaming)
• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement

---

Description

• Refactored monolithic Rakefile (~1600 lines) into modular per-language task files for improved maintainability and clarity

• Created 9 new language-specific rake files: java.rake, python.rake, ruby.rake, node.rake, dotnet.rake, rust.rake, grid.rake, appium.rake, and shared common.rb

• Implemented namespace loading with language aliases (rb/ruby, py/python, node/js/javascript) for backward compatibility

• Extracted language-specific build, release, and package automation tasks:

  • Java: Maven publishing with Sonatype API integration
  • Python: PyPI wheel/sdist building and publishing
  • Ruby: RubyGems gem building and publishing
  • Node.js: npm package building and publishing with pnpm dependency management
  • .NET: NuGet package building and publishing to GitHub Packages
  • Rust: Selenium Manager build and Cargo dependency pinning via Bazel
  • Grid: Server JAR and ZIP artifact packaging for GitHub releases
  • Appium: iOS driver atoms building for Appium integration

• Created shared SeleniumRake utility module for common operations: version bumping, changelog generation, and package verification

• Added new top-level tasks: prep_release, lint, update_cdp, and authors

• Preserved legacy task aliases for CI compatibility (e.g., publish-maven → java:release)

• Simplified main Rakefile to focus on global tasks, bazel rules, and namespace aggregation

---

Diagram Walkthrough

flowchart LR
  A["Monolithic Rakefile<br/>~1600 lines"] -->|"Extract language-specific tasks"| B["Language-specific<br/>rake files"]
  B --> C["java.rake"]
  B --> D["python.rake"]
  B --> E["ruby.rake"]
  B --> F["node.rake"]
  B --> G["dotnet.rake"]
  B --> H["rust.rake"]
  B --> I["grid.rake"]
  B --> J["appium.rake"]
  K["common.rb<br/>Shared utilities"] -->|"Used by all"| B
  L["Rakefile<br/>Refactored"] -->|"Loads with namespaces"| B
  L -->|"Provides aliases"| M["rb/ruby, py/python<br/>node/js/javascript"]

Loading

File Walkthrough

Relevant files

Enhancement

10 files java.rake Java language-specific Rake tasks and release automation  rake_tasks/java.rake Extracted Java-specific build, release, and package tasks from monolithic Rakefile Implements Maven publishing workflow with Sonatype API integration for staged releases Includes credential validation, version management, documentation generation, and dependency pinning Provides linting via google-java-format and changelog generation from git history +381/-0  ruby.rake Ruby language-specific Rake tasks and gem release workflow rake_tasks/ruby.rake Extracted Ruby-specific gem building, release, and package tasks Implements RubyGems publishing with credential management and local gem installation Includes dependency pinning via Gemfile.lock checksums, documentation generation, and changelog updates Provides linting via rubocop and steep type checker +210/-0  python.rake Python language-specific Rake tasks and PyPI release automation rake_tasks/python.rake Extracted Python-specific wheel/sdist building and PyPI release tasks Implements PyPI publishing with credential validation and nightly version support Includes local development file copying, documentation generation via Sphinx, and changelog management Provides linting via ruff (check and format) +163/-0  dotnet.rake .NET language-specific Rake tasks and NuGet release workflow rake_tasks/dotnet.rake Extracted .NET-specific nupkg building and NuGet release tasks Implements NuGet publishing with support for both standard and nightly releases to GitHub Packages Includes dependency management via Paket, documentation generation, and changelog updates Provides credential validation and local package installation +121/-0  node.rake Node.js language-specific Rake tasks and npm release automation rake_tasks/node.rake Extracted Node.js/JavaScript-specific npm package building and release tasks Implements npm registry publishing with dry-run support and credential management Includes pnpm-based dependency pinning and updating, documentation generation, and changelog management Provides linting via prettier code formatter +133/-0  common.rb Shared utilities module for cross-language release operations rake_tasks/common.rb Shared utility module SeleniumRake for common release and build operations Provides version bumping logic supporting nightly, snapshot, and standard releases across all languages Implements changelog generation from git history with language-specific formatting Includes package publication verification via HTTP requests to package registries +76/-0    rust.rake Rust language-specific Rake tasks for Selenium Manager      rake_tasks/rust.rake Extracted Rust/Selenium Manager-specific build and version management tasks Implements Cargo dependency pinning via Bazel integration with CARGO_BAZEL_REPIN Includes version updates with special handling for 0.4.x beta versioning pattern Provides linting via rustfmt and changelog generation +62/-0    grid.rake Grid server-specific Rake tasks and packaging workflow      rake_tasks/grid.rake Extracted Grid server-specific build, package, and release tasks Implements packaging of executable Grid JAR and server ZIP artifacts for GitHub releases Supports both standard and nightly release configurations Reuses java_version helper for consistent versioning +29/-0    appium.rake Appium iOS driver atoms build task definition                        rake_tasks/appium.rake Extracted iOS driver atoms building task for Appium integration Defines Bazel build dependencies for 20 JavaScript atom fragments targeting iOS platform Minimal task wrapper that delegates to Bazel for actual compilation +32/-0    Rakefile Split monolithic Rakefile into modular per-language task files Rakefile Refactored monolithic Rakefile (~1600 lines) into modular per-language task files loaded within namespaces Removed language-specific task definitions and moved them to separate rake files (rake_tasks/*.rake) Replaced global @git variable with SeleniumRake.git class variable for better encapsulation Added namespace loading for java, ruby, python, node, dotnet, rust, grid, and appium with language aliases (rb/ruby, py/python, node/js/javascript) Simplified main Rakefile to focus on global tasks, bazel rules, and namespace aggregation Added new top-level tasks: prep_release, lint, update_cdp, and authors Preserved legacy task aliases for backward compatibility (e.g., publish-maven → java:release) Consolidated all:lint task to iterate over language-specific linters with skip support +139/-1255

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Command injection Description: Potential command injection risk: update_changelog builds a shell command string (git log #{tag}...HEAD ... -- #{path}) and executes it via backticks, so if tag or path can be influenced (e.g., via unexpected tag names or future task changes passing user-controlled values), it could allow arbitrary shell execution; prefer Open3.capture3/system with argv array and strict validation/escaping of tag and path. common.rb [52-54] Referred Code command = "git log #{tag}...HEAD --pretty=format:'%s' --reverse -- #{path}" log = `#{command}`
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit context: Release/publish actions are only printed via unstructured puts/warn without user identity, timestamp, or explicit success/failure fields, so it may be impossible to reconstruct who performed a release and its outcome from these tasks alone. Referred Code desc 'Deploy all jars to Maven (pass deployment_id to retry a failed publish)' task :release do |_task, arguments| args = arguments.to_a nightly = args.delete('nightly') deployment_id = args.first Rake::Task['java:check_credentials'].invoke(*(nightly ? ['nightly'] : [])) ENV['MAVEN_USER'] ||= ENV.fetch('SEL_M2_USER', nil) ENV['MAVEN_PASSWORD'] ||= ENV.fetch('SEL_M2_PASS', nil) token = sonatype_auth_token # Retry mode: just poll and publish an existing deployment if deployment_id puts "Retrying deployment: #{deployment_id}" poll_and_publish_deployment(deployment_id, token) return end repo_domain = 'central.sonatype.com' repo = if nightly ... (clipped 25 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Mixed error strategy: Network/publish flows mix raise, warn+exit(1), and broad rescue StandardError retries which may produce inconsistent behavior and may not always propagate actionable context in a consistent, machine-consumable way. Referred Code def trigger_sonatype_validation(token) puts 'Triggering Sonatype validation...' uri = URI('https://ossrh-staging-api.central.sonatype.com/manual/upload/defaultRepository/org.seleniumhq') req = Net::HTTP::Post.new(uri) req['Authorization'] = "Basic #{token}" req['Accept'] = '*/*' req['Content-Length'] = '0' begin res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true, open_timeout: 10, read_timeout: 180) do |http| http.request(req) end rescue Net::ReadTimeout, Net::OpenTimeout => e warn <<~MSG Request timed out waiting for deployment ID. The deployment may still have been created on the server. Check https://central.sonatype.com/publishing/deployments for pending deployments, then run: ./go java:release <deployment_id> MSG ... (clipped 50 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: HTTP body in error: Error messages include raw HTTP response bodies (e.g., Sonatype API errors), which could leak internal details depending on what the remote service returns and where these task outputs are surfaced (CI logs/user-visible logs). Referred Code def sonatype_api_post(url, token) uri = URI(url) req = Net::HTTP::Post.new(uri) req['Authorization'] = "Basic #{token}" res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) { |http| http.request(req) } raise "Sonatype API error (#{res.code}): #{res.body}" unless res.is_a?(Net::HTTPSuccess) res.body.to_s.empty? ? {} : JSON.parse(res.body) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: External input parsing: Credential/config parsing and file writes rely on simple string matching (e.g., scanning /.m2/settings.xml, /.pypirc, ~/.npmrc) without robust validation, so correctness and security assumptions depend on file format and surrounding environment not visible in the diff. Referred Code def setup_gem_credentials gem_dir = File.join(Dir.home, '.gem') credentials = File.join(gem_dir, 'credentials') return if File.exist?(credentials) && File.read(credentials).include?(':rubygems_api_key:') token = ENV.fetch('GEM_HOST_API_KEY', nil) if token.nil? || token.empty? raise 'Missing RubyGems credentials: set GEM_HOST_API_KEY or configure ~/.gem/credentials' end FileUtils.mkdir_p(gem_dir) if File.exist?(credentials) File.open(credentials, 'a') { |f| f.puts(":rubygems_api_key: #{token}") } else File.write(credentials, ":rubygems_api_key: #{token}\n") end File.chmod(0o600, credentials) end Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 40108b9

Category	Suggestion	Impact
Possible issue	Don’t misparse non-language flags Fix the argument parsing in the all:lint task to correctly handle flags. The current implementation incorrectly treats any argument starting with - as a language to skip, which will break with flags like --verbose. Rakefile [260-275] task :lint do |_task, arguments| all_langs = %w[java py rb node rust] - skip = arguments.to_a.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } + skip = arguments.to_a + .select { |a| a.start_with?('-') && !a.start_with?('--') } + .map { |a| a.delete_prefix('-') } invalid = skip - all_langs raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? langs = all_langs - skip failures = [] langs.each do |lang| puts "Linting #{lang}..." Rake::Task["#{lang}:lint"].invoke rescue StandardError => e failures << "#{lang}: #{e.message}" end raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? end Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a bug in the argument parsing logic that would cause the task to fail with valid command-line flags like --verbose, and provides a correct fix.	Medium
	Prevent nil version results Add input validation and explicit error handling to the updated_version method to prevent it from returning nil for unexpected version formats. rake_tasks/common.rb [21-33] def self.updated_version(current, desired = nil, nightly = nil) if !desired.nil? && desired != 'nightly' # If desired is present, return full 3 digit version - desired.split('.').tap { |v| v << 0 while v.size < 3 }.join('.') - elsif current.split(/\.|-/).size > 3 + return desired.split('.').tap { |v| v << 0 while v.size < 3 }.join('.') + end + + parts = current.to_s.split(/\.|-/) + + if parts.size > 3 + raise 'Missing nightly suffix for prerelease bump' if nightly.nil? || nightly.empty? + # if current version is already nightly, just need to bump it; this will be noop for some languages pattern = /-?\.?(nightly|SNAPSHOT|dev|\d{12})\d*$/ - current.gsub(pattern, nightly) - elsif current.split(/\.|-/).size == 3 + return current.gsub(pattern, nightly) + end + + if parts.size == 3 + raise 'Missing nightly suffix for nightly bump' if nightly.nil? || nightly.empty? + # if current version is not nightly, need to bump the version and make nightly - "#{current.split(/\.|-/).tap { |i| (i[1] = i[1].to_i + 1) && (i[2] = 0) }.join('.')}#{nightly}" + bumped = parts.tap { |i| (i[1] = i[1].to_i + 1) && (i[2] = 0) }.join('.') + return "#{bumped}#{nightly}" end + + raise "Unrecognized version format: #{current}" end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a scenario where the updated_version method could return nil, leading to runtime errors, and proposes adding validation to fail fast with a clear error message, which improves robustness.	Medium
	Validate lockfile and checksum block Add checks to the rb:pin task to validate the format of Gemfile.lock and MODULE.bazel, raising an error if parsing fails, to prevent silent failures or NoMethodError exceptions. rake_tasks/ruby.rake [149-196] lock_content = File.read(gemfile_lock) gem_section = lock_content[/GEM\n\s+remote:.*?\n\s+specs:\n(.*?)(?=\n[A-Z]|\Z)/m, 1] +raise "Could not parse GEM section from #{gemfile_lock}" if gem_section.nil? || gem_section.empty? + gems = gem_section.scan(/^ ([a-zA-Z0-9_-]+) \(([^)]+)\)$/) +raise "No gems found in GEM specs section of #{gemfile_lock}" if gems.empty? + needed_gems = gems.map { |name, version| "#{name}-#{version}" } ... +unless module_content.match?(/ gem_checksums = \{[^}]+\},/m) + raise "Could not find gem_checksums block in #{module_bazel}" +end + new_content = module_content.sub(/ gem_checksums = \{[^}]+\},/m, formatted) File.write(module_bazel, new_content) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that parsing external files without validation can lead to nil errors or silent failures, and the proposed checks make the rb:pin task more robust and its failures more explicit.	Medium
	Prevent double-loading task files Avoid reloading the same Rake file for namespace aliases. Instead, load each file once and create aliases by defining tasks that delegate to the canonical ones. Rakefile [33-39] +namespace(:java) { load 'rake_tasks/java.rake' } namespace(:rb) { load 'rake_tasks/ruby.rake' } -namespace(:ruby) { load 'rake_tasks/ruby.rake' } namespace(:py) { load 'rake_tasks/python.rake' } -namespace(:python) { load 'rake_tasks/python.rake' } # alias namespace(:node) { load 'rake_tasks/node.rake' } -namespace(:js) { load 'rake_tasks/node.rake' } # alias -namespace(:javascript) { load 'rake_tasks/node.rake' } # alias +namespace(:dotnet) { load 'rake_tasks/dotnet.rake' } +namespace(:rust) { load 'rake_tasks/rust.rake' } +namespace(:grid) { load 'rake_tasks/grid.rake' } +namespace(:bazel) { load 'rake_tasks/bazel.rake' } +namespace(:appium) { load 'rake_tasks/appium.rake' } +# Aliases (delegate instead of re-loading files) +namespace(:ruby) { task(:build => :'rb:build'); task(:lint => :'rb:lint'); task(:docs => :'rb:docs'); task(:update => :'rb:update') } +namespace(:python) { task(:build => :'py:build'); task(:lint => :'py:lint'); task(:docs => :'py:docs'); task(:update => :'py:update') } +namespace(:js) { task(:build => :'node:build'); task(:lint => :'node:lint'); task(:docs => :'node:docs'); task(:update => :'node:update') } +namespace(:javascript) { task(:build => :'node:build'); task(:lint => :'node:lint'); task(:docs => :'node:docs'); task(:update => :'node:update') } + Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that loading the same file for aliases is bad practice and can cause warnings or errors, proposing a more robust delegation pattern which improves maintainability.	Medium
	Make wheel install deterministic Modify the py:install task to use Dir.glob to find the wheel file path instead of relying on shell globbing, ensuring the command is more reliable across different platforms. rake_tasks/python.rake [118-122] desc 'Install Python wheel locally' task :install do Bazel.execute('build', [], '//py:selenium-wheel') - sh 'pip install bazel-bin/py/selenium-*.whl' + wheels = Dir.glob('bazel-bin/py/selenium-*.whl') + raise "Expected 1 wheel, found #{wheels.size}: #{wheels.join(', ')}" unless wheels.size == 1 + + sh 'pip', 'install', wheels.first end Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that shell globbing can be unreliable and proposes a more robust, platform-independent way to find the wheel file, which improves the reliability of the :install task.	Low
General	Ensure lint tasks always run Ensure language-specific lint tasks can be re-run within the same process by calling reenable on each task before invoking it. This prevents them from being skipped on subsequent calls. Rakefile [268-273] langs.each do |lang| puts "Linting #{lang}..." - Rake::Task["#{lang}:lint"].invoke + t = Rake::Task["#{lang}:lint"] + t.reenable + t.invoke rescue StandardError => e failures << "#{lang}: #{e.message}" end Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out a potential issue where tasks might not re-run if invoked multiple times, improving the task's reliability by using reenable before invoke.	Low
Learned best practice	Fail fast on missing version If SE_VERSION is not found, this returns nil and later URL/version uses will fail unclearly; raise a descriptive error and validate the extracted value is non-blank. rake_tasks/python.rake [5-9] def python_version File.foreach('py/BUILD.bazel') do |line| - return line.split('=').last.strip.tr('"', '') if line.include?('SE_VERSION') + next unless line.include?('SE_VERSION') + + v = line.split('=').last&.strip&.tr('"', '') + return v unless v.nil? || v.empty? end + + raise "Could not determine Python version from py/BUILD.bazel (missing/blank SE_VERSION)" end Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., file parsing) so missing/blank values fail fast with a clear error.	Low
	Reenable tasks invoked twice Rake tasks only run once per process, so the second invoke is a no-op; call reenable before the second invocation to ensure dependencies are re-pinned after edits. rake_tasks/java.rake [337-361] task :update do puts 'Updating Maven dependencies' # Make sure things are in a good state to start with Rake::Task['java:pin'].invoke ... File.write(file_path, content) - Rake::Task['java:pin'].invoke + t = Rake::Task['java:pin'] + t.reenable + t.invoke end [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Make lifecycle-sensitive task invocations robust by ensuring idempotent/repeatable execution (reenable tasks when invoking multiple times).	Low
Update

---

Previous suggestions

✅ Suggestions up to commit ac29f0c

Category	Suggestion	Impact
Possible issue	Fix retry-loop exception handling Fix a syntax error in the retry loop's exception handling by wrapping the loop's body in a begin...rescue...end block. rake_tasks/java.rake [144-165] def poll_and_publish_deployment(deployment_id, token) encoded_id = URI.encode_www_form_component(deployment_id.strip) status = {} max_attempts = 60 delay = 5 max_attempts.times do |attempt| - status = sonatype_api_post("https://central.sonatype.com/api/v1/publisher/status?id=#{encoded_id}", token) - state = status['deploymentState'] - puts "Deployment state: #{state}" + begin + status = sonatype_api_post("https://central.sonatype.com/api/v1/publisher/status?id=#{encoded_id}", token) + state = status['deploymentState'] + puts "Deployment state: #{state}" - case state - when 'VALIDATED', 'PUBLISHED' then break - when 'FAILED' then raise "Deployment failed: #{status['errors']}" + case state + when 'VALIDATED', 'PUBLISHED' then break + when 'FAILED' then raise "Deployment failed: #{status['errors']}" + end + + sleep(delay) unless attempt == max_attempts - 1 + rescue StandardError => e + raise if e.message.start_with?('Deployment failed') + + warn "API error (attempt #{attempt + 1}/#{max_attempts}): #{e.message}" + sleep(delay) unless attempt == max_attempts - 1 end - sleep(delay) unless attempt == max_attempts - 1 - rescue StandardError => e - raise if e.message.start_with?('Deployment failed') - - warn "API error (attempt #{attempt + 1}/#{max_attempts}): #{e.message}" - sleep(delay) unless attempt == max_attempts - 1 end Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a syntax error in the poll_and_publish_deployment method where rescue is improperly used with a do...end block, which would prevent the script from running.	High
	Parse JSON for package version Robustly extract the package version by parsing package.json instead of using a simple string match, which could return an incorrect value. rake_tasks/node.rake [3-7] def node_version - File.foreach('javascript/selenium-webdriver/package.json') do |line| - return line.split(':').last.strip.tr('",', '') if line.include?('version') - end + json = JSON.parse(File.read('javascript/selenium-webdriver/package.json')) + json.fetch('version') end Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that the current line-based parsing of package.json is fragile and could extract an incorrect version, proposing a robust fix using JSON parsing.	Medium
	Ensure credential checks rerun Use t.reenable before t.invoke within the all:check_credentials task to ensure that per-language credential checks can be run multiple times within the same process. Rakefile [226-235] task :check_credentials do |_task, arguments| args = arguments.to_a failures = [] %w[java py rb dotnet node].each do |lang| - Rake::Task["#{lang}:check_credentials"].invoke(*args) + t = Rake::Task["#{lang}:check_credentials"] + t.reenable + t.invoke(*args) rescue StandardError => e failures << "#{lang}: #{e.message}" end raise "Credential check failed:\n#{failures.join("\n")}" unless failures.empty? end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that Rake::Task#invoke only runs once and proposes using reenable to ensure the credential checks can be re-executed, improving the task's robustness.	Medium
	✅ Include nested task files Suggestion Impact: The commit modified the same filegroup to include rake_tasks via a glob, but it used non-recursive patterns (rake_tasks/*.rake and rake_tasks/*.rb) rather than the suggested recursive ** patterns. code diff: @@ -25,7 +25,7 @@ name = "rakefile", srcs = [ "Rakefile", - ], + ] + glob(["rake_tasks/*.rake", "rake_tasks/*.rb"]), visibility = ["//rb:__subpackages__"], Update the glob in the rakefile filegroup to be recursive ("rake_tasks//*.rake") to prevent future build failures if task files are moved into subdirectories.** BUILD.bazel [24-30] filegroup( name = "rakefile", srcs = [ "Rakefile", - ] + glob(["rake_tasks/*.rake", "rake_tasks/*.rb"]), + ] + glob(["rake_tasks/**/*.rake", "rake_tasks/**/*.rb"]), visibility = ["//rb:__subpackages__"], ) Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the glob pattern is not recursive and proposes a change to rake_tasks/**/*.rake to make the build more robust against future file organization changes.	Low
General	Add timeouts to HTTP checks Add open_timeout and read_timeout to the HTTP request in verify_package_published to prevent tasks from hanging on network stalls. rake_tasks/common.rb [68-75] def self.verify_package_published(url) puts "Verifying #{url}..." uri = URI(url) - res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: uri.scheme == 'https') { |http| http.request(Net::HTTP::Get.new(uri)) } + res = Net::HTTP.start(uri.hostname, uri.port, + use_ssl: uri.scheme == 'https', + open_timeout: 10, + read_timeout: 30) do |http| + http.request(Net::HTTP::Get.new(uri)) + end raise "Package not published: #{url}" unless res.is_a?(Net::HTTPSuccess) puts 'Verified!' end Suggestion importance[1-10]: 7 __ Why: The suggestion improves the robustness of the verify_package_published method by adding network timeouts, preventing CI jobs from hanging indefinitely on network issues.	Medium
Learned best practice	Harden API calls and parsing Use use_ssl based on the URL scheme, add timeouts, and handle non-JSON/invalid responses to avoid unexpected exceptions during release automation. rake_tasks/java.rake [74-83] def sonatype_api_post(url, token) - uri = URI(url) + uri = URI.parse(url.to_s.strip) req = Net::HTTP::Post.new(uri) req['Authorization'] = "Basic #{token}" - res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: true) { |http| http.request(req) } + res = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == 'https', open_timeout: 10, read_timeout: 30) do |http| + http.request(req) + end raise "Sonatype API error (#{res.code}): #{res.body}" unless res.is_a?(Net::HTTPSuccess) - res.body.to_s.empty? ? {} : JSON.parse(res.body) + body = res.body.to_s + return {} if body.strip.empty? + + JSON.parse(body) +rescue JSON::ParserError => e + raise "Sonatype API returned non-JSON response: #{e.message}" end Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add validation/guards at integration boundaries (e.g., URLs/HTTP/JSON parsing) before use, and fail with clear errors.	Low

✅ Suggestions up to commit a2ac5b7

Category	Suggestion	Impact
Incremental [*]	Normalize task arguments to strings Convert Rake task arguments to strings using map(&:to_s) before processing to prevent runtime errors from non-string values. Rakefile [260-261] all_langs = %w[java py rb node rust] -skip = arguments.to_a.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } +args = arguments.to_a.map(&:to_s) +skip = args.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } Suggestion importance[1-10]: 7 __ Why: This is a valid defensive programming suggestion that prevents a potential NoMethodError if the Rake task is invoked programmatically with non-string arguments, improving the script's robustness.	Medium
	Use absolute path for loading Make the load path absolute using File.expand_path to ensure it works correctly regardless of the current working directory. Rakefile [43] -namespace(:appium) { load 'rake_tasks/appium.rake' } +namespace(:appium) { load File.expand_path('rake_tasks/appium.rake', __dir__) } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that using a relative path with load can be fragile and makes the build script more robust by using an absolute path, which is a good practice.	Low
Possible issue	Avoid reading wrong Maven credentials Improve the parsing of settings.xml to ensure credentials are only read from the correct block by stopping when is found. rake_tasks/java.rake [85-105] def read_m2_user_pass settings_path = File.join(Dir.home, '.m2', 'settings.xml') unless File.exist?(settings_path) warn "Maven settings file not found at #{settings_path}" return end puts 'Maven environment variables not set, inspecting ~/.m2/settings.xml.' settings = File.read(settings_path) - found_section = false + in_central_server = false + settings.each_line do |line| - if !found_section - found_section = line.include? '<id>central</id>' + if !in_central_server + in_central_server = line.include?('<id>central</id>') + next + end + + if line.include?('</server>') + break elsif line.include?('<username>') ENV['MAVEN_USER'] = line[%r{<username>(.*?)</username>}, 1] elsif line.include?('<password>') ENV['MAVEN_PASSWORD'] = line[%r{<password>(.*?)</password>}, 1] end + break if ENV['MAVEN_PASSWORD'] && ENV['MAVEN_USER'] end end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential bug where incorrect credentials could be parsed from settings.xml and provides a more robust implementation to prevent this.	Medium
	Avoid hard dependency on git Wrap the Git.open call in a begin/rescue block to prevent crashes in non-git environments, setting SeleniumRake.git to nil on failure. Rakefile [29] -SeleniumRake.git = Git.open(__dir__) +begin + SeleniumRake.git = Git.open(__dir__) +rescue StandardError + SeleniumRake.git = nil +end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that Git.open can fail and crash the Rakefile, making it more robust by handling the exception gracefully.	Medium
	Make git staging best-effort Use the safe navigation operator (&.) for all calls to SeleniumRake.git.add to prevent NoMethodError if git is not available. Rakefile [63] -SeleniumRake.git.add('common/repositories.bzl') +SeleniumRake.git&.add('common/repositories.bzl') Suggestion importance[1-10]: 7 __ Why: This suggestion correctly points out that if git initialization fails, calls to SeleniumRake.git.add will raise an error, and proposes using the safe navigation operator (&.) to prevent crashes.	Medium
	Handle missing outdated command output Add a check to ensure the output from the Bazel command is not nil or empty before attempting to parse it, preventing a potential crash. rake_tasks/java.rake [344-349] output = nil Bazel.execute('run', [], '@maven//:outdated') do |out| output = out end +if output.nil? || output.strip.empty? + raise 'Failed to fetch Maven outdated report (no output returned)' +end + versions = output.scan(/(\S+) \[\S+ -> (\S+)\]/).to_h Suggestion importance[1-10]: 6 __ Why: The suggestion correctly adds a guard clause to prevent a NoMethodError if the Bazel.execute command returns no output, improving the script's robustness.	Low
	Make local file copying robust Make the local file copying more robust by creating the destination directory if it doesn't exist and raising an error if a source atom file is missing. rake_tasks/python.rake [92-94] +FileUtils.mkdir_p("#{lib_path}/remote") + %w[getAttribute.js isDisplayed.js findElements.js].each do |atom| - FileUtils.cp("#{bazel_bin}/remote/#{atom}", "#{lib_path}/remote/#{atom}") + src = "#{bazel_bin}/remote/#{atom}" + dest = "#{lib_path}/remote/#{atom}" + raise "Missing generated atom: #{src}" unless File.exist?(src) + + FileUtils.cp(src, dest) end Suggestion importance[1-10]: 5 __ Why: The suggestion improves the robustness of the file copy operation by ensuring the destination directory exists and that source files are present, preventing potential script failures.	Low
General	✅ Forward linter task arguments Suggestion Impact: The commit rewrote the `all:lint` task to split incoming arguments into skip flags (those starting with "-") and forwardable args, then invoked each `#{lang}:lint` task with `*forward_args`. This implements the requested argument forwarding behavior while preserving skip-language functionality. code diff: + desc 'Run linters for all languages (skip with: ./go all:lint -rb -rust)' + task :lint do |_task, arguments| + all_langs = %w[java py rb node rust] + args = arguments.to_a + skip_flags, forward_args = args.partition { |a| a.start_with?('-') } + skip = skip_flags.map { |a| a.delete_prefix('-') } + invalid = skip - all_langs + raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? + + langs = all_langs - skip + failures = [] + langs.each do |lang| + puts "Linting #{lang}..." + Rake::Task["#{lang}:lint"].invoke(*forward_args) + rescue StandardError => e + failures << "#{lang}: #{e.message}" end - - Bazel.execute('run', [], '//py:mypy') - Bazel.execute('run', [], '//py:ruff') - Bazel.execute('run', [], '//rb:steep') - shellcheck = Bazel.execute('build', [], '@multitool//tools/shellcheck') - Bazel.execute('run', ['--', '-shellcheck', shellcheck], '@multitool//tools/actionlint:cwd') - end - - # Example: `./go all:prepare[4.31.0,early-stable]` - # Equivalent to .github/workflows/pre-release.yml in a single command - desc 'Update everything in preparation for a release' - task :prepare, [:version, :channel] do |_task, arguments| - version = arguments[:version] - - Rake::Task['update_browsers'].invoke(arguments[:channel]) - Rake::Task['all:update_cdp'].invoke(arguments[:channel]) - Rake::Task['update_manager'].invoke - Rake::Task['java:update'].invoke - Rake::Task['authors'].invoke - Rake::Task['all:version'].invoke(version) - Rake::Task['all:changelogs'].invoke + raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? end Modify the all:lint task to forward any command-line arguments (except for the language skip flags) to each individual language's lint task. Rakefile [258-274] desc 'Run linters for all languages (skip with: ./go all:lint -rb -rust)' task :lint do |_task, arguments| all_langs = %w[java py rb node rust] - skip = arguments.to_a.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } + raw_args = arguments.to_a + skip_args = raw_args.select { |a| a.start_with?('-') } + skip = skip_args.map { |a| a.delete_prefix('-') } + invalid = skip - all_langs raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? + forward_args = raw_args - skip_args langs = all_langs - skip + failures = [] langs.each do |lang| puts "Linting #{lang}..." - Rake::Task["#{lang}:lint"].invoke + Rake::Task["#{lang}:lint"].invoke(*forward_args) rescue StandardError => e failures << "#{lang}: #{e.message}" end + raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? end [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that arguments are not being passed to the individual linter tasks and provides a correct implementation to forward them, improving the task's functionality.	Low

✅ Suggestions up to commit 08841ce

Category	Suggestion	Impact
Security	✅ Tighten artifact file permissions Suggestion Impact: Updated the chmod permissions for both generated .NET release zip artifacts from world-writable (0o666) to 0o644 as suggested. code diff: FileUtils.copy('bazel-bin/dotnet/release.zip', "build/dist/selenium-dotnet-#{dotnet_version}.zip") - FileUtils.chmod(0o666, "build/dist/selenium-dotnet-#{dotnet_version}.zip") + FileUtils.chmod(0o644, "build/dist/selenium-dotnet-#{dotnet_version}.zip") FileUtils.copy('bazel-bin/dotnet/strongnamed.zip', "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") - FileUtils.chmod(0o666, "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") + FileUtils.chmod(0o644, "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") Change the file permissions for the generated .zip artifacts from world-writable 0o666 to a more secure 0o644 to prevent tampering. rake_tasks/dotnet.rake [21-24] FileUtils.copy('bazel-bin/dotnet/release.zip', "build/dist/selenium-dotnet-#{dotnet_version}.zip") -FileUtils.chmod(0o666, "build/dist/selenium-dotnet-#{dotnet_version}.zip") +FileUtils.chmod(0o644, "build/dist/selenium-dotnet-#{dotnet_version}.zip") FileUtils.copy('bazel-bin/dotnet/strongnamed.zip', "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") -FileUtils.chmod(0o666, "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") +FileUtils.chmod(0o644, "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies overly permissive file permissions (0o666) on release artifacts, which is a security risk, and proposes a more secure alternative (0o644).	Medium
	Avoid shelling out unsafely Replace the backtick shell execution of git log with Open3.capture2 to prevent potential shell injection vulnerabilities from the tag or path variables. rake_tasks/common.rb [52-53] -command = "git log #{tag}...HEAD --pretty=format:'%s' --reverse -- #{path}" -log = `#{command}` +require 'open3' +... +log, status = Open3.capture2('git', 'log', "#{tag}...HEAD", "--pretty=format:%s", '--reverse', '--', path.to_s) +raise "git log failed for changelog generation (#{tag}..HEAD)" unless status.success? Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential shell injection vulnerability and recommends using Open3.capture2 with an argument array, which is a best practice for security and robustness.	Medium
Possible issue	✅ Add HTTP timeouts for downloads Suggestion Impact: The commit added `response = nil` before the retry loop, matching the first line of the suggested change, but it did not implement the main suggestion of using `Net::HTTP.start` with `open_timeout` and `read_timeout`. code diff: uri = URI("https://rubygems.org/gems/#{key}.gem") + response = nil 5.times do response = Net::HTTP.get_response(uri) Add open_timeout and read_timeout to the Net::HTTP request when downloading gems to prevent the task from hanging indefinitely on network issues. rake_tasks/ruby.rake [168-181] +response = nil 5.times do - response = Net::HTTP.get_response(uri) + response = Net::HTTP.start(uri.hostname, uri.port, + use_ssl: uri.scheme == 'https', + open_timeout: 10, + read_timeout: 30) do |http| + http.request(Net::HTTP::Get.new(uri)) + end break unless response.is_a?(Net::HTTPRedirection) uri = URI(response['location']) end unless response.is_a?(Net::HTTPSuccess) puts " #{key}: failed (HTTP #{response.code})" failed << key next end sha = Digest::SHA256.hexdigest(response.body) [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the HTTP request lacks timeouts, which can cause the task to hang indefinitely, and proposes a robust solution using Net::HTTP.start with timeouts.	Medium
	Avoid forwarding nil arguments Add .compact to arguments.to_a in the docs task to filter out nil values before forwarding them, preventing potential errors in sub-tasks. Rakefile [200-207] task :docs do |_task, arguments| - args = arguments.to_a + args = arguments.to_a.compact Rake::Task['java:docs'].invoke(*args) Rake::Task['py:docs'].invoke(*args) Rake::Task['rb:docs'].invoke(*args) Rake::Task['dotnet:docs'].invoke(*args) Rake::Task['node:docs'].invoke(*args) Rake::Task['rust:docs'].invoke(*args) end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that removing .compact can introduce nil arguments, which is a valid concern for robustness, and reverts a change made in the PR.	Medium
	Reuse sanitized forwarded arguments In the build task, sanitize arguments by calling arguments.to_a.compact once and reuse the result for all invoke calls to prevent nil propagation. Rakefile [210-216] task :build do |_task, arguments| - Rake::Task['java:build'].invoke(*arguments.to_a) - Rake::Task['py:build'].invoke(*arguments.to_a) - Rake::Task['rb:build'].invoke(*arguments.to_a) - Rake::Task['dotnet:build'].invoke(*arguments.to_a) - Rake::Task['node:build'].invoke(*arguments.to_a) + args = arguments.to_a.compact + Rake::Task['java:build'].invoke(*args) + Rake::Task['py:build'].invoke(*args) + Rake::Task['rb:build'].invoke(*args) + Rake::Task['dotnet:build'].invoke(*args) + Rake::Task['node:build'].invoke(*args) end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue with forwarding nil arguments and proposes a good practice of sanitizing and reusing the arguments list, improving both robustness and readability.	Medium
	Harden argument parsing for skips In the lint task, add .compact.map(&:to_s) to arguments.to_a to prevent NoMethodError from nil values when parsing arguments. Rakefile [258-262] task :lint do |_task, arguments| all_langs = %w[java py rb node dotnet rust] - skip = arguments.to_a.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } + raw_args = arguments.to_a.compact.map(&:to_s) + skip = raw_args.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } invalid = skip - all_langs raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that arguments.to_a can contain nil values, which would cause a NoMethodError, and proposes a robust way to handle them.	Medium
Learned best practice	Harden external JSON parsing Guard against non-JSON or whitespace-only responses from Sonatype by stripping the body and rescuing JSON::ParserError to raise a clearer error including the raw response. rake_tasks/java.rake [82] -res.body.to_s.empty? ? {} : JSON.parse(res.body) +body = res.body.to_s.strip +return {} if body.empty? +JSON.parse(body) + Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (external HTTP/JSON) to avoid failing with unclear errors on unexpected responses.	Low
	Validate parsed credential values Strip extracted values and only set ENV when a non-empty username/password was actually parsed to prevent propagating blank/whitespace credentials. rake_tasks/java.rake [98-102] elsif line.include?('<username>') - ENV['MAVEN_USER'] = line[%r{<username>(.*?)</username>}, 1] + user = line[%r{<username>(.*?)</username>}, 1].to_s.strip + ENV['MAVEN_USER'] = user unless user.empty? elsif line.include?('<password>') - ENV['MAVEN_PASSWORD'] = line[%r{<password>(.*?)</password>}, 1] + pass = line[%r{<password>(.*?)</password>}, 1].to_s.strip + ENV['MAVEN_PASSWORD'] = pass unless pass.empty? end Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (environment variables and config files) by trimming and checking presence before use.	Low

✅ Suggestions up to commit fb43c38

Category	Suggestion	Impact
Possible issue	✅ Validate required release inputs Suggestion Impact: The commit added a guard clause that raises an explicit error when the version argument is nil or empty, matching the suggested validation to prevent silent failures. code diff: version = arguments[:version] + raise 'Missing required version: ./go prep_release[4.31.0,early-stable]' if version.nil? || version.empty? Add validation to the prep_release task to ensure the required version argument is provided, preventing silent failures and providing a clear error message to the user if it is missing. Rakefile [107-118] desc 'Update everything in preparation for a release' task :prep_release, [:version, :channel] do |_task, arguments| version = arguments[:version] + raise 'Missing required version: ./go prep_release[4.31.0,early-stable]' if version.nil? || version.empty? - Rake::Task['update_browsers'].invoke(arguments[:channel]) - Rake::Task['update_cdp'].invoke(arguments[:channel]) + channel = arguments[:channel] || 'stable' + + Rake::Task['update_browsers'].invoke(channel) + Rake::Task['update_cdp'].invoke(channel) Rake::Task['update_manager'].invoke Rake::Task['java:update'].invoke Rake::Task['authors'].invoke Rake::Task['all:version'].invoke(version) Rake::Task['all:changelogs'].invoke end [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies that the new prep_release task lacks validation for the required version argument, which could lead to silent failures, and proposes adding an explicit check to improve robustness.	Medium
	✅ Handle missing settings file safely Suggestion Impact: The commit introduces settings_path, checks File.exist? before reading settings.xml, emits a warning, and returns early, preventing a crash when the file is missing. code diff: + settings_path = File.join(Dir.home, '.m2', 'settings.xml') + unless File.exist?(settings_path) + warn "Maven settings file not found at #{settings_path}" + return + end + puts 'Maven environment variables not set, inspecting ~/.m2/settings.xml.' - settings = File.read("#{Dir.home}/.m2/settings.xml") + settings = File.read(settings_path) Check if ~/.m2/settings.xml exists before attempting to read it to prevent the program from crashing and provide a clear warning if the file is missing. rake_tasks/java.rake [81-95] def read_m2_user_pass puts 'Maven environment variables not set, inspecting ~/.m2/settings.xml.' - settings = File.read("#{Dir.home}/.m2/settings.xml") + settings_path = File.join(Dir.home, '.m2', 'settings.xml') + unless File.exist?(settings_path) + warn "Maven settings file not found at #{settings_path}" + return + end + + settings = File.read(settings_path) found_section = false settings.each_line do |line| if !found_section found_section = line.include? '<id>central</id>' elsif line.include?('<username>') ENV['MAVEN_USER'] = line[%r{<username>(.*?)</username>}, 1] elsif line.include?('<password>') ENV['MAVEN_PASSWORD'] = line[%r{<password>(.*?)</password>}, 1] end break if ENV['MAVEN_PASSWORD'] && ENV['MAVEN_USER'] end end [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: The suggestion improves error handling by proactively checking for the existence of the settings.xml file, preventing a crash and providing a more user-friendly warning message.	Medium
	Fix linter argument parsing Refine the argument parsing for the all:lint task to correctly differentiate between language-skip flags (e.g., -rb) and other command-line options (e.g., --config), preventing incorrect behavior. Rakefile [243-259] desc 'Run linters for all languages (skip with: ./go all:lint -rb -rust)' task :lint do |_task, arguments| all_langs = %w[java py rb node dotnet rust] - skip = arguments.to_a.select { |a| a.start_with?('-') }.map { |a| a.delete_prefix('-') } - invalid = skip - all_langs + args = arguments.to_a + + # Only treat exact `-<lang>` tokens as skip flags; allow other `--foo` args to pass through. + skip_tokens, pass_through = args.partition { |a| all_langs.any? { |lang| a == "-#{lang}" } } + skip = skip_tokens.map { |a| a.delete_prefix('-') } + + invalid = (args.grep(/\A-/).map { |a| a.delete_prefix('-') } - pass_through.grep(/\A--/).map { |a| a.delete_prefix('-') } - all_langs) raise "Unknown languages: #{invalid.join(', ')}. Valid: #{all_langs.join(', ')}" if invalid.any? langs = all_langs - skip failures = [] langs.each do |lang| puts "Linting #{lang}..." - Rake::Task["#{lang}:lint"].invoke + Rake::Task["#{lang}:lint"].invoke(*pass_through) rescue StandardError => e failures << "#{lang}: #{e.message}" end raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a bug in argument parsing for the new all:lint task, where any flag starting with - is treated as a language to skip, and proposes a more robust implementation.	Medium
	Prevent undefined response usage Initialize response to nil before the redirect-following loop to prevent a potential NameError and use the safe navigation operator when accessing its properties. rake_tasks/ruby.rake [167-181] to_download.each do |key| uri = URI("https://rubygems.org/gems/#{key}.gem") + response = nil 5.times do response = Net::HTTP.get_response(uri) break unless response.is_a?(Net::HTTPRedirection) uri = URI(response['location']) end unless response.is_a?(Net::HTTPSuccess) - puts " #{key}: failed (HTTP #{response.code})" + code = response&.code || 'no response' + puts " #{key}: failed (HTTP #{code})" failed << key next end Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies a potential NameError if the loop were not to execute, and the proposed change with pre-initialization and safe navigation makes the code more robust against unexpected control flow.	Low
Learned best practice	Validate URL before HTTP call Validate that url is a non-empty HTTP(S) URL before calling URI() to avoid unexpected crashes and ambiguous errors. rake_tasks/common.rb [68-75] def self.verify_package_published(url) + url = url.to_s.strip + raise 'Package URL must be provided' if url.empty? + + uri = URI(url) + raise "Unsupported URL scheme: #{uri.scheme}" unless %w[http https].include?(uri.scheme) + puts "Verifying #{url}..." - uri = URI(url) - res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: uri.scheme == 'https') { |http| http.request(Net::HTTP::Get.new(uri)) } + res = Net::HTTP.start(uri.hostname, uri.port, use_ssl: uri.scheme == 'https') do |http| + http.request(Net::HTTP::Get.new(uri)) + end raise "Package not published: #{url}" unless res.is_a?(Net::HTTPSuccess) puts 'Verified!' end Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/availability guards at integration boundaries (e.g., URLs) before use.	Low
Security	Restrict artifact file permissions Change the file permissions for release artifacts from world-writable (0o666) to a more secure 0o644 to prevent unintended modifications. rake_tasks/dotnet.rake [21-24] FileUtils.copy('bazel-bin/dotnet/release.zip', "build/dist/selenium-dotnet-#{dotnet_version}.zip") -FileUtils.chmod(0o666, "build/dist/selenium-dotnet-#{dotnet_version}.zip") +FileUtils.chmod(0o644, "build/dist/selenium-dotnet-#{dotnet_version}.zip") FileUtils.copy('bazel-bin/dotnet/strongnamed.zip', "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") -FileUtils.chmod(0o666, "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") +FileUtils.chmod(0o644, "build/dist/selenium-dotnet-strongnamed-#{dotnet_version}.zip") Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that world-writable permissions (0o666) are too permissive for release artifacts and recommends a more secure default (0o644), which is a good security practice.	Low

✅ Suggestions up to commit 5d81dcf

Category	Suggestion	Impact
Possible issue	Swap target diff logic Correct the inverted logic for identifying missing and extra release targets by swapping the operands in the array subtraction operations. rake_tasks/java.rake [45-65] def verify_java_release_targets - query = 'kind(maven_publish, set(//java/... //third_party/...))' - current_targets = [] - - Bazel.execute('query', [], query) do |output| - current_targets = output.lines.map(&:strip).reject(&:empty?).select { |line| line.start_with?('//') } - end - - missing_targets = current_targets - JAVA_RELEASE_TARGETS - extra_targets = JAVA_RELEASE_TARGETS - current_targets - - return if missing_targets.empty? && extra_targets.empty? - - error_message = 'Java release targets are out of sync with Bazel query results.' - - error_message += "\nMissing targets: #{missing_targets.join(', ')}" unless missing_targets.empty? - - error_message += "\nObsolete targets: #{extra_targets.join(', ')}" unless extra_targets.empty? - - raise error_message + # ... + missing_targets = JAVA_RELEASE_TARGETS - current_targets + extra_targets = current_targets - JAVA_RELEASE_TARGETS + # ... end Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a logical flaw in the target verification logic, which would report incorrect results and cause confusion. Fixing this is critical for the task's correctness.	High
	Separate dry-run flag into args Pass the --dry-run=true flag as a separate element in the arguments array for the Bazel.execute call to ensure it is correctly interpreted. rake_tasks/node.rake [72-74] target = '//javascript/selenium-webdriver:selenium-webdriver.publish' -target += ' -- --dry-run=true' if dry_run -Bazel.execute('run', ['--config=release'], target) +args = ['--config=release'] +if dry_run + args += ['--', '--dry-run=true'] +end +Bazel.execute('run', args, target) Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that command-line arguments are being incorrectly concatenated into the target string, which would cause the dry-run flag to be ignored, leading to an unintended release.	Medium
	Fix incorrect linter failure handling Remove the begin...rescue block around the Rake::Task['all:lint'].invoke call to ensure the main lint task aborts immediately if any language-specific linter fails. Rakefile [121-153] task :lint do |_task, arguments| failures = [] - begin - Rake::Task['all:lint'].invoke(*arguments.to_a) - rescue StandardError => e - failures << e.message - end + Rake::Task['all:lint'].invoke(*arguments.to_a) puts 'Linting Bazel files...' begin Bazel.execute('run', [], '//:buildifier') rescue StandardError => e failures << "buildifier: #{e.message}" end puts 'Linting shell scripts and GitHub Actions...' begin shellcheck = Bazel.execute('build', [], '@multitool//tools/shellcheck') Bazel.execute('run', ['--', '-shellcheck', shellcheck], '@multitool//tools/actionlint:cwd') rescue StandardError => e failures << "shellcheck/actionlint: #{e.message}" end puts 'Updating copyright headers...' begin Bazel.execute('run', [], '//scripts:update_copyright') rescue StandardError => e failures << "copyright: #{e.message}" end raise "Lint failed:\n#{failures.join("\n")}" unless failures.empty? end Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the lint task should fail fast if the all:lint sub-task fails, improving the task's logic and providing quicker feedback.	Medium
General	Fix memoization of target verification Fix the memoization in java_release_targets by explicitly setting @targets_verified to true after a successful verification, preventing redundant checks. rake_tasks/java.rake [39-43] def java_release_targets - @targets_verified ||= verify_java_release_targets - + unless @targets_verified + verify_java_release_targets + @targets_verified = true + end JAVA_RELEASE_TARGETS end Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug in the memoization logic that causes an expensive verification process to ...

[Copilot]

Pull request overview

This PR refactors the monolithic Rakefile (~1600 lines) into modular, per-language task files to improve maintainability and organization. The refactoring creates a shared SeleniumRake module for common utilities and loads language-specific tasks into namespaces with aliases for backward compatibility.

Changes:

• Split Rakefile into 9 focused task files organized by language/component
• Introduced rake_tasks/common.rb with shared utilities (version management, changelog generation, package verification)
• Created namespace-based loading with aliases (e.g., rb/ruby, py/python, node/js/javascript) to maintain backward compatibility
• Added aggregation tasks in all:* namespace for cross-language operations

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
Rakefile	Reduced from ~1600 to ~294 lines; loads per-language tasks into namespaces; maintains legacy task aliases
rake_tasks/common.rb	New shared SeleniumRake module with version bumping, changelog generation, and package verification utilities
rake_tasks/java.rake	Java-specific tasks: build, release, package, verify, docs, changelogs, version, lint, install, Maven dependency management
rake_tasks/python.rake	Python-specific tasks: build, release, verify, docs, changelogs, version, lint, install, local_dev
rake_tasks/ruby.rake	Ruby-specific tasks: build, release, verify, docs, changelogs, version, lint, install, gem pinning, local_dev
rake_tasks/node.rake	Node/JavaScript-specific tasks: build, release, verify, docs, changelogs, version, lint, install, pnpm dependency management
rake_tasks/dotnet.rake	.NET-specific tasks: build, package, release, verify, docs, changelogs, version, lint, install, paket dependency management
rake_tasks/rust.rake	Rust/Selenium Manager tasks: build, update, changelogs, version, lint, pin
rake_tasks/grid.rake	Grid server tasks: build, package, release
rake_tasks/appium.rake	iOS driver atoms build task for Appium integration

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.