Skip to content

[build] source the Java and .NET versions from a root version.bzl that tests do not read - #18138

Merged
titusfortner merged 5 commits into
trunkfrom
version-bzl-not-test-input
Oct 8, 2026
Merged

titusfortner merged 5 commits into
trunkfrom
version-bzl-not-test-input

Conversation

@titusfortner

@titusfortner titusfortner commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

Builds on #18135

💥 What does this PR do?

  • Stops release commits from unnecessarily invalidating RBE test cache for .NET and Java targets.
  • This should prevent rerunning ~2000 of ~2900 RBE tests during release.

🔧 Implementation Notes

  • The version reached tests through the build-generated Java manifest jar and .NET AssemblyInfo; unstamped builds now omit it from the manifest and embed VERSION_STUB in the AssemblyInfo, and --config=release embeds the real version as before.
  • A root version.bzl replaces the literals in java/version.bzl and dotnet/version.bzl so every binding's version lives in one file for release.yml and rake; BUILD files load() it, which only re-analyzes.
  • version.bzl keeps each binding's format convention with one entry per binding, so patch versions stay independent and the rake bump logic is unchanged.
  • Removed the specific timestamp from committed .NET nightly since it is already updated at build time in the publish job
  • Unstamped Java builds carry no Selenium-Version at all, so BuildInfo reports its existing unknown; SeleniumManager extracts its embedded binary to a per-process temp directory for such builds instead of caching it under ~/.cache/selenium/manager/<version>/, and versioned builds now key that directory on the Selenium release label (4.51.1, 4.51.0-SNAPSHOT) instead of 0.<major.minor> parsed from it, which crashed on labels without a dot.

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code (Fable 5.1)
    • What was generated: the analysis of which test inputs carry the version, the change, the local verification, and this description
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Consider doing something similar for Python since that version file can be easily dynamically generated by bazel
  • Consider Ruby and JavaScript implementation, but their version files must be read at fetch time, so moving them needs a committed placeholder which feels more of a hack.

🔄 Types of changes

  • Cleanup (build configuration; unstamped bazel run builds now report unknown (Java) or VERSION_STUB (.NET) instead of the version)

@selenium-ci selenium-ci added C-dotnet .NET Bindings C-java Java Bindings B-build Includes scripting, bazel and CI integrations labels Oct 8, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Keep Java and .NET version bumps out of unstamped test inputs

⚙️ Configuration changes ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Centralize Java and .NET release versions without embedding them in unstamped test artifacts.
• Preserve real versions in stamped builds and generate unique .NET versions when publishing
 nightlies.
• Point release automation and Rake version tasks at the new root version file.
Diagram

graph TD
  R["Rake version tasks"] --> V["Root versions"] --> J["Java build rules"] --> M["Java metadata"] --> T["RBE test cache"]
  V --> D[".NET build rules"] --> A["AssemblyInfo"] --> T
Loading
High-Level Assessment

Keep the version at build-analysis time and substitute a stable value only in unstamped generated artifacts. This retains release metadata without making routine test outputs depend on version bumps. Moving all versioning into generated files would be more disruptive to existing release tasks and package metadata.

Files changed (8) +46 / -26

Refactor (2) +16 / -7
common.rbAdd shared root-version read and write helpers +14/-0

Add shared root-version read and write helpers

• Adds helpers that locate and replace individual binding entries in version.bzl for language-specific Rake tasks.

rake_tasks/common.rb

java.rakeMove Java version management to the root file +2/-7

Move Java version management to the root file

• Reads and writes the Java entry through the shared Rake helpers instead of editing java/version.bzl directly.

rake_tasks/java.rake

Other (6) +30 / -19
release.ymlTrigger release workflow on root version changes +1/-2

Trigger release workflow on root version changes

• Replaces the Java and .NET version-file path filters with version.bzl so release preparation still triggers the workflow.

.github/workflows/release.yml

version.bzlStub unstamped assembly informational versions +10/-2

Stub unstamped assembly informational versions

• Loads the .NET version from the root file. AssemblyInfo receives the real version when stamping is enabled and VERSION_STUB otherwise.

dotnet/version.bzl

BUILD.bazelRemove real versions from unstamped Java metadata +4/-4

Remove real versions from unstamped Java metadata

• Writes VERSION_STUB to unstamped build-properties and manifest jars. Stamped branches continue to embed the real Java version.

java/src/org/openqa/selenium/BUILD.bazel

version.bzlLoad the Java version from the root file +3/-1

Load the Java version from the root file

• Replaces the local version literal with the Java entry from version.bzl while preserving the existing SE_VERSION binding.

java/version.bzl

dotnet.rakeMove .NET version management to the root file +5/-10

Move .NET version management to the root file

• Reads and updates the root .NET entry. Version tasks now commit an untimestamped nightly value; the release task appends a timestamp before a nightly publish.

rake_tasks/dotnet.rake

version.bzlCentralize Java and .NET binding versions +7/-0

Centralize Java and .NET binding versions

• Introduces one version entry per binding, retaining Java's SNAPSHOT and .NET's untimestamped nightly conventions.

version.bzl

@qodo-code-review

qodo-code-review Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Java builds can run an old manager ✗ Dismissed
Description
getBinaryInCache() uses the release label as its cache directory, but unstamped builds all receive
the constant label VERSION_STUB from the manifest. When different unstamped builds share a cache,
getBinary() finds the first extracted executable and skips a newer bundled manager, potentially
affecting driver resolution.
Code

java/src/org/openqa/selenium/manager/SeleniumManager.java[310]

+        String.format(BINARY_PATH_FORMAT, new BuildInfo().getReleaseLabel(), binaryName));
Evidence
The default manifest writes VERSION_STUB, which BuildInfo reads from its Selenium-Version
property. getBinaryInCache() uses that label for the cache path, and getBinary() extracts the
packaged executable only when the path does not exist; because manager artifacts can change between
builds, an existing path can retain an older executable.

AGENTS.md: Preserve Public API and ABI Compatibility and Deprecate Before Removal
java/src/org/openqa/selenium/manager/SeleniumManager.java[307-310]
java/src/org/openqa/selenium/manager/SeleniumManager.java[206-218]
java/src/org/openqa/selenium/BUILD.bazel[114-119]
java/src/org/openqa/selenium/BUILD.bazel[114-124]
java/src/org/openqa/selenium/BuildInfo.java[35-46]
java/src/org/openqa/selenium/manager/SeleniumManager.java[217-229]
java/src/org/openqa/selenium/manager/BUILD.bazel[34-59]

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

## Issue description
Unstamped Java builds share the `VERSION_STUB` Selenium Manager cache directory, so a later build can reuse an older extracted manager instead of its bundled executable.

## Fix Focus Areas
- java/src/org/openqa/selenium/BUILD.bazel[113-119]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[217-229]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[307-310]

## Recommended Fix
Give unstamped bundled manager binaries a content-dependent cache key, or safely compare the cached executable with the packaged resource and replace it when their contents differ. Preserve version-based directories for stamped releases if desired, and add a focused test with two distinct unstamped manager artifacts sharing a cache.

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


2. Unstamped Java driver lookup crashes ✗ Dismissed
Description
The unstamped manifest sets Selenium-Version to VERSION_STUB, but SeleniumManager assumes the
release label contains a dot and calls substring(0, -1) when it does not. Ordinary Bazel builds
use that unstamped manifest, and DriverFinder initializes SeleniumManager even when a driver
executable is configured explicitly.
Code

java/src/org/openqa/selenium/BUILD.bazel[116]

+        echo "Selenium-Version = VERSION_STUB" >META-INF/selenium-build.properties &&
Evidence
The changed genrule writes the stub to the resource read by BuildInfo. SeleniumManager takes the
substring before the label's last dot without checking whether one exists, and DriverFinder calls
SeleniumManager.getInstance() in its constructor. The release configuration explicitly enables
stamping; ordinary builds take the unstamped branch.

java/src/org/openqa/selenium/BUILD.bazel[102-125]
java/src/org/openqa/selenium/BuildInfo.java[33-52]
java/src/org/openqa/selenium/manager/SeleniumManager.java[81-87]
java/src/org/openqa/selenium/remote/service/DriverFinder.java[42-45]
.bazelrc[164-165]

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

## Issue description
Unstamped Java builds embed `VERSION_STUB`, which causes Selenium Manager initialization to throw when it extracts the version prefix.

## Fix Focus Areas
- java/src/org/openqa/selenium/BUILD.bazel[113-119]
- java/src/org/openqa/selenium/BUILD.bazel[150-157]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[83-87]

## Recommended Fix
Use the same version-independent, dotted placeholder in both unstamped artifacts so Selenium Manager can derive its cache version. Add a test that constructs Selenium Manager with an unstamped build label.

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


3. Nightly package versions go untested ✗ Dismissed
Description
dotnet:version now writes an untimestamped nightly version, while dotnet:release appends the
timestamp, but no automated test checks that task sequence. A change to either task or the new
root-file writer could leave the version used for nightly packaging wrong without a test detecting
it before the release workflow.
Code

rake_tasks/dotnet.rake[R41-43]

    Rake::Task['dotnet:version'].invoke('nightly')
+    # Each nightly publish needs its own version; the committed one has no timestamp
+    SeleniumRake.write_version('dotnet', "#{dotnet_version}#{Time.now.strftime('%Y%m%d%H%M')}")
Evidence
Rule 3 requires coverage for changed behavior. The PR moves timestamp creation from the version task
to the release task and adds a root-file writer, but the repository's tests do not exercise those
helpers or the changed release sequence.

AGENTS.md: Prefer Small Tests That Exercise Real API Contracts
rake_tasks/dotnet.rake[39-49]
rake_tasks/dotnet.rake[95-100]
rake_tasks/common.rb[45-55]

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 changed nightly release sequence and root version-file writer have no focused automated coverage.

## Fix Focus Areas
- rake_tasks/dotnet.rake[40-43]
- rake_tasks/dotnet.rake[95-100]
- rake_tasks/common.rb[45-55]

## Recommended Fix
Add a focused Rake test using a temporary version file and controlled time. Assert that the version task writes an untimestamped nightly entry and that the release path appends a timestamp before packaging.

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


View high (1)
4. Build metadata changes go unchecked ✗ Dismissed
Description
The new stamped and unstamped branches for Java manifests and .NET assembly information have no
tests asserting which version each branch embeds. The existing Java User-Agent test derives its
expectation from the generated metadata, so it would still pass if that metadata contained the wrong
value; no .NET test checks the assembly version or resulting User-Agent.
Code

dotnet/version.bzl[R18-21]

+ASSEMBLY_INFORMATIONAL_VERSION = select({
+    "//dotnet/private:stamp_enabled": SE_VERSION,
+    "//conditions:default": "VERSION_STUB",
+})
Evidence
Rule 3 requires appropriate coverage for changed behavior. The diff changes both metadata branches;
the Java consumer test compares against a value read from that metadata rather than an independent
expected version, and the .NET tests do not assert the changed assembly version.

AGENTS.md: Prefer Small Tests That Exercise Real API Contracts
dotnet/version.bzl[16-21]
java/src/org/openqa/selenium/BUILD.bazel[113-120]
java/src/org/openqa/selenium/BUILD.bazel[150-158]
java/test/org/openqa/selenium/remote/internal/HttpClientTestBase.java[177-185]

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 changed Java and .NET metadata outputs are not tested against independently expected versions in stamped and unstamped builds.

## Fix Focus Areas
- dotnet/version.bzl[18-21]
- java/src/org/openqa/selenium/BUILD.bazel[115-119]
- java/src/org/openqa/selenium/BUILD.bazel[150-157]

## Recommended Fix
Add focused build tests that inspect the generated metadata and assert the configured version in stamped builds and `VERSION_STUB` in unstamped builds.

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



Remediation recommended

5. Temp cleanup breaks later driver lookup ✓ Resolved
Description
getBinaryInCache() now registers Selenium Manager’s extracted directory with the shared
TemporaryFilesystem, while getBinary() retains its path after that directory is deleted. If an
application calls deleteTemporaryFiles() during its lifetime, subsequent Selenium Manager calls
throw rather than extract the bundled binary again.
Code

java/src/org/openqa/selenium/manager/SeleniumManager.java[R289-290]

+      cacheParent =
+          TemporaryFilesystem.getDefaultTmpFS().createTempDir(SELENIUM_MANAGER, "").toPath();
Evidence
createTempDir() adds the directory to the shared filesystem’s tracked set, and its public
deleteTemporaryFiles() method deletes tracked directories. Selenium Manager assigns binary only
on its first lookup; when the cached path no longer exists, its later-call branch throws instead of
rebuilding it. The repository also exercises explicit cleanup of the default filesystem in a test.

java/src/org/openqa/selenium/io/TemporaryFilesystem.java[90-108]
java/src/org/openqa/selenium/io/TemporaryFilesystem.java[132-145]
java/src/org/openqa/selenium/manager/SeleniumManager.java[180-211]
java/test/org/openqa/selenium/io/TemporaryFilesystemTest.java[129-142]

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

## Issue description
Explicit cleanup of the shared temporary filesystem deletes Selenium Manager’s extracted binary, but Selenium Manager retains its path and fails on later calls.
## Fix Focus Areas
- java/src/org/openqa/selenium/manager/SeleniumManager.java[180-211]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[285-294]
## Recommended Fix
Track whether the cached binary is in a Selenium Manager-owned temporary directory. If that binary has been removed, create a new tracked directory and extract it again on the next call; preserve the existing missing-file error for a user-supplied manager path.

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



Informational

6. Every dev-build run leaves empty temp folders ✓ Resolved
Description
getBinaryInCache creates a fresh temporary root when the release label is unknown, places the
binary under manager/unknown/, and leaves its directories behind because the shutdown hook deletes
only the binary. When an unstamped Java build resolves bundled Selenium Manager, BuildInfo.read
returns unknown, so each such process leaves an empty directory tree in the system temp folder
after exiting.
Code

java/src/org/openqa/selenium/manager/SeleniumManager.java[R302-306]

+    String releaseLabel = new BuildInfo().getReleaseLabel();
    Path cacheParent = Paths.get(cachePath);
-    if (!Files.isWritable(cacheParent)) {
+    if (!Files.isWritable(cacheParent) || "unknown".equals(releaseLabel)) {
      cacheParent = Files.createTempDirectory(SELENIUM_MANAGER);
      binaryInTemporalFolder = true;
Evidence
Unstamped builds do not write Selenium-Version, so BuildInfo.read returns unknown and the new
|| "unknown".equals(releaseLabel) condition causes getBinaryInCache to create a temporary root.
getBinary() creates the nested manager/unknown/ directories, while the constructor’s shutdown
hook calls only Files.delete(binary), leaving those directories in place.

java/src/org/openqa/selenium/manager/SeleniumManager.java[300-311]
java/src/org/openqa/selenium/manager/SeleniumManager.java[82-97]
java/src/org/openqa/selenium/BuildInfo.java[68-74]
java/src/org/openqa/selenium/BUILD.bazel[114-119]
java/src/org/openqa/selenium/BuildInfo.java[48-51]
java/src/org/openqa/selenium/BuildInfo.java[61-68]
java/src/org/openqa/selenium/manager/SeleniumManager.java[302-310]
java/src/org/openqa/selenium/manager/SeleniumManager.java[216-230]
java/src/org/openqa/selenium/manager/SeleniumManager.java[83-95]

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

## Issue description
Unstamped Java builds extract Selenium Manager into a fresh temporary root with the binary under `manager/unknown/`, but shutdown deletes only the binary and leaves the directory tree behind.

## Fix Focus Areas
- java/src/org/openqa/selenium/manager/SeleniumManager.java[82-97]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[300-311]

## Recommended Fix
Track the temporary root created by `getBinaryInCache`. When `binaryInTemporalFolder` is true, recursively remove its extracted contents and directories at shutdown instead of deleting only the binary, and log a warning if cleanup fails. Alternatively, place the binary directly in the temporary root and delete both the binary and its parent. Preserve existing behavior for binaries in the shared cache and for a configured manager path.

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


Grey Divider

Context sources
Review mode: Auto: 🚀 Fast: Localized runtime condition change with contained impact and one edit site.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 9c4b81e

Results up to commit 70dd987 ⚖️ Balanced


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


Action required
1. Unstamped Java driver lookup crashes ✗ Dismissed
Description
The unstamped manifest sets Selenium-Version to VERSION_STUB, but SeleniumManager assumes the
release label contains a dot and calls substring(0, -1) when it does not. Ordinary Bazel builds
use that unstamped manifest, and DriverFinder initializes SeleniumManager even when a driver
executable is configured explicitly.
Code

java/src/org/openqa/selenium/BUILD.bazel[116]

+        echo "Selenium-Version = VERSION_STUB" >META-INF/selenium-build.properties &&
Evidence
The changed genrule writes the stub to the resource read by BuildInfo. SeleniumManager takes the
substring before the label's last dot without checking whether one exists, and DriverFinder calls
SeleniumManager.getInstance() in its constructor. The release configuration explicitly enables
stamping; ordinary builds take the unstamped branch.

java/src/org/openqa/selenium/BUILD.bazel[102-125]
java/src/org/openqa/selenium/BuildInfo.java[33-52]
java/src/org/openqa/selenium/manager/SeleniumManager.java[81-87]
java/src/org/openqa/selenium/remote/service/DriverFinder.java[42-45]
.bazelrc[164-165]

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

## Issue description
Unstamped Java builds embed `VERSION_STUB`, which causes Selenium Manager initialization to throw when it extracts the version prefix.

## Fix Focus Areas
- java/src/org/openqa/selenium/BUILD.bazel[113-119]
- java/src/org/openqa/selenium/BUILD.bazel[150-157]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[83-87]

## Recommended Fix
Use the same version-independent, dotted placeholder in both unstamped artifacts so Selenium Manager can derive its cache version. Add a test that constructs Selenium Manager with an unstamped build label.

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


2. Build metadata changes go unchecked ✗ Dismissed
Description
The new stamped and unstamped branches for Java manifests and .NET assembly information have no
tests asserting which version each branch embeds. The existing Java User-Agent test derives its
expectation from the generated metadata, so it would still pass if that metadata contained the wrong
value; no .NET test checks the assembly version or resulting User-Agent.
Code

dotnet/version.bzl[R18-21]

+ASSEMBLY_INFORMATIONAL_VERSION = select({
+    "//dotnet/private:stamp_enabled": SE_VERSION,
+    "//conditions:default": "VERSION_STUB",
+})
Evidence
Rule 3 requires appropriate coverage for changed behavior. The diff changes both metadata branches;
the Java consumer test compares against a value read from that metadata rather than an independent
expected version, and the .NET tests do not assert the changed assembly version.

AGENTS.md: Prefer Small Tests That Exercise Real API Contracts
dotnet/version.bzl[16-21]
java/src/org/openqa/selenium/BUILD.bazel[113-120]
java/src/org/openqa/selenium/BUILD.bazel[150-158]
java/test/org/openqa/selenium/remote/internal/HttpClientTestBase.java[177-185]

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 changed Java and .NET metadata outputs are not tested against independently expected versions in stamped and unstamped builds.

## Fix Focus Areas
- dotnet/version.bzl[18-21]
- java/src/org/openqa/selenium/BUILD.bazel[115-119]
- java/src/org/openqa/selenium/BUILD.bazel[150-157]

## Recommended Fix
Add focused build tests that inspect the generated metadata and assert the configured version in stamped builds and `VERSION_STUB` in unstamped builds.

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


3. Nightly package versions go untested ✗ Dismissed
Description
dotnet:version now writes an untimestamped nightly version, while dotnet:release appends the
timestamp, but no automated test checks that task sequence. A change to either task or the new
root-file writer could leave the version used for nightly packaging wrong without a test detecting
it before the release workflow.
Code

rake_tasks/dotnet.rake[R41-43]

    Rake::Task['dotnet:version'].invoke('nightly')
+    # Each nightly publish needs its own version; the committed one has no timestamp
+    SeleniumRake.write_version('dotnet', "#{dotnet_version}#{Time.now.strftime('%Y%m%d%H%M')}")
Evidence
Rule 3 requires coverage for changed behavior. The PR moves timestamp creation from the version task
to the release task and adds a root-file writer, but the repository's tests do not exercise those
helpers or the changed release sequence.

AGENTS.md: Prefer Small Tests That Exercise Real API Contracts
rake_tasks/dotnet.rake[39-49]
rake_tasks/dotnet.rake[95-100]
rake_tasks/common.rb[45-55]

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 changed nightly release sequence and root version-file writer have no focused automated coverage.

## Fix Focus Areas
- rake_tasks/dotnet.rake[40-43]
- rake_tasks/dotnet.rake[95-100]
- rake_tasks/common.rb[45-55]

## Recommended Fix
Add a focused Rake test using a temporary version file and controlled time. Assert that the version task writes an untimestamped nightly entry and that the release path appends a timestamp before packaging.

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


Results up to commit f2fa0ee ⚖️ Balanced


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


Action required
1. Java builds can run an old manager ✗ Dismissed
Description
getBinaryInCache() uses the release label as its cache directory, but unstamped builds all receive
the constant label VERSION_STUB from the manifest. When different unstamped builds share a cache,
getBinary() finds the first extracted executable and skips a newer bundled manager, potentially
affecting driver resolution.
Code

java/src/org/openqa/selenium/manager/SeleniumManager.java[310]

+        String.format(BINARY_PATH_FORMAT, new BuildInfo().getReleaseLabel(), binaryName));
Evidence
The default manifest writes VERSION_STUB, which BuildInfo reads from its Selenium-Version
property. getBinaryInCache() uses that label for the cache path, and getBinary() extracts the
packaged executable only when the path does not exist; because manager artifacts can change between
builds, an existing path can retain an older executable.

AGENTS.md: Preserve Public API and ABI Compatibility and Deprecate Before Removal
java/src/org/openqa/selenium/manager/SeleniumManager.java[307-310]
java/src/org/openqa/selenium/manager/SeleniumManager.java[206-218]
java/src/org/openqa/selenium/BUILD.bazel[114-119]
java/src/org/openqa/selenium/BUILD.bazel[114-124]
java/src/org/openqa/selenium/BuildInfo.java[35-46]
java/src/org/openqa/selenium/manager/SeleniumManager.java[217-229]
java/src/org/openqa/selenium/manager/BUILD.bazel[34-59]

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

## Issue description
Unstamped Java builds share the `VERSION_STUB` Selenium Manager cache directory, so a later build can reuse an older extracted manager instead of its bundled executable.

## Fix Focus Areas
- java/src/org/openqa/selenium/BUILD.bazel[113-119]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[217-229]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[307-310]

## Recommended Fix
Give unstamped bundled manager binaries a content-dependent cache key, or safely compare the cached executable with the packaged resource and replace it when their contents differ. Preserve version-based directories for stamped releases if desired, and add a focused test with two distinct unstamped manager artifacts sharing a cache.

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


Results up to commit 9b6d2e1 🧠 Deep


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


Informational
1. Every dev-build run leaves empty temp folders ✓ Resolved
Description
getBinaryInCache creates a fresh temporary root when the release label is unknown, places the
binary under manager/unknown/, and leaves its directories behind because the shutdown hook deletes
only the binary. When an unstamped Java build resolves bundled Selenium Manager, BuildInfo.read
returns unknown, so each such process leaves an empty directory tree in the system temp folder
after exiting.
Code

java/src/org/openqa/selenium/manager/SeleniumManager.java[R302-306]

+    String releaseLabel = new BuildInfo().getReleaseLabel();
    Path cacheParent = Paths.get(cachePath);
-    if (!Files.isWritable(cacheParent)) {
+    if (!Files.isWritable(cacheParent) || "unknown".equals(releaseLabel)) {
      cacheParent = Files.createTempDirectory(SELENIUM_MANAGER);
      binaryInTemporalFolder = true;
Evidence
Unstamped builds do not write Selenium-Version, so BuildInfo.read returns unknown and the new
|| "unknown".equals(releaseLabel) condition causes getBinaryInCache to create a temporary root.
getBinary() creates the nested manager/unknown/ directories, while the constructor’s shutdown
hook calls only Files.delete(binary), leaving those directories in place.

java/src/org/openqa/selenium/manager/SeleniumManager.java[300-311]
java/src/org/openqa/selenium/manager/SeleniumManager.java[82-97]
java/src/org/openqa/selenium/BuildInfo.java[68-74]
java/src/org/openqa/selenium/BUILD.bazel[114-119]
java/src/org/openqa/selenium/BuildInfo.java[48-51]
java/src/org/openqa/selenium/BuildInfo.java[61-68]
java/src/org/openqa/selenium/manager/SeleniumManager.java[302-310]
java/src/org/openqa/selenium/manager/SeleniumManager.java[216-230]
java/src/org/openqa/selenium/manager/SeleniumManager.java[83-95]

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

## Issue description
Unstamped Java builds extract Selenium Manager into a fresh temporary root with the binary under `manager/unknown/`, but shutdown deletes only the binary and leaves the directory tree behind.

## Fix Focus Areas
- java/src/org/openqa/selenium/manager/SeleniumManager.java[82-97]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[300-311]

## Recommended Fix
Track the temporary root created by `getBinaryInCache`. When `binaryInTemporalFolder` is true, recursively remove its extracted contents and directories at shutdown instead of deleting only the binary, and log a warning if cleanup fails. Alternatively, place the binary directly in the temporary root and delete both the binary and its parent. Preserve existing behavior for binaries in the shared cache and for a configured manager path.

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


Results up to commit 5a3dcc9 ⚖️ Balanced


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


Remediation recommended
1. Temp cleanup breaks later driver lookup ✓ Resolved
Description
getBinaryInCache() now registers Selenium Manager’s extracted directory with the shared
TemporaryFilesystem, while getBinary() retains its path after that directory is deleted. If an
application calls deleteTemporaryFiles() during its lifetime, subsequent Selenium Manager calls
throw rather than extract the bundled binary again.
Code

java/src/org/openqa/selenium/manager/SeleniumManager.java[R289-290]

+      cacheParent =
+          TemporaryFilesystem.getDefaultTmpFS().createTempDir(SELENIUM_MANAGER, "").toPath();
Evidence
createTempDir() adds the directory to the shared filesystem’s tracked set, and its public
deleteTemporaryFiles() method deletes tracked directories. Selenium Manager assigns binary only
on its first lookup; when the cached path no longer exists, its later-call branch throws instead of
rebuilding it. The repository also exercises explicit cleanup of the default filesystem in a test.

java/src/org/openqa/selenium/io/TemporaryFilesystem.java[90-108]
java/src/org/openqa/selenium/io/TemporaryFilesystem.java[132-145]
java/src/org/openqa/selenium/manager/SeleniumManager.java[180-211]
java/test/org/openqa/selenium/io/TemporaryFilesystemTest.java[129-142]

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

## Issue description
Explicit cleanup of the shared temporary filesystem deletes Selenium Manager’s extracted binary, but Selenium Manager retains its path and fails on later calls.
## Fix Focus Areas
- java/src/org/openqa/selenium/manager/SeleniumManager.java[180-211]
- java/src/org/openqa/selenium/manager/SeleniumManager.java[285-294]
## Recommended Fix
Track whether the cached binary is in a Selenium Manager-owned temporary directory. If that binary has been removed, create a new tracked directory and extract it again on the next call; preserve the existing missing-file error for a user-supplied manager path.

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


Grey Divider

Qodo Logo

Comment thread dotnet/version.bzl
Comment thread rake_tasks/dotnet.rake
Comment thread java/src/org/openqa/selenium/BUILD.bazel Outdated
@titusfortner
titusfortner force-pushed the version-bzl-not-test-input branch from 70dd987 to f2fa0ee Compare October 8, 2026 18:22
Comment thread java/src/org/openqa/selenium/manager/SeleniumManager.java Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner force-pushed the version-bzl-not-test-input branch from f2fa0ee to 9b6d2e1 Compare October 8, 2026 19:58
Comment thread java/src/org/openqa/selenium/manager/SeleniumManager.java Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 9b6d2e1

Comment thread java/src/org/openqa/selenium/manager/SeleniumManager.java
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 9c4b81e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-dotnet .NET Bindings C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants