Skip to content

[build] move Selenium Manager publishing into sm-snapshot.yml so releases no longer skip it - #18017

Merged
titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:sm-snapshot
Sep 10, 2026
Merged

titusfortner merged 2 commits into
SeleniumHQ:trunkfrom
titusfortner:sm-snapshot

Conversation

@titusfortner

@titusfortner titusfortner commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

Regressed from a710cbd — 4.49.0 shipped a Selenium Manager that reports 0.4.49-nightly because the release binaries were built but never published.

💥 What does this PR do?

  • Moves the Selenium Manager build and publish jobs out of ci-rust.yml into a new top-level sm-snapshot.yml that runs on every trunk push touching rust/, so publishing no longer sits downstream of a test job that release prep skips.
  • ci-rust.yml is now the Rust test matrix only.

🔧 Implementation Notes

  • Skipping tests on release left the publish job skipped too: GitHub adds an implicit success() to any job condition without a status-check function, and that is false when anything in the transitive needs chain was skipped. The prep run's update-manager job then pinned whatever the artifacts repo called latest, which was a trunk build from an hour earlier.
  • Alternatives considered: adding !cancelled() && !contains(needs.*.result, 'failure') && !contains(needs.*.result, 'skipped') to the publish job fixes it in one line but keeps a skippable job upstream of publishing; moving the Rust tests into ci.yml also works but scatters the Rust CI across two files.
  • With the test job gone from the publish workflow, every !cancelled(), schedule, and trunk || inputs.release guard from a710cbd goes away; the only condition left is the fork check on the publish job.
  • Release prep keeps calling the workflow on its rust-release-* branch through workflow_call, unchanged in substance. Trunk pushes now publish without waiting on the trunk Rust test run; the commit already passed those tests in PR CI.
  • Runs for the same branch queue behind each other (concurrency without cancel), so a slower older run cannot publish over a newer one and every merge still gets a snapshot; inside ci.yml the old publish job was serialized by that workflow's cancel-in-progress group.
  • The three stable cargo builds are not kept in ci-rust.yml as a PR compile check; bazel test //rust/... already compiles on all four hosts, and a musl or i686 link failure shows up on the next trunk snapshot without moving the pin.

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code (Fable 5.1)
    • What was generated: diagnosis from the workflow run history, the workflow split, and this description
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • First step of the plan to have sm-snapshot.yml pin trunk automatically after each publish and to cross-compile Selenium Manager in Bazel so release packaging stops depending on a pinned download; the workflow_call trigger is deleted once that lands.
  • The 4.49.0 binaries are functionally identical to the 0.4.49 build apart from the version string; no re-release planned.

🔄 Types of changes

  • Bug fix (backwards compatible)

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

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Decouple Selenium Manager publishing from Rust tests

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

Grey Divider

AI Description

• Separates Selenium Manager publishing from the skippable Rust test workflow.
• Publishes snapshots after trunk Rust changes and during release preparation.
• Keeps Rust CI focused on the cross-platform Bazel test matrix.
Diagram

graph TD
  A["Main CI"] --> B["Rust tests"]
  C["Trunk push"] --> E["SM Snapshot"] --> F["Build artifacts"] --> G["Publish snapshot"] --> H[("Artifacts repo")]
  D["Pre-release"] --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Relax the publish job condition
  • ➕ Requires a minimal one-line workflow change.
  • ➕ Avoids moving and duplicating workflow context.
  • ➖ Leaves publishing coupled to a skippable test dependency chain.
  • ➖ Requires subtle status-function logic that is easy to regress.
  • ➖ Keeps unrelated testing and release responsibilities in one workflow.
2. Move Rust tests into ci.yml
  • ➕ Allows publishing to remain in ci-rust.yml without depending on tests.
  • ➕ Keeps the release workflow filename unchanged.
  • ➖ Scatters Rust CI ownership across multiple workflow files.
  • ➖ Makes the main CI workflow larger and more language-specific.
  • ➖ Provides less separation between validation and artifact publication.

Recommendation: The dedicated sm-snapshot.yml workflow is the strongest approach because it cleanly separates validation from publication and removes skipped-job semantics from the release path. The additional workflow file is justified by clearer ownership and reliable trunk and release publishing.

Files changed (5) +321 / -323

Bug fix (2) +320 / -2
pre-release.ymlInvoke the dedicated Selenium Manager snapshot workflow +1/-2

Invoke the dedicated Selenium Manager snapshot workflow

• Routes release-branch Selenium Manager builds through sm-snapshot.yml and removes the obsolete release flag. The generated rust-release branch and publishing token continue to be passed through.

.github/workflows/pre-release.yml

sm-snapshot.ymlAdd an independent Selenium Manager publishing pipeline +319/-0

Add an independent Selenium Manager publishing pipeline

• Introduces trunk, reusable, and manual triggers for building stable and debug binaries across Windows, Linux, and macOS. It also generates SBOM and notice artifacts, computes hashes, updates the artifact repository, and publishes a tagged snapshot without depending on Rust tests.

.github/workflows/sm-snapshot.yml

Documentation (1) +1 / -1
MODULE.bazelPoint Rust toolchain synchronization guidance to snapshot workflow +1/-1

Point Rust toolchain synchronization guidance to snapshot workflow

• Updates the Rust toolchain comment to identify sm-snapshot.yml as the workflow whose Rust version must remain synchronized.

MODULE.bazel

Other (2) +0 / -320
ci-rust.ymlReduce Rust CI to the Bazel test matrix +0/-318

Reduce Rust CI to the Bazel test matrix

• Removes Selenium Manager binary builds, SBOM generation, publishing, release inputs, secrets, and associated conditions. The reusable workflow now solely runs Rust tests across four operating-system runners.

.github/workflows/ci-rust.yml

ci.ymlStop forwarding publishing credentials to Rust tests +0/-2

Stop forwarding publishing credentials to Rust tests

• Removes the Selenium CI token from the main CI invocation because ci-rust.yml no longer publishes artifacts.

.github/workflows/ci.yml

@qodo-code-review

qodo-code-review Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Some trunk merges never publish snapshots ✗ Dismissed 🐞 Bug ≡ Correctness ⭐ New
Description
The workflow-level concurrency block leaves GitHub's default pending-run policy in place, so a newly
queued snapshot replaces an older pending snapshot even with cancel-in-progress: false. When
several Rust pushes arrive while one run is building, only the currently running run and newest
pending run execute, leaving intermediate merges without their promised Selenium Manager snapshot.
Code

.github/workflows/sm-snapshot.yml[R26-28]

+concurrency:
+  group: sm-snapshot-${{ inputs.branch || github.ref_name }}
+  cancel-in-progress: false
Evidence
The workflow defines one concurrency group per branch and disables cancellation only for the running
job, but does not configure the pending queue size. GitHub's documentation states that the default
queue allows at most one pending run and cancels/replaces the existing pending run when a new run is
queued.

.github/workflows/sm-snapshot.yml[26-28]
🌐 GitHub documents that the default concurrency queue has at most one pending run and that a newly queued run cancels and replaces the existing pending run; it also documents the queue setting for allowing multiple pending runs.

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 `sm-snapshot` concurrency group sets `cancel-in-progress: false`, but GitHub Actions still retains only one pending run by default and replaces that pending run when another run enters the same group. Rapid trunk pushes can therefore skip intermediate snapshot publishes, contrary to the workflow's requirement that every merge gets a snapshot.

## Fix Focus Areas
- .github/workflows/sm-snapshot.yml[26-28]

## Recommended Fix
Configure the concurrency group to permit multiple pending runs, using the supported queue setting (for example, `queue: max`) alongside `cancel-in-progress: false`, or otherwise implement an equivalent FIFO queue. Ensure the configuration remains valid for both trunk push and reusable workflow-call executions.

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


2. Older snapshots can become latest ✓ Resolved 🐞 Bug ☼ Reliability
Description
sm-snapshot.yml has no workflow-level concurrency guard, while publish updates the same
artifact-repository branch and latest release from every run. Two rapid Rust pushes can therefore
build concurrently and either make a git push fail from a stale checkout or let an older, slower
run publish after a newer one, replacing the latest hashes and release with stale binaries.
Code

.github/workflows/sm-snapshot.yml[R299-300]

+          git tag "selenium-manager-$short_hash"
+          git push && git push --tags
Evidence
The new workflow starts independently for every Rust-changing trunk push, but unlike the main CI
workflow it has no concurrency declaration. Every run checks out the same artifact repository,
overwrites latest.json, tags the resulting commit, and pushes that shared branch, while consumers
read that branch and the repository's latest release; this directly exposes both non-fast-forward
failures and completion-order regressions.

.github/workflows/sm-snapshot.yml[3-18]
.github/workflows/sm-snapshot.yml[269-307]
.github/workflows/ci.yml[15-17]
scripts/selenium_manager.py[15-28]

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 standalone snapshot workflow permits multiple runs to publish concurrently to the same artifact repository. Concurrent runs can fail with non-fast-forward pushes or allow an older build to replace a newer snapshot.

## Fix Focus Areas
- .github/workflows/sm-snapshot.yml[3-18]
- .github/workflows/sm-snapshot.yml[269-300]

## Recommended Fix
Add a shared workflow-level concurrency group that covers push, dispatch, and reusable invocations without cancelling a run during its publish transaction. Also verify before publishing a trunk snapshot that its source revision has not been superseded, so a slower older build cannot replace the newest snapshot.

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


Grey Divider

Context sources
✅ Web pages:
  +2 more
Review mode: 🚀 Fast: This is a localized four-line workflow concurrency configuration change affecting one execution-order concern, with no broad or security-sensitive logic.

Grey Divider

Tip of the day
💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 5eaac48

Results up to commit a911202 ⚖️ Balanced


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


Action required
1. Older snapshots can become latest ✓ Resolved 🐞 Bug ☼ Reliability
Description
sm-snapshot.yml has no workflow-level concurrency guard, while publish updates the same
artifact-repository branch and latest release from every run. Two rapid Rust pushes can therefore
build concurrently and either make a git push fail from a stale checkout or let an older, slower
run publish after a newer one, replacing the latest hashes and release with stale binaries.
Code

.github/workflows/sm-snapshot.yml[R299-300]

+          git tag "selenium-manager-$short_hash"
+          git push && git push --tags
Evidence
The new workflow starts independently for every Rust-changing trunk push, but unlike the main CI
workflow it has no concurrency declaration. Every run checks out the same artifact repository,
overwrites latest.json, tags the resulting commit, and pushes that shared branch, while consumers
read that branch and the repository's latest release; this directly exposes both non-fast-forward
failures and completion-order regressions.

.github/workflows/sm-snapshot.yml[3-18]
.github/workflows/sm-snapshot.yml[269-307]
.github/workflows/ci.yml[15-17]
scripts/selenium_manager.py[15-28]

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 standalone snapshot workflow permits multiple runs to publish concurrently to the same artifact repository. Concurrent runs can fail with non-fast-forward pushes or allow an older build to replace a newer snapshot.

## Fix Focus Areas
- .github/workflows/sm-snapshot.yml[3-18]
- .github/workflows/sm-snapshot.yml[269-300]

## Recommended Fix
Add a shared workflow-level concurrency group that covers push, dispatch, and reusable invocations without cancelling a run during its publish transaction. Also verify before publishing a trunk snapshot that its source revision has not been superseded, so a slower older build cannot replace the newest snapshot.

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


Grey Divider

Qodo Logo

Comment thread .github/workflows/sm-snapshot.yml
Comment thread .github/workflows/sm-snapshot.yml
@qodo-code-review

Copy link
Copy Markdown
Contributor

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

@titusfortner
titusfortner merged commit 3221998 into SeleniumHQ:trunk Sep 10, 2026
47 checks passed
This was referenced Oct 1, 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