Skip to content

[build] fix the issues that caused the release workflow to fail during last release - #17921

Merged
titusfortner merged 3 commits into
trunkfrom
release-mirror-and-java-verify
Aug 18, 2026
Merged

[build] fix the issues that caused the release workflow to fail during last release#17921
titusfortner merged 3 commits into
trunkfrom
release-mirror-and-java-verify

Conversation

@titusfortner

@titusfortner titusfortner commented Aug 17, 2026

Copy link
Copy Markdown
Member

💥 What does this PR do?

Fixes two things in the release pipeline that required manual intervention during the 4.47.0 release.

  • The release mirror update now pushes successfully instead of being rejected as a non-fast-forward.
  • Verifying the Java release now waits for Maven Central to index the deployment instead of failing on the first 404.

🔧 Implementation Notes

  • Maven Central is the only registry that indexes asynchronously, so the retry lives in the java:verify task and the shared verification helper is untouched.
  • The already-published pre-check in java:release now calls the underlying helper directly rather than invoking java:verify, so it stays a single request instead of inheriting the new polling. Its coverage is unchanged; making it reliable for Java needs the Sonatype staging API and is tracked separately ([build] make release pipeline rerun-safe #17626).
  • The 10-minute timeout should be sufficient. Verification runs ~10 minutes after the upload is triggered, and previous releases with data were satisfied within 13 minutes.
  • The mirror job's push target is now a literal trunk rather than github.ref. The checkout ref is the actual fix; pinning the push branch keeps the two from being derived differently, since updating common/mirror/selenium on trunk is the job's only purpose.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the workflow change and the verification polling, plus the log analysis that identified both failures
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

For follow on PRs:

  • Make update code a separate weekly execution rather than part of the release process.

🔄 Types of changes

  • Bug fix (backwards compatible)

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

qodo-code-review Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. java:release pre-check untested 📘 Rule violation ☼ Reliability ⭐ New
Description
The new direct Maven Central pre-check path is not covered by tests confirming that it performs a
single check and preserves release idempotency without invoking the polling task. A regression could
either redeploy an existing version or make release wait unnecessarily.
Code

rake_tasks/java.rake[284]

+      SeleniumRake.verify_package_published(maven_central_pom_url)
Evidence
Compliance rule 4 requires tests for implemented behavior changes. The changed release path at line
284 bypasses java:verify and directly uses the newly extracted Maven Central URL helper, but the
PR contains no corresponding test update for this behavior.

AGENTS.md: Add or Update Tests for Implemented Changes (Prefer Small Unit Tests, Avoid Mocks)
rake_tasks/java.rake[284-284]
rake_tasks/java.rake[328-330]

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 new `java:release` pre-check directly calls `verify_package_published`, but no test validates its single-check behavior or published/unpublished outcomes.

## Issue Context
This path must remain separate from the retrying `java:verify` task while still skipping releases whose artifacts already exist.

## Fix Focus Areas
- rake_tasks/java.rake[284-284]
- rake_tasks/java.rake[328-330]

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


2. Release not idempotent ✗ Dismissed 🐞 Bug ☼ Reliability
Description
The java:release task no longer checks whether the Java artifacts are already published before
deploying, so re-running the release workflow for an existing tag will attempt to deploy again
instead of skipping. This can cause the release pipeline to fail mid-run and require manual
cleanup/recovery.
Code

rake_tasks/java.rake[L282-285]

-  unless nightly
-    already_published = begin
-      Rake::Task['java:verify'].invoke
-      true
Evidence
The PR deletes the prior java:verify-based early-exit in java:release. The release workflow
explicitly allows tag creation to be skipped if the tag already exists, but still runs `./go
java:release, and ./go` directly executes the corresponding Rake task, so re-dispatching a release
for an existing tag will now proceed into deployment instead of skipping.

rake_tasks/java.rake[277-313]
.github/workflows/release.yml[76-155]
go[13-43]

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

### Issue description
`java:release` used to short-circuit when Maven Central already had the target version (by invoking `java:verify`). This PR removes that, making releases non-idempotent and allowing an operator to re-run a release for the same tag/version and attempt a second deploy.

### Issue Context
The GitHub Actions release workflow can be re-dispatched for a tag that already exists; tag creation is skipped but publishing still runs `./go java:release`, which invokes the `java:release` Rake task.

### Fix Focus Areas
- rake_tasks/java.rake[277-313]
- .github/workflows/release.yml[76-155]
- go[13-43]

### Suggested fix
Reintroduce a safe preflight in `java:release` for non-nightly runs (or add an explicit `force` argument):
- If `java:verify` succeeds, print a clear message and abort/skip.
- If `java:verify` fails with a "not yet published" condition, continue with release.
Alternatively (or additionally), add a workflow-level guard in `.github/workflows/release.yml` to prevent `./go java:release` when the version is already present on Maven Central.

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



Remediation recommended

3. Broad verify retry loop 🐞 Bug ☼ Reliability
Description
java:verify now retries for up to 10 minutes on any StandardError, so non-indexing failures
(e.g., DNS/TLS/network exceptions from Net::HTTP) will be repeatedly retried instead of failing
promptly. This increases release verification latency and makes real outages look like "still
indexing" for up to 10 minutes.
Code

rake_tasks/java.rake[R319-322]

+  begin
+    SeleniumRake.verify_package_published("#{base}/#{java_version}/selenium-java-#{java_version}.pom")
+  rescue StandardError => e
+    if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline
Evidence
The retry loop is triggered for any StandardError. The underlying helper uses Net::HTTP.start,
which can raise networking-related StandardErrors, meaning the new logic will retry those too.

rake_tasks/java.rake[314-330]
rake_tasks/common.rb[124-133]

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 new polling loop rescues `StandardError` broadly and treats any failure as potentially transient Maven indexing. This will also retry genuine connectivity/SSL/DNS issues coming from `Net::HTTP`, delaying failures and producing misleading logs.

### Issue Context
`SeleniumRake.verify_package_published` performs an HTTP GET via `Net::HTTP.start` and raises on non-2xx responses; it can also raise network exceptions.

### Fix Focus Areas
- rake_tasks/java.rake[314-330]
- rake_tasks/common.rb[124-133]

### Suggested fix
Narrow the retry condition to known/expected "not yet available" cases:
- Prefer raising/handling a dedicated exception type (e.g., `PackageNotPublishedError`) from `verify_package_published` when the HTTP response is non-success.
- In `java:verify`, retry only for that exception (or only when the message matches the not-published condition), and fail fast for other exceptions.
- Optionally include HTTP status/details in the raised error so logs distinguish 404 from other failures.

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


4. Timeout obscures primary error ✗ Dismissed 🐞 Bug ◔ Observability
Description
When the verify deadline is exceeded, the code raises a new string RuntimeError, making the
timeout raise site/backtrace the primary reported failure instead of the underlying exception type.
While Ruby may preserve the original exception as cause, many CI outputs won’t surface it clearly,
reducing diagnosability.
Code

rake_tasks/java.rake[R322-324]

+    if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline
+      raise "#{e.message}; check https://central.sonatype.com/publishing/deployments"
+    end
Evidence
The new code replaces the underlying exception with a new raised string on timeout, which changes
the top-level exception/backtrace presented to users.

rake_tasks/java.rake[314-330]

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

### Issue description
On timeout, `java:verify` raises a new string exception. This changes the primary exception class/backtrace to the timeout line, and the original exception details may be hidden in typical CI output.

### Issue Context
The retry loop is intended to add guidance (Sonatype deployments URL) when Maven Central indexing takes too long.

### Fix Focus Areas
- rake_tasks/java.rake[319-329]

### Suggested fix
Raise an exception that preserves the original error context:
- Include `e.class` in the raised message.
- Prefer `raise(RuntimeError.new("..."), cause: e)` (or re-raise the original with appended guidance) so the underlying error remains visible via exception cause chains.
- Optionally log `e.backtrace.first(...)` before raising to ensure CI logs capture the root cause.

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


5. java:verify retry untested 📘 Rule violation ☼ Reliability
Description
The new Maven Central polling/retry behavior in java:verify is a behavioral change but this PR
does not add or update any tests to validate the retry/timeout logic. This increases regression risk
for future releases (e.g., unexpected retry loops, premature failure, or timing edge cases).
Code

rake_tasks/java.rake[R317-320]

+  deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + 600
+
+  begin
+    SeleniumRake.verify_package_published("#{base}/#{java_version}/selenium-java-#{java_version}.pom")
Evidence
PR Compliance ID 3 requires behavioral changes to be accompanied by tests. The diff adds a retry
loop with a deadline and sleep/retry behavior in java:verify, but no corresponding test
additions/updates are present in this PR.

AGENTS.md: Add/Update Tests for Implemented Solutions and Prefer Small (Unit) Tests
rake_tasks/java.rake[314-330]

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

## Issue description
`java:verify` now retries Maven Central publication checks for up to 10 minutes, but this new behavior is not covered by tests.

## Issue Context
The retry/timeout loop is release-critical logic and failures will only surface during a release unless validated with automated coverage.

## Fix Focus Areas
- rake_tasks/java.rake[314-330]

ⓘ 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 keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous review results

Review updated until commit 5ee5917

Results up to commit e1c8ff2 ⚖️ Balanced


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


Action required
1. Release not idempotent ✗ Dismissed 🐞 Bug ☼ Reliability
Description
The java:release task no longer checks whether the Java artifacts are already published before
deploying, so re-running the release workflow for an existing tag will attempt to deploy again
instead of skipping. This can cause the release pipeline to fail mid-run and require manual
cleanup/recovery.
Code

rake_tasks/java.rake[L282-285]

-  unless nightly
-    already_published = begin
-      Rake::Task['java:verify'].invoke
-      true
Evidence
The PR deletes the prior java:verify-based early-exit in java:release. The release workflow
explicitly allows tag creation to be skipped if the tag already exists, but still runs `./go
java:release, and ./go` directly executes the corresponding Rake task, so re-dispatching a release
for an existing tag will now proceed into deployment instead of skipping.

rake_tasks/java.rake[277-313]
.github/workflows/release.yml[76-155]
go[13-43]

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

### Issue description
`java:release` used to short-circuit when Maven Central already had the target version (by invoking `java:verify`). This PR removes that, making releases non-idempotent and allowing an operator to re-run a release for the same tag/version and attempt a second deploy.

### Issue Context
The GitHub Actions release workflow can be re-dispatched for a tag that already exists; tag creation is skipped but publishing still runs `./go java:release`, which invokes the `java:release` Rake task.

### Fix Focus Areas
- rake_tasks/java.rake[277-313]
- .github/workflows/release.yml[76-155]
- go[13-43]

### Suggested fix
Reintroduce a safe preflight in `java:release` for non-nightly runs (or add an explicit `force` argument):
- If `java:verify` succeeds, print a clear message and abort/skip.
- If `java:verify` fails with a "not yet published" condition, continue with release.
Alternatively (or additionally), add a workflow-level guard in `.github/workflows/release.yml` to prevent `./go java:release` when the version is already present on Maven Central.

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



Remediation recommended
2. Broad verify retry loop 🐞 Bug ☼ Reliability
Description
java:verify now retries for up to 10 minutes on any StandardError, so non-indexing failures
(e.g., DNS/TLS/network exceptions from Net::HTTP) will be repeatedly retried instead of failing
promptly. This increases release verification latency and makes real outages look like "still
indexing" for up to 10 minutes.
Code

rake_tasks/java.rake[R319-322]

+  begin
+    SeleniumRake.verify_package_published("#{base}/#{java_version}/selenium-java-#{java_version}.pom")
+  rescue StandardError => e
+    if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline
Evidence
The retry loop is triggered for any StandardError. The underlying helper uses Net::HTTP.start,
which can raise networking-related StandardErrors, meaning the new logic will retry those too.

rake_tasks/java.rake[314-330]
rake_tasks/common.rb[124-133]

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 new polling loop rescues `StandardError` broadly and treats any failure as potentially transient Maven indexing. This will also retry genuine connectivity/SSL/DNS issues coming from `Net::HTTP`, delaying failures and producing misleading logs.

### Issue Context
`SeleniumRake.verify_package_published` performs an HTTP GET via `Net::HTTP.start` and raises on non-2xx responses; it can also raise network exceptions.

### Fix Focus Areas
- rake_tasks/java.rake[314-330]
- rake_tasks/common.rb[124-133]

### Suggested fix
Narrow the retry condition to known/expected "not yet available" cases:
- Prefer raising/handling a dedicated exception type (e.g., `PackageNotPublishedError`) from `verify_package_published` when the HTTP response is non-success.
- In `java:verify`, retry only for that exception (or only when the message matches the not-published condition), and fail fast for other exceptions.
- Optionally include HTTP status/details in the raised error so logs distinguish 404 from other failures.

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


3. Timeout obscures primary error ✗ Dismissed 🐞 Bug ◔ Observability
Description
When the verify deadline is exceeded, the code raises a new string RuntimeError, making the
timeout raise site/backtrace the primary reported failure instead of the underlying exception type.
While Ruby may preserve the original exception as cause, many CI outputs won’t surface it clearly,
reducing diagnosability.
Code

rake_tasks/java.rake[R322-324]

+    if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline
+      raise "#{e.message}; check https://central.sonatype.com/publishing/deployments"
+    end
Evidence
The new code replaces the underlying exception with a new raised string on timeout, which changes
the top-level exception/backtrace presented to users.

rake_tasks/java.rake[314-330]

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

### Issue description
On timeout, `java:verify` raises a new string exception. This changes the primary exception class/backtrace to the timeout line, and the original exception details may be hidden in typical CI output.

### Issue Context
The retry loop is intended to add guidance (Sonatype deployments URL) when Maven Central indexing takes too long.

### Fix Focus Areas
- rake_tasks/java.rake[319-329]

### Suggested fix
Raise an exception that preserves the original error context:
- Include `e.class` in the raised message.
- Prefer `raise(RuntimeError.new("..."), cause: e)` (or re-raise the original with appended guidance) so the underlying error remains visible via exception cause chains.
- Optionally log `e.backtrace.first(...)` before raising to ensure CI logs capture the root cause.

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


4. java:verify retry untested 📘 Rule violation ☼ Reliability
Description
The new Maven Central polling/retry behavior in java:verify is a behavioral change but this PR
does not add or update any tests to validate the retry/timeout logic. This increases regression risk
for future releases (e.g., unexpected retry loops, premature failure, or timing edge cases).
Code

rake_tasks/java.rake[R317-320]

+  deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + 600
+
+  begin
+    SeleniumRake.verify_package_published("#{base}/#{java_version}/selenium-java-#{java_version}.pom")
Evidence
PR Compliance ID 3 requires behavioral changes to be accompanied by tests. The diff adds a retry
loop with a deadline and sleep/retry behavior in java:verify, but no corresponding test
additions/updates are present in this PR.

AGENTS.md: Add/Update Tests for Implemented Solutions and Prefer Small (Unit) Tests
rake_tasks/java.rake[314-330]

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

## Issue description
`java:verify` now retries Maven Central publication checks for up to 10 minutes, but this new behavior is not covered by tests.

## Issue Context
The retry/timeout loop is release-critical logic and failures will only surface during a release unless validated with automated coverage.

## Fix Focus Areas
- rake_tasks/java.rake[314-330]

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


Qodo Logo

Comment thread rake_tasks/java.rake Outdated
Comment thread rake_tasks/java.rake
Comment thread rake_tasks/java.rake
Comment thread rake_tasks/java.rake
Comment thread rake_tasks/java.rake
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner titusfortner changed the title [build] fix release mirror push target and poll Maven Central when verifying Java [build] fix the issues that caused the release workflow to fail during last release Aug 18, 2026
@titusfortner
titusfortner merged commit 6962623 into trunk Aug 18, 2026
27 checks passed
@titusfortner
titusfortner deleted the release-mirror-and-java-verify branch August 18, 2026 01:49
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