Skip to content

[build] make Java and .NET release reruns pass when already published - #17935

Merged
titusfortner merged 1 commit into
trunkfrom
java-release-rerun-safety
Aug 21, 2026
Merged

[build] make Java and .NET release reruns pass when already published#17935
titusfortner merged 1 commit into
trunkfrom
java-release-rerun-safety

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

  • Makes the Java release rerun-safe. Java is the only binding where publishing a version twice causes a problem rather than being ignored/rejected
  • Removes the release.yml workflow guard that failed the Java publish job on a rerun
  • Ensures Java and .NET always build and package artifacts to publish with GitHub release

🔧 Implementation Notes

  • Before deploying, java:release checks if a previous release attempt left a problematic state.
  • We can't get the information we need from repo1.maven.org because it is slow to update and will always be a race condition.
Check What it means What we do
ossrh-staging-api reports repo as open or closed a previous attempt has not landed — still publishing, or already rejected raise, linking to the deployments page
central.sonatype.com published endpoint reports true already released skip the deploy
no such repo, and published reports false nothing in progress, nothing released deploy
  • If the repo is "closed" we could wait for it to either show published (and skip deploy) or failed (and error), but I think it is best to just error when it isn't in a known end state
  • The lookup must pass ip=any to ensure it sees attempts from different CI runners or local executions
  • Created a new Sonatype module for these methods so they aren't in the global namespace for all binding rake files.
  • The .NET guard from [release] Skip release if package is already published #17686 is removed because if the task is rerun on the CI we need to make sure the packages are generated for the GitHub release, and since //dotnet:publish uses --skip-duplicate, the guard seemed excessive to avoid a quick noop.

🤖 AI assistance

💡 Additional Considerations

  • Should update Releasing Selenium notes to suggest if releasing Java locally to also ensure ./go java:verify passes before rerunning
  • Could consider auto-dropping an open staging repository instead of erroring, but I'm not certain what that would look like, and now that our releases are more reliable, hopefully it won't be necessary.

🔄 Types of changes

  • Bug fix (backwards compatible)

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Aug 21, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Make Java/.NET release reruns safe when the version is already published

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Make Java release rerun-safe by detecting existing Sonatype deployments and published versions.
• Remove workflow/task guards so reruns still build release artifacts for GitHub releases.
• Add a shared HTTP request helper with timeouts for publish/verification calls.
Diagram

graph TD
  GA["release.yml"] --> GO["./go <lang>:release"] --> JAVA["java:release"] --> SONA("Sonatype APIs") --> MAVEN[("Maven Central")]
  GO --> DOTNET["dotnet:release"] --> NUGET[("NuGet.org")]
  subgraph Legend
    direction LR
    _wf["Workflow/Task"] ~~~ _svc("External service") ~~~ _db[("Package registry")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep Maven Central (repo1) publish check only
  • ➕ No Sonatype API auth or JSON parsing required
  • ➕ Less dependency on Sonatype Portal API stability
  • ➖ Indexing lag makes it inherently race-prone for reruns
  • ➖ Cannot detect/handle leftover open/closed staging repos
2. Auto-drop open/closed staging repositories on rerun
  • ➕ Could fully self-heal abandoned staging repos without human intervention
  • ➕ Avoids blocking releases on manual cleanup
  • ➖ Higher risk: automated deletion could remove a legitimate in-progress deploy
  • ➖ Needs careful selection criteria and auditing to avoid data loss
3. Wait/poll for closed staging repos to reach a terminal state
  • ➕ Could turn some transient 'closed' states into a clean skip/continue outcome
  • ➕ Less manual intervention for slow publishes
  • ➖ Adds timeouts and longer CI runs; still may end in ambiguous states
  • ➖ More complex control flow and error handling

Recommendation: The PR’s approach is the best balance for CI rerun safety: fail fast on non-terminal staging states (open/closed) to force human inspection, and skip deploy when the Portal reports the version is already published. This avoids Maven Central indexing races and prevents piling new deploys onto a potentially broken Sonatype state, while still ensuring GitHub release artifacts are rebuilt on reruns.

Files changed (4) +113 / -96

Bug fix (2) +101 / -88
dotnet.rakeAlways build/package on release reruns instead of skipping +0/-16

Always build/package on release reruns instead of skipping

• Removes the pre-check that invoked dotnet:verify and skipped release when packages already existed. This ensures reruns still generate build/dist artifacts for the GitHub release while publish remains effectively idempotent.

rake_tasks/dotnet.rake

java.rakeMake Java release idempotent via Sonatype staging + published checks +101/-72

Make Java release idempotent via Sonatype staging + published checks

• Adds a Sonatype module to encapsulate credential loading, API requests, staging repository inspection, and published-version detection. Java release now always builds/packages, skips deploy when the version is already published, and errors when a previous attempt left open/closed staging repos; it then triggers Sonatype automatic publishing when a deploy is performed.

rake_tasks/java.rake

Refactor (1) +12 / -4
common.rbAdd reusable HTTP GET helper with timeouts +12/-4

Add reusable HTTP GET helper with timeouts

• Introduces SeleniumRake.get_request to accept either a URL or a pre-built Net::HTTPRequest, enabling custom headers. Updates verify_package_published to use this helper and standardize timeouts.

rake_tasks/common.rb

Other (1) +0 / -4
release.ymlRemove Java rerun-blocking guard in release workflow +0/-4

Remove Java rerun-blocking guard in release workflow

• Deletes the conditional that hard-failed Java publishes on workflow reruns. Reruns can now proceed to run the Java release task, relying on the improved rerun-safe logic in the rake task.

.github/workflows/release.yml

@qodo-code-review

qodo-code-review Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Sonatype deploy logic untested 📘 Rule violation ☼ Reliability
Description
The PR adds new rerun-safety behavior and external HTTP decision points for Java releases (e.g.,
Sonatype.already_deployed?, Sonatype.published?, and SeleniumRake.get_request) but does not
add/update automated tests to validate these paths. This increases the risk of release regressions
and failures when endpoints or response shapes change.
Code

rake_tasks/java.rake[R347-350]

+  next if !nightly && Sonatype.already_deployed?(java_version)
+
+  puts "Deploying Java artifacts to '#{ENV.fetch('MAVEN_REPO', nil)}'"
  java_release_targets.each { |target| Bazel.execute('run', ['--config=release'], target) }
Evidence
PR Compliance ID 6 requires adding/updating tests when behavior changes. The PR introduces new HTTP
request helpers and new release decision logic
(already_deployed?/published?/staging_repositories) that changes whether a deploy is executed,
but there are no corresponding test changes in this PR to validate those behaviors.

AGENTS.md: Add/Update Tests for Implemented Changes; Prefer Small Unit Tests
rake_tasks/java.rake[76-180]
rake_tasks/common.rb[124-141]

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 Java release rerun-safety logic introduces new control flow and HTTP interactions, but there are no automated tests covering the new behaviors (e.g., skipping deploy when already published, erroring on `open`/`closed` staging repos, JSON parsing/HTTP error handling).

## Issue Context
This code governs release automation; regressions can block releases or cause incorrect publish attempts. Adding small, focused tests (ideally using a lightweight local HTTP server or a narrowly-scoped stub that reflects real response shapes) would reduce risk.

## Fix Focus Areas
- rake_tasks/java.rake[76-180]
- rake_tasks/common.rb[124-141]
- rb/spec/unit/rake_tasks/sonatype_spec.rb[1-200]

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


2. Prepared request may crash ✗ Dismissed 🐞 Bug ☼ Reliability
Description
SeleniumRake.get_request assumes any Net::HTTPRequest has a non-nil request.uri; if a caller builds
a request without a URI (common when constructing requests from just a path), this will raise at
runtime and break release-time HTTP checks. This is risky because the method’s comment explicitly
advertises accepting a “prepared request” for custom headers.
Code

rake_tasks/common.rb[R124-127]

+  # Takes a url or a prepared request, so callers needing headers can build their own.
+  def self.get_request(target)
+    request = target.is_a?(Net::HTTPRequest) ? target : Net::HTTP::Get.new(URI(target))
+    uri = request.uri
Evidence
The new helper explicitly claims it accepts a “prepared request”, but it immediately dereferences
request.uri and uses it to open the connection. The new Sonatype JSON helper passes a
Net::HTTP::Get request object into SeleniumRake.get_request, making this contract important to
keep correct and future-proof.

rake_tasks/common.rb[124-133]
rake_tasks/java.rake[143-149]

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

### Issue description
`SeleniumRake.get_request` supports passing a pre-built `Net::HTTPRequest`, but it unconditionally reads `request.uri`. Some valid `Net::HTTPRequest` instances (e.g., created with a path rather than a full `URI`) will have no `uri`, causing a `NoMethodError`/`nil` failure at runtime.

### Issue Context
This helper is now used by `Sonatype.request_json`, so a runtime crash here would fail the Java release rerun-safety checks.

### Fix Focus Areas
- rake_tasks/common.rb[124-133]

### Suggested fix
- Make `get_request` robust to requests without a URI:
 - Option A (most explicit): change the API to `get_request(uri, request)` (or `get_request(url, request=nil)`), where `uri` is always present.
 - Option B: if `target` is a `Net::HTTPRequest` and `target.uri` is nil, raise a clear error telling callers to construct with a `URI` (or pass a URL string), instead of failing with a confusing nil dereference.
 - Option C: infer the URI from `target['Host']` + scheme default + `target.path` if present (only if you’re confident this is safe/desired for all callers).

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



Informational

3. NuGet push always reruns ✗ Dismissed 🐞 Bug ☼ Reliability
Description
The .NET release task no longer short-circuits when packages are already published, so reruns will
always invoke the NuGet push step and depend on NuGet availability even when nothing needs
publishing. This increases rerun failure surface and external API load versus only
rebuilding/staging artifacts when already published.
Code

rake_tasks/dotnet.rake[L39-42]

-  unless nightly
-    already_published = begin
-      Rake::Task['dotnet:verify'].invoke
-      true
Evidence
The removed block previously invoked dotnet:verify to decide whether to skip releasing; it is now
gone, and the task always executes //dotnet:publish. The publish implementation uses `dotnet nuget
push --skip-duplicate`, so this will often no-op, but it still performs external calls and can fail
for unrelated network/auth issues.

rake_tasks/dotnet.rake[35-55]
dotnet/private/nuget_push.bzl[37-46]

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

### Issue description
`dotnet:release` used to detect “already published” and exit early. After the change, reruns always run `//dotnet:publish`, which still performs network calls to NuGet even if duplicates are skipped.

### Issue Context
The PR goal is to always generate artifacts for GitHub releases on reruns; that can be achieved without always attempting a push when the version is already published.

### Fix Focus Areas
- rake_tasks/dotnet.rake[35-55]

### Suggested fix
- Keep the current behavior of always building/packaging, but restore an “already published” check to skip only the push step:
 - Run `Rake::Task['dotnet:package']` unconditionally.
 - If `!nightly` and `dotnet:verify` succeeds, print a message and skip `Bazel.execute('run', ..., '//dotnet:publish')`.
 - Otherwise run the publish step as today.
- Ensure you reenable `dotnet:verify` if invoked inside `dotnet:release`.

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


Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread rake_tasks/java.rake
Comment thread rake_tasks/common.rb
Comment thread rake_tasks/dotnet.rake
@titusfortner
titusfortner merged commit 949d4c1 into trunk Aug 21, 2026
30 checks passed
@titusfortner
titusfortner deleted the java-release-rerun-safety branch August 21, 2026 23:28
This was referenced Aug 28, 2026
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants