AINFRA-2806: Make localization downloads reviewable

[iangmaia]

📘 Part of: AINFRA-2806

Fixes AINFRA-2806
Releases V2 PR: https://github.a8c.com/Automattic/wpcom/pull/232628

Splits app translation and iOS/tvOS App Store metadata downloads out of beta preparation and release finalization.

A dedicated Fastlane lane and Buildkite pipeline now download the localized content into a phase-specific branch, lint it, and create one reviewable PR against the active release branch when changes exist.

Release finalization still checks translation progress, lints the reviewed files, and uploads iOS/tvOS metadata after the separate localization PR has been merged.

Testing

• Triggered from ReleasesV2 the build https://buildkite.com/automattic/pocket-casts-ios/builds/17222 (test release with branch release/0.0)
  • This other run generated no PR (based on this branch, which has latest strings): https://buildkite.com/automattic/pocket-casts-ios/builds/17210
• Generated PR (I added some changes first to force a diff): Update translations and App Store metadata for 0.0 #4919
• The entire end to end flow will be tested in the next release cycle.

Checklist

• I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
• I have considered adding unit tests for my changes.
• I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.

[claude]

Claude finished @iangmaia's task in 2m 50s —— View job

---

PR Review: Make localization downloads reviewable

I reviewed the new download_translations_and_metadata lane, the changes to new_beta_release/finalize_release, and the new Buildkite pipeline against the surrounding Fastfile conventions.

Overall: Clean, well-scoped change that follows existing patterns closely. The pipeline YAML mirrors new-beta-release.yml/finalize-release.yml exactly, input validation is solid, and the no-op cleanup path is handled properly. A few things worth considering below.

---

Suggestions

1. PR-creation failure is silent (medium) — fastlane/Fastfile:457-467

pr_url = find_or_create_pull_request(...)
buildkite_annotate(..., message: "Translations and metadata PR: #{pr_url || 'Error creating PR'}") if is_ci

If find_or_create_pull_request returns nil, the lane still exits 0, so the Buildkite job goes green while only an annotation reports the error. Given this is triggered from ReleasesV2 and manual retry is disabled, a failed PR creation could go unnoticed until someone checks for the PR. Consider failing loudly:

UI.user_error!('Failed to create the translations and metadata PR') if pr_url.nil?

Fix this →

2. Re-run behavior when the phase branch already exists on remote (low) — fastlane/Fastfile:444-456
On a re-trigger for the same version+phase, create_branch recreates translations/release-<version>-<phase> locally from base_branch, and push_to_git_remote(set_upstream: true) pushes without force. If a remote branch from a prior run exists with divergent history, this push could be rejected (non-fast-forward). Risk is low because retries are discouraged/blocked, but it may be worth confirming the intended recovery path (delete stale remote branch, or force-push).

3. finalize_release now implicitly depends on the localization PR being merged (by design, worth guarding) — fastlane/Fastfile:499-501
With the download removed, finalize_release uploads whatever metadata is already on the release branch. If the separate localization PR wasn't merged first, it will silently upload stale metadata. This matches the PR's intent, but there's no guard verifying the reviewed content is present. A lightweight check (or a clear runbook note) would prevent a "forgot to merge the localization PR" mistake from shipping stale App Store metadata.

---

Minor / nits

• Phase regex \A[a-z0-9-]+\z (Fastfile:441) correctly prevents branch-name injection 👍, but it also permits leading/trailing/consecutive hyphens (e.g. --), producing odd branch names. Cosmetic only.
• The created PR body doesn't mention the phase, so multiple localization PRs for the same release read near-identically in the body (the title disambiguates them). Minor.

Things that look good

• require_env_vars!('GITHUB_TOKEN'), ensure_git_status_clean, and ensure_git_branch(branch: '^release/') guards up front.
• No-op path (point_to_same_commit?) cleanly checks out the base branch and deletes the local phase branch — no dangling state.
• Relies on the download sub-lanes' allow_nothing_to_commit: true commits, so point_to_same_commit? is a reliable "nothing changed" signal.

Note: I couldn't inspect the pinned fastlane-plugin-wpmreleasetoolkit (14.11.2) gem in this environment. Worth a quick confirmation that find_or_create_pull_request and the GitHelper calls (create_branch(from:), point_to_same_commit?, delete_local_branch_if_exists!) match that version's API — bundle exec fastlane lanes won't catch an unknown action name since it isn't resolved until runtime.

[Copilot]

Pull request overview

This PR splits GlotPress translation and App Store metadata downloads out of the existing beta/release lanes and into a dedicated Fastlane lane plus Buildkite pipeline, so that localization updates land via a separate, reviewable PR against the active release/* branch.

Changes:

• Removes automatic GlotPress download steps from new_beta_release and finalize_release, leaving these lanes to lint reviewed localization content (and upload reviewed metadata during finalization).
• Adds a new download_translations_and_metadata lane that creates a dedicated translations/release-... branch, downloads updates, lints them, and opens/updates a PR.
• Introduces a new Buildkite pipeline to run the new lane as part of ReleasesV2 scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
fastlane/Fastfile	Adds a dedicated lane to download/lint translations & metadata and open a PR; removes download from beta/finalize lanes.
.buildkite/release-pipelines/download-translations-and-metadata.yml	New Buildkite pipeline step to invoke the new Fastlane lane during ReleasesV2.

[claude]

PR Review: Make localization downloads reviewable

Reviewing the latest push (branch lifecycle changes + CI toolkit checkout).

• Gather context (diff, latest commits, prior review threads)
• Review download_translations_and_metadata lane
• Review Buildkite pipeline
• Post review

View job run

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[claude]

Claude finished @iangmaia's task in 4m 13s —— View job

---

PR Review: Make localization downloads reviewable

• Gather context (diff, latest commits, prior review threads)
• Review download_translations_and_metadata lane
• Review Buildkite pipeline
• Post review

---

Re-reviewed after 7fea737, 3930421, 900230f.

What got fixed since my last pass: the branch lifecycle is now correct. Replacing delete_remote_branch_if_exists! + non-forced push with a plain force-push resolves mokagio's finding — the ls-remote vs. refs/remotes/* source-of-truth mismatch no longer matters because the push doesn't depend on the local tracking ref existing, and the PR identity (number, discussion, annotation URL) survives reruns. --force rather than --force-with-lease is the right call here, since the freshly-created local branch has no lease to check against. Recreating from base_branch each run also means the branch auto-rebases onto a release branch that moved forward — good property.

---

Findings

1. Merge-order dependency on checkout_release_branch (medium) — .buildkite/release-pipelines/download-translations-and-metadata.yml:15

This pipeline is now the only one calling the CI toolkit function; new-beta-release.yml:15, finalize-release.yml:15, and the rest still call .buildkite/commands/checkout-release-branch.sh, which is still present on this branch. CI_TOOLKIT_PLUGIN_VERSION="6.1.1" (.buildkite/shared-pipeline-vars:17) is unchanged by this PR, so worth confirming 6.1.1 actually ships checkout_release_branch — if it landed after 6.1.1, this job dies at command not found and, with manual retry disabled, the only recovery is re-triggering from ReleasesV2.

Also note the local script handles a case the toolkit version needs to cover too: git reset --hard FETCH_HEAD to fix up a reused Buildkite working copy where a previous job left the release branch at a different commit. That matters here specifically, because create_branch(from: base_branch) reads the local release/<version> ref — a stale local branch would silently seed the localization PR from the wrong base.

2. Failed PR creation still exits 0 (medium) — fastlane/Fastfile:462-472

Unchanged from my first review. If find_or_create_pull_request returns nil, the branch is pushed, the annotation reads Translations and metadata PR: Error creating PR, and the Buildkite job goes green. Given this runs unattended from ReleasesV2, the failure surfaces only when someone notices the PR is missing.

UI.user_error!("Failed to create the translations and metadata PR for `#{pr_branch}`") if pr_url.nil?

That also lets the || 'Error creating PR' fallback in the annotation go away.

Fix this →

3. git push --delete can hard-fail an otherwise successful no-op run (low-medium) — fastlane/Fastfile:450-459

sh raises on non-zero exit, so if the remote branch disappears between the branch_exists_on_remote? check and the delete — most likely when the localization PR was just merged and GitHub's auto-delete-on-merge removed the branch — the lane aborts. That's a red job for the nothing to do case, plus the cleanup below it (checkout_and_pull, local branch delete) never runs, leaving the agent's working copy on the phantom branch for the next job. Same outcome if branch protection blocks deletion.

Since deletion is best-effort cosmetics (the PR is empty either way), tolerating failure seems right:

sh('git', 'push', 'origin', '--delete', pr_branch) do |status, _, cmd|
  UI.important("`#{cmd}` failed (#{status.exitstatus}); the branch may already be gone.") unless status.success?
end

Fix this →

4. Every rerun dismisses review state, even when nothing changed (low-medium) — fastlane/Fastfile:444-461

The branch is recreated and re-committed on each run, so git_commit mints new SHAs from new timestamps — the tree can be byte-identical to what's already on the remote branch and the force-push still rewrites history. Consequence: reviewers see a force-push, and if the repo has dismiss stale approvals enabled, an approval on the localization PR is lost every time the lane re-runs for the same release. For a PR whose entire purpose is to be reviewed, that's a real papercut.

Cheap guard — skip the push when the content matches what's already up there:

if remote_branch_exists
  sh('git', 'fetch', 'origin', pr_branch)
  unchanged = sh('git', 'diff', '--quiet', 'FETCH_HEAD', 'HEAD') { |status, _, _| status.success? }
end

and short-circuit to the existing-PR annotation when unchanged. Worth weighing against the added complexity, but it's the difference between "PR gets refreshed" and "PR gets refreshed and re-approved".

5. version isn't tied to the checked-out branch (low) — fastlane/Fastfile:435-437

ensure_git_branch(branch: '^release/') accepts any release branch, then base_branch is built from version independently. In CI both come from RELEASE_VERSION so they agree, but a local invocation with a mismatched version: silently branches off whatever stale local release/<version> ref happens to exist. Anchoring the guard makes the invariant explicit and costs one line:

ensure_git_branch(branch: "^#{Regexp.escape(base_branch)}$")

This also covers an explicit version:'' (the keyword default only fires on omission, so '' would yield base_branch = 'release/').

Fix this →

Nits

• fastlane/Fastfile:429 — @param [String] version inverts the file's convention (@param base_version [String] at line 378, @param interactive [Boolean] at 1180).
• fastlane/Fastfile:467 — the PR body could warn that the branch is regenerated and force-pushed on every run, so a reviewer who commits a translation fix directly to it will lose that commit. Pointing them at the release branch instead makes the automation's ownership of the branch legible.

Verified good

• .gitignore:29 covers fastlane/app-localization-downloads/*.lproj, so the download scratch dir won't trip the next run's ensure_git_status_clean on a reused agent working copy.
• labels: ['Releases'] matches existing usage at fastlane/Fastfile:1681, so the label exists.
• The download sub-lanes all commit via git_commit(..., allow_nothing_to_commit: true) (fastlane/Fastfile:1149-1153 and download_localized_app_store_metadata), which is what makes point_to_same_commit? a trustworthy "nothing changed" signal.
• No-op path ordering is correct: checkout_and_pull(base_branch) before delete_local_branch_if_exists!(pr_branch), so it never tries to delete the current branch.
• get_required_env! at line 469 matches the file's convention (lines 885, 913, 945, 993).

Caveat: fastlane-plugin-wpmreleasetoolkit (14.11.2) isn't installed in this environment, so I couldn't confirm the GitHelper signatures (create_branch(name, from:), delete_local_branch_if_exists!, checkout_and_pull(String), point_to_same_commit?) or find_or_create_pull_request's nil-vs-raise behavior against that version. Your test run on build 17222 exercised the happy path, which covers most of this.
• Branch

[claude]

Claude finished @iangmaia's task in 5m 13s —— View job

---

PR Review: Make localization downloads reviewable

• Gather context (diff, commits since last review)
• Review download_translations_and_metadata lane
• Review Buildkite pipeline + toolkit checkout
• Post review

---

Re-reviewed after fd5c500, 40d4daf, 90cb576, 4571676.

What's resolved since my last pass: the toolkit checkout is now consistent with its siblings (CI_TOOLKIT_PLUGIN_VERSION="6.3.0", and new-beta-release.yml:15/finalize-release.yml:15 both call checkout_release_branch too, with .buildkite/commands/checkout-release-branch.sh gone — so #4916 landed and this matches). UI.user_error! on a nil pr_url (Fastfile:488) closes the silent-failure hole. The anchored ensure_git_branch(branch: "^#{Regexp.escape(base_branch)}$") (:438) ties version to the checked-out branch, which also incidentally makes the branch names injection-proof without needing the old phase regex. And the tree comparison at :461-467 is a neat answer to the approval-dismissal problem — an identical rerun now skips the force-push entirely, so reviewers don't see churn. The PR body warning about regeneration is a good touch.

---

Findings

1. Remote state is sampled before the download and trusted after it (low-medium) — fastlane/Fastfile:440, :452, :463

remote_branch_exists is computed by ls-remote before download_localized_strings_and_metadata_from_glotpress, but it's only ever consumed after it — and the GlotPress download plus lint is a multi-minute window. Both consumers then run commands that raise via sh if the sampled state has drifted:

• :463 sh('git', 'fetch', 'origin', pr_branch) — fails if the branch disappeared mid-run.
• :452 sh('git', 'push', 'origin', '--delete', pr_branch) — same, plus fails if branch protection forbids deletion.

The realistic trigger is benign and human: a reviewer merges the localization PR while this job is downloading, and GitHub's auto-delete-on-merge removes the head branch. base_branch is the local ref from before the merge, so the downloads still look like changes → the else path runs → git fetch origin translations/release-X dies → red job for a run that should have simply re-pushed. In the no-op path the failure is worse than cosmetic: sh raises before checkout_and_pull(base_branch) and the local branch delete at :458-459, so the agent's working copy is left sitting on the phantom translations/... branch.

Cheapest fix is to move the sample to where it's used — ls-remote is a sub-second call, and the early UI.important at :441 can stay as a heads-up:

# after lint_localizations
remote_branch_exists = Fastlane::Helper::GitHelper.branch_exists_on_remote?(branch_name: pr_branch)

and make the delete best-effort, since it's cosmetic either way (the PR is empty regardless):

sh('git', 'push', 'origin', '--delete', pr_branch) do |status, _, cmd|
  UI.important("`#{cmd}` failed (#{status.exitstatus}); the branch may already be gone.") unless status.success?
end

Fix this →

2. finalize_release silently uploads whatever metadata is on the branch (medium, by design — worth a guard) — fastlane/Fastfile:523-525

With the download gone, lint_localizations + update_metadata_on_app_store_connect{,_tvos} operate on whatever happens to be committed. If the localization PR wasn't merged, finalization succeeds and pushes stale metadata to App Store Connect — no warning, and check_all_translations_progress won't catch it because it queries GlotPress, not the branch. The failure mode is a human forgetting a merge step that used to not exist, which is exactly the kind of thing worth a machine check. Even a soft one helps:

if Fastlane::Helper::GitHelper.branch_exists_on_remote?(branch_name: "translations/release-#{release_version_current}")
  UI.user_error!("`translations/release-#{release_version_current}` still exists on the remote — merge (or close) the localization PR before finalizing.")
end

Caveat: that false-positives if the repo doesn't auto-delete merged head branches, in which case querying for an open PR with that head is the accurate signal. If you'd rather keep the lane dumb, a line in the release runbook covers it — but right now nothing does.

3. Tree comparison via two rev-parse calls can silently fall through (nit) — fastlane/Fastfile:464-465

Fastlane's sh collects stdout and stderr together (Open3.popen2e), so any git warning on stderr gets concatenated into remote_tree/downloaded_tree and the equality check quietly becomes false → force-push. It degrades in the safe direction, so not a bug, but a single diff expresses the intent more directly and costs one call instead of two:

remote_branch_matches_downloads = sh('git', 'diff', '--quiet', 'FETCH_HEAD', 'HEAD') { |status, _, _| status.success? }

4. No-op message reads oddly when a PR was just closed (nit) — fastlane/Fastfile:455

'No translations or App Store metadata updates were downloaded, so no PR was created' also fires on the path that just deleted the remote branch and thereby closed an existing PR. The Buildkite annotation is the only artifact a release manager sees, and "no PR was created" undersells "the previous PR was closed." Worth folding the outcome into the message when remote_branch_exists.

Verified good

• .gitignore:29 covers fastlane/app-localization-downloads/*.lproj, so the scratch dir won't trip the next run's ensure_git_status_clean on a reused agent working copy.
• sh('git', 'rev-parse', 'HEAD^{tree}') uses the array form, so ^{tree} never reaches a shell — no quoting hazard.
• labels: ['Releases'] and repository: GITHUB_REPO (Fastfile:63) match existing usage.
• All three download sub-lanes commit through git_commit(..., allow_nothing_to_commit: true) / download_localized_app_store_metadata (Fastfile:1108-1192), which is what makes point_to_same_commit? a trustworthy "nothing changed" signal.
• No-op cleanup ordering is right: checkout_and_pull(base_branch) before delete_local_branch_if_exists!(pr_branch), so it never deletes the current branch.
• version: release_version_current as a keyword default matches the file's convention (:236, :307, :382) and evaluates lazily, so the version file is only read when version: is omitted.
• Pipeline YAML is a faithful copy of finalize-release.yml — same env, plugins, agents, and no-manual-retry block.

Caveat: fastlane-plugin-wpmreleasetoolkit (14.11.2) isn't installed here and I have no network access in this run, so find_or_create_pull_request's nil-vs-raise behavior and the GitHelper signatures are still unverified against that version. Build 17222 covers the happy path; the rerun-with-existing-branch path added in 4571676 hasn't been exercised on CI yet, and it's the one with the fetch/delete raises in finding #1.
• Branch

[claude]

Claude finished @iangmaia's task in 3m 33s —— View job

---

PR Review: Make localization downloads reviewable

• Gather context (diff, commits since last review)
• Review download_translations_and_metadata lane
• Review the new finalization guard
• Post review

---

Re-reviewed after ca61eb6 (simplify refreshes) and aa87f31 (finalization guard).

Resolved since my last pass: remote_branch_exists is now sampled after the download/lint (Fastfile:446), so the multi-minute window where the sampled remote state could go stale is gone — it's now a sub-second gap before the two consumers. The no-op annotation distinguishes "existing PR was closed" from "no PR was created" (:455-459). And ensure_no_open_localization_pr! gives finalization the machine check I asked for, placed before the UI.confirm so it fails fast. The pipeline is now a faithful sibling of finalize-release.yml — same checkout_release_branch "${RELEASE_VERSION}", CI_TOOLKIT_PLUGIN_VERSION="6.3.0", .buildkite/commands/checkout-release-branch.sh gone.

---

Findings

1. The guard puts an unverified toolkit API on the critical release path (medium) — fastlane/Fastfile:1865-1877

Fastlane::Helper::GithubHelper#find_pull_request appears nowhere else in this Fastfile, and it's now the first thing finalize_release does after the git guards. Three ways it can go wrong, each with a sharp consequence:

• Method doesn't exist in 14.11.2 → NoMethodError aborts every finalize_release, for every release, before anything else runs.
• state isn't open-only by default → once the localization PR is merged, the guard keeps finding it and finalization is permanently blocked, with no bypass (skip_confirm doesn't cover it). The release manager's only escape would be editing the Fastfile.
• head: needs owner:branch qualification. GitHub's list-PRs endpoint filters head as user:ref-name; a bare translations/release-X matches nothing. If the helper doesn't qualify it, the guard silently never fires (a false pass — the exact failure it exists to catch), and by symmetry find_or_create_pull_request at :466 would fail to find the existing PR and open a duplicate on every rerun, undoing the PR-identity fix from 90cb576.

That last one is worth singling out because the find path has never been exercised on CI: build 17222 created PR #4919 fresh. A rerun against an existing open PR would validate both find_or_create_pull_request's find branch and, indirectly, the head-matching semantics the guard depends on.

2. The guard catches an open PR, not a skipped download (medium) — fastlane/Fastfile:509-510

If the download job never ran for this release — pipeline not wired into the scenario, job failed with retry disabled, someone triggered finalization early — there's no PR to find, the guard passes, and update_metadata_on_app_store_connect{,_tvos} uploads whatever metadata happens to be committed on the branch. check_all_translations_progress won't catch it either: it queries GlotPress percentages, not the branch contents. So the "forgot the localization step" mistake is only half-closed: forgetting to merge now fails loudly, forgetting to run is still silent. If the ReleasesV2 scenario guarantees the download step always precedes finalization, that's fine — worth stating in the lane comment so the assumption is legible. Otherwise a positive signal (a merged localization PR for this version, or a metadata commit newer than the code freeze) would close it.

3. Every rerun force-pushes, even when the downloaded content is identical (low-medium) — fastlane/Fastfile:441, :465

ca61eb6 removed the tree comparison, so the else path always force-pushes. Because the branch is recreated and re-committed each run, git_commit mints commits with fresh committer timestamps — different SHAs even when the tree is byte-identical to what's already on the remote. So a rerun that downloads nothing new still rewrites the branch. Two consequences: reviewers see a force-push on a PR they've already read, and if the branch protection covering PRs into release/* has dismiss stale approvals enabled, the approval is dropped every time. For a PR whose entire purpose is being reviewed, that's the one form of churn worth avoiding.

I flagged this before and the simplification was deliberate, so this is a tradeoff note rather than a request — reruns should be rare, and the previous version cost ~10 lines and two sh calls. Just want the cost stated explicitly: it's "PR gets refreshed" vs. "PR gets refreshed and needs re-approval." If dismissal is off in this repo, the whole concern evaporates.

4. git push --delete can still red-job an otherwise successful no-op run (low) — fastlane/Fastfile:452

sh raises on non-zero exit. The ordering fix in aa87f31 shrank the race window a lot, but the delete can still fail: branch protection covering translations/*, a concurrent job, or a merge landing in that sub-second gap. When it does, the lane aborts before the cleanup at :462-463, leaving the agent's working copy checked out on the now-phantom branch for the next job that reuses it. Deletion is cosmetic here — the PR is empty either way — so tolerating failure costs nothing:

sh('git', 'push', 'origin', '--delete', pr_branch) do |status, _, cmd|
  UI.important("`#{cmd}` failed (#{status.exitstatus}); the branch may already be gone.") unless status.success?
end

Fix this →

5. new_beta_release has the same implicit dependency, unguarded (low) — fastlane/Fastfile:411

Beta prep now lints whatever strings are on the branch with no download in front of it, so an unmerged localization PR means the beta ships without the latest translations. Less severe than pushing stale App Store metadata (no external artifact to correct), but it's the same class of mistake, and the beta is where translators would normally catch problems. Worth deciding explicitly whether beta prep should warn (not necessarily fail).

Nits

• fastlane/Fastfile:509 — release_version is introduced but release_version_current is still called at :530, :531, :533. Half-migrated; either thread the local through or leave it out.
• fastlane/Fastfile:437 and :1868 — "translations/release-#{version}" is now built independently in the lane and in the guard. If one ever changes, the guard silently stops matching (a false pass, per finding Podcast Settings: Play Speed changes when switching podcast's settings #1's third bullet). There's precedent for extracting it: release_branch_name at :1393.
• fastlane/Fastfile:1867-1869 — pinning base: means a retargeted or manually-rebased localization PR isn't seen. Matching on head alone would be the stricter guard.
• fastlane/Fastfile:479 — the message says "find or create," which is accurate, but a reader debugging a nil there benefits from knowing which half failed. Minor.

Verified good

• Pipeline YAML is now structurally identical to finalize-release.yml (same env, plugins: [$CI_TOOLKIT], agents.queue, no-manual-retry block), and checkout_release_branch ships in the pinned 6.3.0 (.buildkite/shared-pipeline-vars:17) — mokagio's finding is closed, and .buildkite/commands/checkout-release-branch.sh is gone from the branch, so there's no stale sibling left behind.
• ensure_git_branch(branch: "^#{Regexp.escape(base_branch)}$") (:438) ties version to the checked-out branch, makes the branch names injection-proof without the old phase regex, and catches an explicit version:''.
• Both buildkite_annotate calls share context: 'translations-and-metadata-pr-url', so a rerun replaces the previous annotation instead of stacking.
• require_env_vars!('GITHUB_TOKEN', *ASC_API_KEY_ENV_VARS) at :503 runs before the guard, so get_required_env!('GITHUB_TOKEN') inside the helper can't be the thing that fails.
• No-op cleanup ordering is right: checkout_and_pull(base_branch) before delete_local_branch_if_exists!(pr_branch), so it never deletes the current branch.
• All three download sub-lanes commit through git_commit(..., allow_nothing_to_commit: true) (:1163-1167, :1170-1186), which is what makes point_to_same_commit? a trustworthy "nothing changed" signal.
• The PR body's warning that direct commits may be overwritten is the right call now that the force-push is unconditional.
• version: release_version_current as a lazily-evaluated keyword default matches the file's convention (:236, :307, :382).

Caveat: fastlane-plugin-wpmreleasetoolkit (14.11.2) isn't installed in this environment and I have no network access, so GithubHelper#find_pull_request (existence, state default, head qualification, .html_url on the return value) and the GitHelper signatures are unverified against that version. Finding #1 is entirely about that gap — everything else stands on the diff alone.
• Branch

[mokagio]

I think the Claude review comments are a bit over the board, like the one about a possible error in Releases V2. Or maybe not over the board, but something that we might want to address later in a hardening pass of the whole flow instead?

Some of the nits, however, might be worth applying. Example:

fastlane/Fastfile:509 — release_version is introduced but release_version_current is still called at :530, :531, :533. Half-migrated; either thread the local through or leave it out.

[claude]

Claude finished @iangmaia's task in 2m 39s —— View job

---

PR Review: Make localization downloads reviewable

• Gather context (diff, commits since last review)
• Verify toolkit APIs against 14.11.3
• Review the lane + finalization guard
• Post review

---

Re-reviewed after 83a3283 (toolkit → 14.11.3). Keeping this short — mokagio approved and rightly called some of my earlier findings hardening-pass material, so I've dropped those rather than repeating them.

What 83a3283 changes: the raw sh('git', 'push', 'origin', '--delete', pr_branch) in the no-op path becomes Fastlane::Helper::GitHelper.delete_remote_branch_if_exists!(pr_branch) (fastlane/Fastfile:452), alongside the lockfile bump. That's a good swap on two counts: it matches the existing call at :574, and the helper's internal existence check makes the delete best-effort, so the branch vanishing between :446 and :452 (auto-delete-on-merge, a concurrent job) no longer raises and aborts the lane before the cleanup at :462-463. My earlier "red job on a no-op run" finding is closed by this.

---

One thing to confirm before merge

The no-op path's correctness now rests on 14.11.3 having fixed delete_remote_branch_if_exists! — fastlane/Fastfile:446, :452

This is mokagio's earlier finding in mirror image. In 14.11.2 the two predicates disagreed on source of truth: branch_exists_on_remote? shells out to git ls-remote, while delete_remote_branch_if_exists! consulted only local refs/remotes/*. This lane hits exactly the case where they diverge — checkout_release_branch "${RELEASE_VERSION}" fetches the release branch, so a CI agent has no refs/remotes/origin/translations/release-<version> tracking ref even when the branch is live on GitHub.

If 14.11.3 didn't change that helper, the failure is quiet rather than loud, which is worse than the raise it replaced:

1. :446 ls-remote → true, so remote_branch_exists is set.
2. :452 finds no local tracking ref → returns without deleting.
3. :455-461 annotates "the existing PR was closed" — it wasn't.
4. The stale localization PR stays open, and ensure_no_open_localization_pr! (:1866) then hard-blocks finalize_release with no bypass, on a run whose annotation said everything was clean.

The version bump landing in the same commit reads like 14.11.3 was cut precisely for this, and if so everything above is moot — but I can't confirm it: the gem isn't installed in this environment and I have no network access, so delete_remote_branch_if_exists!'s 14.11.3 implementation is unverified. Worth a glance at the toolkit's 14.11.3 changelog. If the fix isn't there, a git fetch origin #{pr_branch} before line 452 makes the local ref exist and sidesteps the whole question.

Related: Gemfile:10 still says ~> 14.11.0. If the lane depends on behavior introduced in .3, tightening to ~> 14.11.3 documents that; the lockfile pins it either way, so this is about intent, not runtime.

Nits

• fastlane/Fastfile:509, :530, :531, :533 — the release_version local mokagio flagged is still half-migrated. Behaviourally identical (commit_version_bump at :524 only moves the build code, so release_version_current still returns the same string at :530), so purely cosmetic. Fix this →
• fastlane/Fastfile:1873-1875 — ensure_no_open_localization_pr! raises without a buildkite_annotate(style: 'error', ...), unlike its sibling ensure_branch_does_not_exist! at :1860. Since this guard fires unattended from ReleasesV2, an error annotation is what tells the release manager why finalization stopped, without opening the raw log. Fix this →
• fastlane/Fastfile:437 and :1868 — "translations/release-#{version}" is still built independently in the lane and in the guard. Divergence would make the guard silently stop matching (a false pass). release_branch_name at :1393 is the precedent for extracting it.

Still open by choice

Noting these only so the tradeoffs are on the record — I'm not asking for changes:

• Every rerun force-pushes even when the tree is unchanged (:465), since recreate + re-commit mints fresh SHAs. Costs an approval each rerun if the branch protection on release/* PRs dismisses stale approvals; harmless otherwise.
• The finalization guard catches a forgotten merge, not a skipped download — if the download job never ran, there's no PR to find and stale metadata uploads silently. Fine if the ReleasesV2 scenario guarantees ordering.
• new_beta_release has the same implicit dependency with no guard (:411).

Verified good

• Pipeline is a faithful sibling of finalize-release.yml: same env, plugins: [$CI_TOOLKIT], agents.queue, no-manual-retry block, and checkout_release_branch "${RELEASE_VERSION}" against the pinned CI_TOOLKIT_PLUGIN_VERSION="6.3.0". .buildkite/commands/checkout-release-branch.sh is gone from the branch, so no stale sibling remains — Use checkout_release_branch from a8c-ci-toolkit #4916 is fully absorbed.
• delete_remote_branch_if_exists!(pr_branch) matches the positional-arg call at :574, so the signature is exercised by existing working code.
• ensure_git_branch(branch: "^#{Regexp.escape(base_branch)}$") (:438) ties version to the checked-out branch and makes the derived branch names injection-proof.
• Guard placement in finalize_release is right: after require_env_vars! (:501, so get_required_env!('GITHUB_TOKEN') inside the helper can't be what fails) and before UI.confirm (:513), so it fails fast.
• Both buildkite_annotate calls share context: 'translations-and-metadata-pr-url', so reruns replace rather than stack.
• No-op cleanup ordering is correct — checkout_and_pull(base_branch) before delete_local_branch_if_exists!(pr_branch).
• All download sub-lanes commit via git_commit(..., allow_nothing_to_commit: true), which is what makes point_to_same_commit? a trustworthy "nothing changed" signal.

Caveat: fastlane-plugin-wpmreleasetoolkit isn't installed here and this run has no network, so delete_remote_branch_if_exists!'s 14.11.3 behavior, GithubHelper#find_pull_request (state default, head qualification), and find_or_create_pull_request's nil-vs-raise contract remain unverified against the pinned version. Build 17222 covers the create path; the no-op-with-existing-remote-branch path — the one the confirmation above is about — hasn't run on CI.
• Branch

[claude]

Claude finished @iangmaia's task in 3m 4s —— View job

---

PR Review: Make localization downloads reviewable

• Gather context (diff since last review)
• Review the lane + finalization guard
• Post review

---

Re-reviewed after 64e51f2. Small commit, and it closes both of mokagio's nits — but the branch-name extraction changed the branch name, which is worth a look before merge.

---

One thing to confirm before merge

The localization branch silently renamed from translations/release-<version> to translations/release/<version> — fastlane/Fastfile:1398-1400

def translations_and_metadata_branch_name(version)
  "translations/#{release_branch_name(version)}"
end

Composing on release_branch_name interpolates release/7.94, so the result is translations/release/7.94 — a nested path, not the hyphenated translations/release-7.94 this PR has used until now. My earlier nit asked only that the string stop being built independently in the lane (:437) and the guard (:1875); it didn't ask for a rename. So: intentional, or a side effect of the extraction?

Mechanically the new name is fine — I checked the ref-layout worry and there's no directory/file conflict, because a hotfix version nests no deeper than its base (translations/release/7.94 and translations/release/7.94.1 are siblings in one directory, exactly like the existing release/7.94 + release/7.94.1 pair). git branch --list 'translations/*' and GitHub's head filter both handle the slash.

The part that does matter is merge timing. Any translations/release-<version> branch with an open PR at the moment this lands becomes invisible to the new code, in both directions:

• find_or_create_pull_request (:466) searches head: translations/release/<version>, finds nothing, and opens a second localization PR for the same release from the new branch — the old one lingers open.
• ensure_no_open_localization_pr! (:1871) searches the same new name, finds nothing, and passes — so finalize_release proceeds and uploads whatever metadata is on the release branch, without the reviewed changes. That's precisely the failure the guard was added to catch, and it fails silently rather than loudly.

Your own test PR #4919 is on the old naming (against release/0.0). Closing it plus deleting the old branch before merge, and confirming no live release has a translations/release-* PR open, covers it. If the rename wasn't the goal, keeping the literal inside the helper preserves the dedup without the transition:

def translations_and_metadata_branch_name(version)
  "translations/release-#{version}"
end

Fix this →

Secondary effect either way: the create path was exercised on build 17222 under the old name, so the name the code now produces hasn't run. Low risk — it's a string — but it's no longer the tested value.

Nits

• fastlane/Fastfile:317, :599 — now that release_branch_name(version) exists, these two still build "release/#{...}" by hand. They can't just call the helper as-is, since the local variable of the same name shadows the method inside those lanes; renaming the local (e.g. new_release_branch = release_branch_name(new_version)) would finish the dedup the helper was extracted for. Fix this →
• fastlane/Fastfile:1392, :1398 — asymmetric defaults: release_branch_name defaults to the current version, translations_and_metadata_branch_name requires one. Defensible (both callers always have a version), just reads slightly uneven next to its neighbour.

Verified good in this commit

• release_version migration is complete and behaviour-preserving. release_version = release_version_current (:509) is captured before commit_version_bump (:524), and that bump only writes the build code (VERSION_FILE.write(version_long: build_code_next)), so the milestone calls at :530-531 and the rescue message at :533 get the same string they did when they called release_version_current directly. mokagio's nit closed with no change in output.
• The guard's error annotation matches its sibling exactly — buildkite_annotate(style: 'error', ..., message: error_message) if is_ci before UI.user_error!, same shape as ensure_branch_does_not_exist! (:1860). The context: 'error-checking-localization-pr' is distinct from 'error-checking-branch', so the two guards don't overwrite each other's annotation. A release manager blocked by this now sees why without opening the log.
• Gemfile:10 → ~> 14.11.3 matches Gemfile.lock:173's resolved 14.11.3 and Gemfile.lock:388, so the dependency now documents that the no-op path depends on .3 behaviour rather than relying on the lockfile alone. ~> 14.11.3 still admits 14.11.x patch updates.
• Both branch-name call sites (:436-437, :1874-1875) now go through the helpers, so the lane and the guard can't drift apart — which was the actual point of the nit.
• The lane body is otherwise unchanged from the version mokagio approved: anchored ensure_git_branch (:438), post-download remote sampling (:446), best-effort delete_remote_branch_if_exists! (:452), UI.user_error! on a nil pr_url (:479), shared annotation context so reruns replace rather than stack.
• Pipeline YAML untouched and still a faithful sibling of finalize-release.yml; checkout_release_branch against the pinned CI_TOOLKIT_PLUGIN_VERSION="6.3.0".

Still open by choice

Recording only, not asking for changes — mokagio's call that these belong in a hardening pass reads right to me:

• Every rerun force-pushes even when the tree is unchanged (:465), costing an approval per rerun if release/* PR protection dismisses stale approvals.
• The finalization guard catches a forgotten merge, not a skipped download — if the download job never ran there's no PR to find, and stale metadata uploads silently.
• new_beta_release (:411) has the same implicit dependency with no guard.

Caveat: this environment has no network and fastlane-plugin-wpmreleasetoolkit isn't installed, so I couldn't run ruby -c on the Fastfile or verify GithubHelper#find_pull_request's state default and head qualification, or delete_remote_branch_if_exists!'s 14.11.3 behaviour. Everything above is from the diff. The no-op-with-existing-remote-branch path still hasn't run on CI.
• Branch