vs/Automate Firefox train branch management

[vsangereanMOZ]

Relevant Links

Bugzilla: _
TestRail: _

Description of Code / Doc Changes

This PR adds a minimal POC for automated Firefox train branch management.

Branch model:

• Nightly runs from the nightly branch.
• Beta runs from the main branch.
• Released Firefox versions use releaseNNN branches.

When Nightly is promoted to Beta, the workflow:

• Preserves the current main commit as releaseNNN.
• Updates main to the current nightly commit.
• Leaves nightly available for the next development cycle.
• Removes old releaseNNN branches according to the retention value.

###Triggers

The workflow uses workflow_dispatch. It can be started manually from GitHub Actions or through the GitHub Actions workflow dispatch API.
The API request must target the main branch and provide released_major.
retention is optional and defaults to 2.

dry_run is optional and defaults to true. A live promotion must explicitly provide dry_run as false.

[github-actions]

Review summary

This is a well-architected PR. The branch model (nightly/main/firefoxNNN) is clearly defined, the Python logic is cleanly separated from the workflow glue, and the unit tests cover the core promotion scenarios thoroughly. A few things worth addressing before merging:

Security

• resolve-firefox-branch.yml line 96: the channel input is interpolated directly into the bash script as a GitHub Actions expression (expression injection). The value is already available as FX_CHANNEL in the step env -- use that instead. See inline comment.
• sync-firefox-branches.yml line 47: ref: github.ref allows a workflow_dispatch user to run the branch-management code from any branch they choose, while the job has contents: write permission. For schedule and repository_dispatch this defaults to the default branch and is fine; for workflow_dispatch it is worth restricting.

Bugs

• collect_executables.py line 352: line_text[0][-2] extracts only the second-to-last character of the link text (e.g. build1/ yields 1). This silently breaks for two-digit build numbers like build10/ (returns 1 not 10). Pre-existing, but this file is being actively modified so a good time to fix.
• collect_executables.py line 244: the DevEdition discovery loop starts at a hardcoded 135.0b5, far behind BACKSTOP = 146.0b9. This causes unnecessary network round-trips on every run.

Minor

• sync-firefox-branches.yml line 188: unquoted DELETE_BRANCHES relies on bash word-splitting; the branch-name validation guard makes it safe in practice, but making the splitting explicit would be more robust.

[github-actions]

Review: Firefox Train Branch Management

This is a well-designed CI system with strong security fundamentals: immutable SHA pinning, atomic branch pushes with post-push validation, regex-guarded branch deletion, and comprehensive unit tests. A few things to address:

Issues found:

1. Shell injection pattern in resolve-firefox-branch.yml line 125 — the workflow expression inputs.channel is substituted into a bash heredoc at parse time. If this input ever became user-controllable via a workflow_dispatch wrapper, shell metacharacters in the value would execute. Assign to an env var first and reference $CHANNEL in the heredoc. See inline comment.

2. Stale BACKSTOP constant (collect_executables.py line 27) — 146.0b9 is roughly 10 major versions behind current (156). The discovery loop issues one HTTP request per major version, so this adds ~10 extra archive.mozilla.org requests on every scheduled run. Update to a recent beta.

3. Opaque merge failure in sync-firefox-branches.yml line 157 — git merge --no-edit exits non-zero on merge conflicts, but the output won't identify conflicting files. A diagnostic message before the merge would help on-call engineers.

4. Merge runs before dry-run check in promote step — lines 154-158 perform a real local merge before the DRY_RUN guard at line 160. This is harmless (local-only), but dry-run can surface merge conflicts. Worth noting in the dry-run echo message.

Minor:

• firefox_branches.py _retention_deletes — silently ignores include_branch when it is already in refs. Correct idempotent behavior, but a brief comment would prevent confusion.
• nightly-tests.yml intentionally skips Linux. If Nightly Linux coverage is ever needed, the matrix is the place to add it.

[github-actions]

Review: Firefox train branch management

Overall this is a well-designed implementation with good safety mechanisms: atomic pushes during promotion, SHA validation for immutable refs, dry-run mode, and branch name allow-listing before deletion. The Python logic is clean and well-tested. A few things to address:

Missing permissions: blocks

functional-test-beta.yml was explicitly updated to permissions: contents: read, but smoke-test-beta.yml, smoke-test-devedition.yml, smoke-test-rc.yml, and test-l10n-beta.yml all add a Resolve-Firefox job (which calls the reusable workflow that needs contents: read) without declaring a permissions: block. This works if the repo's default token permissions include contents: read, but it's fragile. Add explicit permissions: contents: read to each of those workflows for consistency and resilience.

Bug fix in collect_executables.py

The old build-number extraction logic (int(line_text[0][-2])) was extracting a single character by index — correctly only for single-digit build numbers. The new latest_candidate_build using re.fullmatch is a real fix for any build10+ scenario. Worth calling out as a bugfix in the PR description.

[github-actions]

Review

Well-structured PR with good separation of concerns: deterministic branch planning in Python (easily testable), shell execution in CI, and immutable commit pinning before artifact resolution. The atomic push for train promotion and the idempotency guard via the firefoxN archive branch are solid.

A few issues worth addressing:

Network error handling in discover_nightly_builds

requests.get() can raise requests.exceptions.RequestException (timeout, connection error, DNS failure) before a status code is available. The current code only checks status_code >= 300, so network-level failures will propagate as unhandled tracebacks rather than clean RuntimeError messages. The same applies to the metadata fetch loop. Wrapping the calls in try/except requests.exceptions.RequestException would give consistent error output in CI.

archive.invalid synthetic URLs lack a comment

Both resolve-firefox-branch.yml and sync-firefox-branches.yml construct artifact_stub="https://archive.invalid/firefox-..." and pass it to the Python scripts. Without a comment, maintainers may assume these are real network calls. A one-liner noting that .invalid is an RFC 2606 reserved TLD used only for version string extraction—no HTTP request is made—would save confusion.

Linux nightly tests silently omitted

nightly-tests.yml resolves linux_download_url through Resolve-Firefox but does not pass it to main.yml, and the matrix only contains Test-Windows / Test-MacOS. This is consistent with the PR description ("Test scheduling remains outside the scope"), but a brief comment in the workflow explaining that Linux nightly tests are intentionally deferred would avoid a future "why is Linux missing?" investigation.

Pinned FX_VERSION skips directory existence check

When FX_VERSION is set for beta/RC, collect_executables.py constructs the URL directly and goes straight to scraping the executable listing. If the specified build number does not exist, the failure is reported by the HTML scraper, not by an explicit 404 check. Adding a requests.get / status_code check on the constructed fx_download_dir_url before scraping (matching the existing pattern in the discovery path) would produce a clearer error message.

Minor: requests.exceptions.JSONDecodeError vs json.JSONDecodeError

requests.exceptions.JSONDecodeError was added in requests 2.28.0. If older versions are ever used, the except clause would silently not match. Using json.JSONDecodeError (which requests.exceptions.JSONDecodeError is a subclass of) is more portable.

[Tracy-Walker]

Removed myself from review list as I am not familiar with this area of code.

[github-actions]

Review: sync-firefox-branches.yml

The core mechanics are solid — --atomic push, --force-with-lease on main, idempotent re-runs via the archive branch guard, cancel-in-progress: false, and set -euo pipefail throughout. A few concrete issues:

Incorrect actions/checkout version comment (inline)

The pinned SHA has a # v6.0.2 comment, but actions/checkout has no v6 release (latest is v4). The comment should match the actual tag the SHA was taken from. See inline comment on line 47.

Missing persist-credentials comment (inline)

Every other checkout in this repo sets persist-credentials: false. The omission here is intentional (the push needs the token in git config), but without a comment it looks like an oversight to future reviewers. See inline comment on line 46.

Open PRs silently retargeted (inline)

After promotion, any PR targeting main silently follows main to the new Beta train. Adding a step that lists open main-targeting PRs to the job summary gives the operator visibility without blocking the workflow. See inline comment on line 161.

Note on scope

Ben has already flagged architectural concerns in his review. The issues above are independent of that discussion and apply regardless of how the final shape of the solution is settled.

[github-actions]

Review: Promote Firefox train branches workflow

Overall this is well-structured — good use of set -euo pipefail, pinned action hash, --atomic push, --force-with-lease to guard against races on main, concurrency group preventing concurrent promotions, and the stale/out-of-order promotion safeguard.

A couple of things worth addressing:

Best-practice security nit (Write summary step): The run: block uses ${{ steps.rotate.outputs.* }} directly. This is a GitHub Actions injection pattern to avoid — even though the values here are all validated/controlled (action is a hardcoded string, release_branch is "release"+digits, SHAs are hex), the safer convention is to pass them through env: vars so there is no risk if output shapes ever change. See inline comment.

Missing job timeout: The job has no timeout-minutes, so it inherits GitHub's 6-hour default. For a workflow that should complete in seconds, add something like timeout-minutes: 5 to fail fast on unexpected hangs.

Minor: The three git ls-remote calls (two for required-branch existence, one for release refs) could be collapsed into a single network round-trip, though this is not a meaningful concern in practice.

[github-actions]

Review

Overall this is a solid POC — the dry-run default, atomic push, force-with-lease, and stale-promotion guard are all good. A few things worth addressing:

No environment approval gate for live runs
The job has contents: write over the entire repository, and a single workflow_dispatch with dry_run: false updates main and nightly in one shot. Consider backing the live path with a GitHub Environment that requires one or more reviewers to approve before the push step executes. This gives a second pair of eyes on a branch-rotation before it lands.

released_major is not cross-validated against repo content
The user-supplied version number is only checked to be a positive integer; nothing confirms that main actually tracks that Firefox version. A wrong value silently creates a misnamed release branch. Even a brief comment advising operators to verify the version before triggering a live run would help; ideally a check against something authoritative (version file, tag naming convention) would be stronger.

Old-branch deletions have no lease
--force-with-lease covers only refs/heads/main. Branches in old_branches are deleted unconditionally — if someone pushed to a release branch between discovery and the git push, that work is lost. For stale release branches this is probably acceptable in practice, but it is worth a comment explaining the deliberate choice.

[vsangereanMOZ]

Made some new changes.

Changes made:

• Live runs now require approval through the firefox-branch-promotion environment.
• Permissions were not removed; they are scoped only to the promote job. This should be more friendly in security terms.
• released_major must be exactly the next version after the newest releaseNNN.
• The first run is explicitly documented as a bootstrap.
• Every deleted branch has its own force-with-lease. This makes sure we don't delete something by mistake.
• A concurrent branch modification or deletion rejects the entire atomic push.
• The redundant dry_run validation was removed. I hope I understood this correctly. This was a reviewer bot comm.
• The retention validation remains because it requires a positive integer.
• Checkout no longer persists credentials.
• Git authentication is configured explicitly only for live runs.
• Job purposes and permissions are now documented.

I have some limitations here. I don't have enough rights in Github.
I understand we need to :

Open Settings → Environments.
Create firefox-branch-promotion.
Add at least one required reviewer. (maybe avoid self review?)
Maybe to restrict this to main?

[ben-c-at-moz]

We're almost ready on this one. I think just addressing what exists, and linking a successful dry_run to the description, and we should be ready to go.

[vsangereanMOZ]

We're almost ready on this one. I think just addressing what exists, and linking a successful dry_run to the description, and we should be ready to go.

I just made a fork to do this.
I completed the dry-run validation in my personal fork using:

• Branch: vs/firefox-train-versioning
• released_major: 156
• retention: 2
• dry_run: true (named: Show the planned changes without updating branches)

The workflow completed successfully. The live-approval job was skipped, and no branches were modified.

https://github.com/vsangereanMOZ/fx-desktop-qa-automation/actions/runs/33764531601

Is this ok @ben-c-at-moz ?

[ben-c-at-moz]

We're almost ready on this one. I think just addressing what exists, and linking a successful dry_run to the description, and we should be ready to go.

I just made a fork to do this. I completed the dry-run validation in my personal fork using:

* Branch: `vs/firefox-train-versioning`

* `released_major`: `156`

* `retention`: `2`

* `dry_run`: `true`    (named: Show the planned changes without updating branches)

The workflow completed successfully. The live-approval job was skipped, and no branches were modified.

https://github.com/vsangereanMOZ/fx-desktop-qa-automation/actions/runs/33764531601

Is this ok @ben-c-at-moz ?

Oh if you've forked it, why not try it live and see if it changes the branches?

[vsangereanMOZ]

It was difficult to find out how to run the changes on the fork and branch. Must have run on main:D

https://github.com/vsangereanMOZ/fx-desktop-qa-automation/actions/runs/33869114350

[vsangereanMOZ]

[ben-c-at-moz]

Alright, I think we're at the point where we can try it