Realign the release branch on its remote during checkout

[AliSoftware]

What does it do?

Part of AINFRA-2725, following the WooCommerce iOS 25.1 release incident. The same change is going out to every mobile product repo.

1. Realign the release branch on the remote (the actual fix)

.buildkite/commands/checkout-release-branch.sh fetched the release branch and checked it out, but never moved the local branch onto the fetched commit:

git fetch origin "$BRANCH_NAME"
git checkout "$BRANCH_NAME"

Buildkite cleans the working copy between jobs, but it can reuse it. A refs/heads/release/x.y left behind by an earlier job on the same agent therefore survives, and git checkout then simply switches to that stale local ref rather than to what was just fetched. Whatever runs next — the version bump, or the GitHub Release draft that finalize_release creates from HEAD — would then be based on the wrong commit.

The fix adds the missing realignment. reset --hard rather than git pull: no extra network round trip, and no merge commit if the refs diverged.

2. Reset to FETCH_HEAD rather than the remote-tracking ref

git fetch origin <branch> always writes FETCH_HEAD, but it only updates refs/remotes/origin/<branch> when the remote's fetch refspec covers that branch. With Buildkite's default +refs/heads/*:refs/remotes/origin/* the two are equivalent — but on a clone whose refspec was narrowed after origin/<branch> already existed, the fetch leaves that ref stale and reset --hard "origin/$BRANCH_NAME" lands on the old commit: the very failure this script exists to prevent, reintroduced through the back door. Resetting to FETCH_HEAD drops the refspec dependency entirely.

3. Standardize the script across all repos

Slightly tangent to the issue, but bundled deliberately: the script had drifted into five different shapes across the repos — argument required via ${1?…} or ${1:?…}, argument plus a BUILDKITE_BRANCH fallback, argument with a hand-rolled usage check, the same under a different variable name, and (in one repo) no argument at all, reading RELEASE_VERSION from the environment. Having rolled the same one-line fix out thirteen times this week, that divergence is pure friction.

All repos now share one canonical script. The resolution order is a superset of what every repo did before — argument, then RELEASE_VERSION from the environment, then the release/* branch the build runs on — so no call site needed changing. The BUILDKITE_BRANCH fallback only fires on a branch matching ^release/, and derives the branch name back from that same value, so it cannot select a branch other than the one the build was already triggered on.

It also closes two latent bugs: under bash -eu, both [[ -z "${RELEASE_VERSION}" ]] on an unset variable and a bare RELEASE_VERSION=$1 with no arguments abort with unbound variable before their intended usage message can print. And an argument that is passed but empty — what happens when a pipeline forwards an unset $RELEASE_VERSION — no longer resolves to a bare release/: it falls through to the environment variable, then to the release/* branch the build runs on, and finally to a clear error if none of those yields a version.

4. Bump release-toolkit to 14.11.2

Picks up wordpress-mobile/release-toolkit#763, the first half of AINFRA-2725: publish_github_release now publishes the most recently created GitHub Release when several share the same name, rather than whichever one the API happened to list first. Without it, a re-run of finalize_release can still leave the git tag on the wrong commit — the root cause of the 25.1 incident.

bundle update also refreshed a few unrelated transitive gems and bumped BUNDLED WITH to the current 4.0.17.

Testing instructions

No behaviour change on a fresh checkout, which is the normal case: the local branch is already at the fetched commit, so the reset is a no-op. The argument-resolution logic was exercised across all combinations (argument / empty argument / environment variable / release/* branch / trunk / feature branch / unset), and the resulting script passes shellcheck in every repo. The next release build exercising this script is the real check.

🤖 Generated with Claude Code

[claude]

Claude finished @AliSoftware's task in 2m 42s —— View job

---

Code review

• Read the PR diff against origin/main
• Read the full script and trace all callers
• Check for regressions in the removed git pull behaviour
• Post review

---

The fix is correct and addresses a real failure mode. git reset --hard "origin/$BRANCH_NAME" after the checkout is the right primitive here, and the rationale for preferring it over git pull (no extra round trip, cannot create a merge commit on divergence) holds. I also confirmed this is the only place in .buildkite/ doing git branch manipulation, so there's no sibling script left with the old pattern.

Two hardening points, both about the same "reused working copy" premise the PR is built on. Neither is a blocker.

1. The reset trusts a remote-tracking ref that git fetch origin <branch> only updates opportunistically

.buildkite/commands/checkout-release-branch.sh:24-28

git fetch origin "$BRANCH_NAME"
git checkout "$BRANCH_NAME"
git reset --hard "origin/$BRANCH_NAME"

A refspec-less git fetch origin <branch> writes FETCH_HEAD unconditionally, but updating refs/remotes/origin/<branch> is a side effect that only happens when the remote's configured fetch refspec covers that branch (git ≥ 1.8.4). With the default +refs/heads/*:refs/remotes/origin/* that Buildkite's checkout sets up, this is fine — which is why the PR works in practice today. But if a reused working copy ever has a narrowed refspec (single-branch clone, mirror/reference clone, a hand-tweaked remote.origin.fetch), origin/$BRANCH_NAME stays stale and reset --hard silently resets to exactly the wrong commit this PR is trying to avoid — a no-op that looks like a fix. Given the whole value of the change is "the reset lands on the fetched commit", it's worth making that guarantee explicit rather than inherited:

git fetch origin "+refs/heads/$BRANCH_NAME:refs/remotes/origin/$BRANCH_NAME"

Worth noting the existing git checkout "$BRANCH_NAME" already depends on the same ref for its DWIM branch creation on a fresh checkout, so this isn't a regression the PR introduces — just an assumption now load-bearing for correctness rather than only for convenience.

2. Under set -e, git checkout runs before the reset and can abort the job first

The script is #!/bin/bash -eu (line 1). If a reused working copy has modified tracked files that differ between the current tip and $BRANCH_NAME, git checkout "$BRANCH_NAME" fails with "Your local changes to the following files would be overwritten by checkout" and the script exits before line 28 ever runs. That's the same class of leftover state the PR is defending against, just tracked-file dirt instead of a stale ref.

Both points collapse into one command that is also shorter than what's there now:

# Buildkite can reuse a working copy where "$BRANCH_NAME" was left dirty or at an older commit by a
# previous job. `-B` (re)points the local branch at the fetched commit and `-f` drops leftover local
# changes — rather than `git pull`, which needs an extra round trip and could merge on divergence.
git fetch origin "+refs/heads/$BRANCH_NAME:refs/remotes/origin/$BRANCH_NAME"
git checkout -f -B "$BRANCH_NAME" "origin/$BRANCH_NAME"

-B creates-or-resets the branch to the start point, subsuming the reset --hard. Discarding local changes is safe for every caller — all four invoke this script as the first step of the job, before secrets are applied or any file is generated:

• .buildkite/commands/release-build.sh:5
• .buildkite/release-pipelines/finalize-release.yml:12
• .buildkite/release-pipelines/finalize-hotfix-release.yml:12
• .buildkite/release-pipelines/new-beta-release.yml:12

One caveat if you take this: neither -f nor reset --hard removes untracked files. Buildkite's between-job clean normally handles those, so I wouldn't add git clean -ffdx speculatively — just don't read either form as "the tree is pristine".

Fix this →

Checked and clear

• No upstream-tracking regression from dropping git pull. reset --hard doesn't touch branch.<name>.merge/.remote, so any later refspec-less git push (the fastlane version bump, finalize_release) resolves exactly as before.
• Security: nothing sensitive touched. $BRANCH_NAME is derived from $1/$BUILDKITE_BRANCH and is consistently quoted; it's passed as a single argv element to git, not through a shell, so there's no injection surface. The ^release/ guard at lines 16-21 already constrains the $BUILDKITE_BRANCH path.
• Performance: reset --hard is local-only, so this adds no network work — a genuine improvement over git pull.
• Testing claim in the description holds: on a fresh checkout the local branch is already at the fetched commit, so the reset is a no-op.

Nit, ignore freely: the comment on line 26 quotes "$BRANCH_NAME" as if it were shell code, which reads a little oddly inside a comment where nothing expands.

I didn't run builds or tests — this is a .buildkite shell script with no Gradle or Kotlin surface, and the repo has no shellcheck configuration to run against it. I also couldn't run a local git fixture to demonstrate point 1 empirically (sandbox declined the scratch-directory writes), so that one is reasoned from git's fetch semantics and flagged as conditional on the remote's refspec rather than as an observed failure.
· branch ainfra-2725/reset-release-branch-checkout

[AliSoftware]

About the 2 points Claude raises:

1. 👍 The first point seems a decent feedback, I'll investigate in more depth tomorrow.
2. 👎 Second point is not really a concern, and we don't want to apply the suggested fix, because:
   • We actually want git checkout to fail if the release/ branch does not exist—rather than creating it if it does not, which is what git checkout -B would do.
   • During my chat with Claude on this issue (see also the Slack thread attached to the Linear issue), we explicitly rejected git checkout -B (and Claude was the one who suggested git reset --hard would be better for our use case and needs here) exactly because git checkout -B would have the side-effect of creating the branch if it did not exist (even though, technically, if that were the case, I think the git fetch would have failed first. But still, semantically we don't want to "checkout-or-create", we want to "checkout-and-fail-if-not-exists, then reset") while we want to avoid that behavior in this (unlikely) possibility.

[claude]

Claude finished @AliSoftware's task in 1m 3s —— View job

---

Re-review of b2fc3bf

• Diff the new commit against the previously reviewed state
• Check that mokagio's wording suggestion was applied as given
• Re-check my earlier point 2 against @AliSoftware's reasoning
• Confirm the status of my earlier point 1
• Check the merge gate named in the PR description

---

The only change since my last pass is the comment wording. It matches mokagio's suggestion verbatim, and the ambiguity they flagged is gone — "force the local branch to the fetched commit" says what the reset does without reading as an operation performed on the remote. .buildkite/commands/checkout-release-branch.sh:24-29 is otherwise unchanged, and it's still the only git branch manipulation anywhere under .buildkite/.

Point 2 (checkout -f -B) — withdrawn, your reasoning is right

"checkout-and-fail-if-not-exists, then reset" is the semantics you want, and plain git checkout is what encodes it. -B would turn a missing release branch into a silently created one — exactly the wrong failure mode for a script whose whole job is to guarantee the build sits on a specific released commit. And you're right that the git fetch on line 24 would already have failed under -eu (fatal: couldn't find remote ref …), so the create-or-reset path is unreachable in practice; but encoding the intent in the command rather than relying on an upstream guard is the better call regardless. Dropping it.

Point 1 (explicit fetch refspec) — still open, as you left it

Line 24 is unchanged, so git reset --hard "origin/$BRANCH_NAME" still relies on refs/remotes/origin/$BRANCH_NAME having been updated as an opportunistic side effect of the refspec-less fetch. Fine under the default +refs/heads/*:refs/remotes/origin/*; the concern is only that a narrowed refspec would make the reset a no-op that looks like the fix. Flagging it as still-outstanding rather than re-arguing it — you said you'd investigate, and this commit wasn't meant to address it.

Two cosmetic notes on the new comment, both ignorable

• "left at an older commit" — a stale refs/heads/release/x.y isn't necessarily older; if a previous job pushed a version bump and the remote branch was later force-moved, or the leftover ref came from a different release line, it could be unrelated rather than behind. "at a different commit" would cover the general case, and it's the general case the reset --hard actually handles.
• The wrap is uneven now — line 26 runs to ~110 chars while 27 and 28 are half-empty. Reflowing to three even lines reads better, though nothing enforces this (spotlessCheck doesn't cover .buildkite/**, and there's no shellcheck config in the repo).

Neither is worth a commit on its own; fold them in if you touch the file for point 1.

Merge gate reminder

Gemfile.lock:169 is still fastlane-plugin-wpmreleasetoolkit (14.11.1), so the bump your PR description makes a merge precondition hasn't landed here yet. Still blocked on release-toolkit#763 → gem release → bump in this same PR.

I didn't run builds or tests: the diff is a shell comment in .buildkite/, with no Gradle, Kotlin, or Ruby surface to exercise.
· branch ainfra-2725/reset-release-branch-checkout

[claude]

Claude finished @AliSoftware's task in 4m 10s —— View job

---

Re-review of c7e24f5 + cb5363c

• Diff the two new commits against the previously reviewed state
• Check whether the FETCH_HEAD switch closes my earlier point 1
• Re-verify the script end to end and its callers
• Check the merge gate named in the PR description

---

Two commits since my last pass. c7e24f5 closes my point 1; cb5363c is a larger rewrite than the PR description covers, and it contains one behaviour change worth an explicit sign-off rather than an implicit one.

Point 1 — closed, and FETCH_HEAD is the better fix

.buildkite/commands/checkout-release-branch.sh:34-40. Resetting to FETCH_HEAD is stronger than the explicit-refspec fetch I'd suggested: instead of making origin/$BRANCH_NAME trustworthy, it stops depending on it. git fetch origin <branch> writes FETCH_HEAD unconditionally, including when the fetch was a no-op, so line 40 lands on the commit line 34 just fetched regardless of remote.origin.fetch.

git checkout on line 35 still DWIMs off refs/remotes/origin/$BRANCH_NAME on a working copy with no local branch — but that residual dependency now fails loudly (fatal: invalid reference under -eu) instead of silently seating the build on a stale commit, and if the ref merely exists-but-stale, line 40 corrects it. That's the right shape.

One maintenance note, not a defect: FETCH_HEAD is repository-global positional state, so lines 34 and 40 are now coupled by adjacency. Any future git fetch, git submodule update, or git pull inserted between them silently redirects the reset. A comment or FETCHED_COMMIT="$(git rev-parse FETCH_HEAD)" right after the fetch would pin it, though at 6 lines apart I wouldn't bother today.

The "superset" claim in cb5363c doesn't hold for this repo

.buildkite/commands/checkout-release-branch.sh:20-31. The commit message says the new resolution order "is a superset of what every repo did before … so no call site needed changing". It's a superset for a non-empty argument. For an empty one it is strictly narrower than what pocket-casts-android did before:

# before
if [[ -n "${1:-}" ]]; then          # empty arg → falls through
elif [[ "${BUILDKITE_BRANCH:-}" =~ ^release/ ]]; then   # → recovers here

# after
if [[ $# -gt 0 ]]; then             # empty arg → matches
  RELEASE_VERSION="${1:?…}"         # → hard error, both fallbacks skipped

That case is reachable. All three release pipelines invoke the script as checkout-release-branch.sh "${RELEASE_VERSION}", and Buildkite interpolates an unset variable to the empty string at upload time (the same mechanism that makes $CI_TOOLKIT work) — so an operator triggering finalize-release without RELEASE_VERSION passes one empty argument. Previously the script recovered from the release/* branch the build was already on and finalized correctly; now it aborts:

• .buildkite/release-pipelines/finalize-release.yml:12
• .buildkite/release-pipelines/finalize-hotfix-release.yml:12
• .buildkite/release-pipelines/new-beta-release.yml:12

.buildkite/commands/release-build.sh is unaffected — its own ${1:?RELEASE_VERSION parameter missing} on line 4 fires first, so it can never forward an empty argument.

Your commit message calls this out deliberately ("is now a hard error everywhere, rather than … silently falling through to the current branch"), and for a release script I think aborting on ambiguous input is the right call — especially given the incident this PR descends from. So this is a request to make the accept explicit, not to change it. Two things follow from it:

• The PR description still says "No behaviour change on a fresh checkout". True of the reset, no longer true of the script: on a fresh checkout with an unset RELEASE_VERSION, the job now fails where it used to succeed. Worth a line in the description, since ReleasesV2 operators are the people who'd hit it.
• The same message argues the BUILDKITE_BRANCH fallback is safe ("cannot select a branch other than the one the build was already triggered on") — which is exactly the case being removed. If the goal is "never guess", ${1:?} is right; if it's "never guess wrong", the old fallback met that bar. Fine either way, but the two halves of the message pull in opposite directions.

Fix this →

Precedence level 2 is dead code in this repo

Line 24's RELEASE_VERSION environment fallback can't be reached from any call site here: all four pass an argument, so [[ $# -gt 0 ]] always wins. That's fine as cross-repo canonicalization — the commit message says one repo was env-only — but the header comment reads as a live three-tier safety net, and in this repo only tier 1 is ever exercised. No change needed; just don't let a future reader assume tiers 2 and 3 are load-bearing here.

Checked and clear

• Bug fixes in cb5363c verified. Under bash -eu, both [[ -z "${RELEASE_VERSION}" ]] on an unset variable and a bare RELEASE_VERSION=$1 with no arguments do abort with unbound variable before printing their usage message. Neither shape was present in this repo's version, so those are cross-repo fixes landing here for uniformity, not fixes to a live bug.
• FETCH_HEAD can't be shadowed by a tag. Tag auto-following can only append entries after the requested refspec's, and git rev-parse FETCH_HEAD resolves the first line — so line 40 gets the branch, not a tag. Reasoned from git's fetch semantics; the sandbox again declined the scratch-directory writes for a local fixture, so this is not empirically demonstrated.
• No upstream-tracking regression. reset --hard doesn't touch branch.<name>.merge/.remote, so the later refspec-less git push from the fastlane version bump and finalize_release resolves as before. No submodules in the repo, so reset --hard not updating submodule worktrees is moot.
• Security. $BRANCH_NAME derives from $1/RELEASE_VERSION/BUILDKITE_BRANCH, is consistently quoted, and reaches git as a single argv element — no injection surface. Nothing sensitive touched; all callers run this before configure_apply.
• Still the only git branch manipulation under .buildkite/.
• Earlier cosmetic notes (the ~110-char line 36, "older commit" vs "different commit", shell-style quoting inside a comment) are unchanged. Genuinely ignorable; I'm not re-arguing them.

Merge gate

Gemfile.lock:169 is still fastlane-plugin-wpmreleasetoolkit (14.11.1), so the bump your description makes a merge precondition hasn't landed. Still blocked on release-toolkit#763 → gem release → bump here.

I didn't run builds or tests: the diff is one .buildkite shell script, with no Gradle, Kotlin, or Ruby surface to exercise, and no shellcheck config in the repo to lint it with. spotlessCheck doesn't cover .buildkite/**. Both Gradle Wrapper Validation runs on this push passed.
· branch ainfra-2725/reset-release-branch-checkout

[AliSoftware]

@mokagio heads-up — these PRs grew since your review, so they're worth another look rather than assuming they're still the three-line change you saw. Same comment on all 13 repos.

Two commits were added on top of the wording fix you suggested:

1. Reset to FETCH_HEAD rather than origin/$BRANCH_NAME. git fetch origin <branch> always writes FETCH_HEAD, but it only updates refs/remotes/origin/<branch> when the remote's configured fetch refspec covers that branch. With Buildkite's default +refs/heads/*:refs/remotes/origin/* the two are equivalent — but on a clone whose refspec was narrowed after origin/<branch> already existed, the fetch leaves that ref stale and the reset lands on the old commit: the very failure this script exists to prevent, reintroduced through the back door. Resetting to FETCH_HEAD drops the refspec dependency entirely.

2. Standardized the script across all 13 repos. Tangent to the original issue, bundled deliberately. The script had drifted into five different shapes — argument required via ${1?…} or ${1:?…}, argument plus a BUILDKITE_BRANCH fallback, argument with a hand-rolled usage check, the same under a different variable name, and (in simplenote-android) no argument at all, reading RELEASE_VERSION from the environment. Having rolled the same one-line fix out thirteen times this week, that divergence is pure friction.

All 13 now share one canonical script. The resolution order is a superset of what every repo did before — argument, then RELEASE_VERSION from the environment, then the release/* branch the build runs on — so no call site needed changing. The BUILDKITE_BRANCH fallback only fires on a branch matching ^release/, and it derives the branch name back from that same value, so it cannot select a branch other than the one the build was already triggered on.

It also closes two latent bugs: under bash -eu, both [[ -z "${RELEASE_VERSION}" ]] on an unset variable and a bare RELEASE_VERSION=$1 with no arguments abort with unbound variable before their intended usage message can print. And an argument that is passed but empty — what happens when a pipeline forwards an unset $RELEASE_VERSION — is now a hard error everywhere, instead of resolving to release/ or silently falling through to the current branch.

The scripts are byte-identical across all 13 repos and pass shellcheck everywhere; the argument resolution was exercised across every combination (argument / empty argument / environment variable / release/* / trunk / feature branch / unset). Full rationale in the updated PR description.

Worth flagging that the release-toolkit PR also picked up a real fix from your review — the tag-based lookup was preferring a leftover draft over the release that actually owns the tag.

[claude]

Claude finished @AliSoftware's task in 3m 30s —— View job

---

Re-review of 76470ed

• Diff the new commit against the previously reviewed state
• Verify the new resolution logic across all input combinations
• Re-check my earlier "superset" concern against the new shape
• Confirm the merge gate

---

One commit since my last pass, and it resolves the substantive point I raised on cb5363c. The three-way if/elif/else collapsing into an assignment plus two guards reads better, and the : # Already provided… no-op branch was the weakest part of the previous shape — good to see it gone.

The "superset" claim now actually holds

.buildkite/commands/checkout-release-branch.sh:20-29. My objection to cb5363c was that [[ $# -gt 0 ]] + ${1:?…} turned an empty argument into a hard error, where this repo previously recovered from BUILDKITE_BRANCH — reachable because all three release pipelines pass "${RELEASE_VERSION}" and Buildkite interpolates an unset variable to the empty string at upload time. Line 20 fixes that:

RELEASE_VERSION="${1:-${RELEASE_VERSION:-}}"

${1:-…} (colon form) treats empty and unset identically, so an empty argument now falls through to the environment and then to the release/* branch, ending at the same generic error as the unset case. Traced against origin/main's version, the resolution is now a genuine superset for every input:

input	origin/main	this PR
non-empty arg	arg	arg
empty arg, on release/X	X	X
empty arg, on trunk	error	error
no arg, on release/X	X	X
no arg, RELEASE_VERSION in env	error	env value (new tier)
nothing available	error	error

So the note I asked for in the PR description isn't needed any more — the description is accurate again as written.

The nested :- guard is necessary, and the commit message gets the reason exactly right

Worth confirming since it's the kind of thing a future reader may try to "simplify" back: bash evaluates the default word of ${1:-word} lazily, so ${1:-$RELEASE_VERSION} would not abort when an argument is passed — it would abort only when neither $1 nor RELEASE_VERSION is set, i.e. precisely the case that needs to reach the usage error on line 27. Under bash -eu the script would die with RELEASE_VERSION: unbound variable instead. The inner :- is load-bearing and the comment history explains why; nothing to change.

Also confirmed the self-reference on line 20 is safe — the RHS is fully expanded before the assignment — and that reassigning an already-exported RELEASE_VERSION preserves the export attribute, so nothing downstream sees a difference (moot here anyway, since every caller runs this as a subprocess rather than sourcing it).

Two notes on the trade-off you accepted, neither a blocker

Env now outranks the branch fallback for an empty argument. New ordering relative to origin/main, which had no env tier: empty arg + runtime RELEASE_VERSION=X + build on release/Y resolved to Y before and resolves to X now. Unreachable from any call site in this repo — nothing sets RELEASE_VERSION in these jobs' runtime env; it's interpolated into the argument at upload time — and the explicit variable is the better signal if it ever is set. Noting it only because it's the one row where the two versions disagree on a non-error outcome.

Empty argument is now a silent recovery rather than a loud failure. Your commit message justifies this on the grounds that the fallback "resolves to the branch the build is already running on, which cannot be a different branch than intended". That holds for finalize-release.yml and new-beta-release.yml. For .buildkite/release-pipelines/finalize-hotfix-release.yml:12 it rests on ReleasesV2 triggering the job on the hotfix branch (release/X.Y.Z, as created by new_hotfix_release) rather than on the base release/X.Y — if it ever triggered on the base with RELEASE_VERSION unset at upload, the script would now silently finalize the base release instead of erroring. That's origin/main's behaviour restored, not a regression this PR introduces, and I believe the trigger branch is the hotfix branch in practice. Flagging it because this PR descends from a wrong-commit-released incident, and that one call site is the only place cb5363c's stricter variant had a defensible edge. Your call, and I'd accept it as-is.

.buildkite/commands/release-build.sh is unaffected either way — its own ${1:?RELEASE_VERSION parameter missing} on line 4 fires on an empty value first, so it can never forward one.

Checked and clear

• My earlier point 1 (FETCH_HEAD) stays closed. Lines 32-38 are untouched by this commit. The adjacency coupling between the fetch and the reset is still a maintenance note only, not a defect.
• "a different commit" replaced "an older commit" on line 34, which was my suggestion — a stale local ref genuinely needn't be behind. Line 34 is now ~114 chars while 35-37 sit around half that; still nothing enforces wrapping here (spotlessCheck doesn't cover .buildkite/**), so still ignorable.
• Header comment, line 7: "the first of these that is set and non-empty" is accurate for tiers 1-2; tier 3 additionally requires ^release/, and a branch named exactly release/ yields an empty version and lands on the generic error. Correct behaviour, marginally under-described comment. Not worth a commit.
• Security. Unchanged surface. $BRANCH_NAME derives from $1 / RELEASE_VERSION / BUILDKITE_BRANCH, is consistently quoted, and reaches git as a single argv element — no injection path. All callers still run this before configure_apply, so no secrets are in play. BUILDKITE_BRANCH=release/1.2/3 round-trips to the same branch name, so the strip-and-reprefix can't retarget.
• Performance. No change; reset --hard remains local-only.
• Still the only git branch manipulation anywhere under .buildkite/.

Merge gate

Gemfile.lock:169 is still fastlane-plugin-wpmreleasetoolkit (14.11.1), so the bump your description makes a merge precondition hasn't landed. Still blocked on release-toolkit#763 → gem release → bump here.

I did not empirically run the resolution matrix this time: the sandbox declined executing a scratch harness, and shellcheck isn't installed on this runner, so the table above is traced from bash parameter-expansion semantics rather than observed. Your own cross-repo test run covers the same combinations. No builds or tests run — the diff is one .buildkite shell script with no Gradle, Kotlin, or Ruby surface.
· branch ainfra-2725/reset-release-branch-checkout

[claude]

Claude finished @AliSoftware's task in 49s —— View job

---

Re-review of d2a3ee1

• Diff the new commit against the previously reviewed state
• Re-verify the full script end to end
• Confirm the merge gate

---

Comment-only change since 76470ed — the three-sentence rationale is now a lead-in line plus two bullets. git diff 76470ed..HEAD touches nothing but lines 34-37; the executable body of .buildkite/commands/checkout-release-branch.sh is byte-identical.

It reads better. The old form buried two independent "X rather than Y" justifications in one run-on sentence with an awkward mid-clause wrap; splitting them makes each choice scannable on its own line, which is what someone opening this file at 3am during a release actually needs.

Everything I've previously signed off on still holds and I'm not re-arguing any of it:

• Reset target (line 38): FETCH_HEAD closes my original point 1 — no dependence on remote.origin.fetch covering the branch.
• Resolution order (lines 20-29): genuine superset of origin/main for every input, including the empty-argument case Buildkite produces from an unset RELEASE_VERSION.
• git checkout without -B (line 33): deliberate, per your reasoning — fail on a missing release branch rather than silently create it.
• Security / performance: unchanged surface. $BRANCH_NAME is consistently quoted and reaches git as a single argv element; reset --hard is local-only.
• Still the only git branch manipulation anywhere under .buildkite/.

Two cosmetic notes, ignorable

• The bullets are punctuated inconsistently — line 36 has no terminating period, line 37 does. Trivial, but they're a parallel pair.
• Line 34 (~107 chars) and line 37 (~110) are still the long ones while 35-36 sit near half that. Nothing enforces wrapping here (spotlessCheck doesn't cover .buildkite/**, and there's no shellcheck config in the repo), and the bullet form makes the ragged right read as intentional structure rather than a bad wrap — so this bothers me less than it did on the previous shape.

Not worth a commit on their own.

Merge gate

Gemfile.lock:169 is still fastlane-plugin-wpmreleasetoolkit (14.11.1), so the bump your description makes a merge precondition hasn't landed. Still blocked on release-toolkit#763 → gem release → bump in this same PR.

No builds or tests run: the diff is three shell comment lines, with no Gradle, Kotlin, or Ruby surface to exercise.
· branch ainfra-2725/reset-release-branch-checkout

[mokagio]

Reviewed the updated change on bloom/DayOne-Android#8243. The script is byte-identical across every repo in this rollout (verified: same sha256, same 1665 bytes), so that review applies here unchanged.

Short version: a worthwhile refinement on the original implementation. One non-blocking nitpick — git checkout "$BRANCH_NAME" still fails if the branch is absent locally and the fetch refspec doesn't create origin/$BRANCH_NAME, before the reset can reach FETCH_HEAD; git checkout -B "$BRANCH_NAME" FETCH_HEAD would do both and drop the dependency entirely.

As on #8243, withholding approval until the release-toolkit update lands, to avoid accidental merges as warned in the description.

Posted by Claude Code (Opus 5) on behalf of @mokagio with approval.

[AliSoftware]

a worthwhile refinement on the original implementation. One non-blocking nitpick — git checkout "$BRANCH_NAME" still fails if the branch is absent locally and the fetch refspec doesn't create origin/$BRANCH_NAME, before the reset can reach FETCH_HEAD; git checkout -B "$BRANCH_NAME" FETCH_HEAD would do both and drop the dependency entirely.

As already explained before, it is intentional that we don't use git checkout -B, precisely because we don't want this script to create the release branch if it doesn't exist, but instead intentionally want the script it fail if the branch doesn't exist

[mokagio]

@AliSoftware

As already explained before, it is intentional that we don't use git checkout -B

Where I went wrong in this set was that I used the DayOne-Android PR as the hub for my reviewing assuming all conversations would go through it. That was an oversight because I didn't actually suggest to operate in that way...

Based on that assumption, I only asked my agents to double check the changes were byte-identical. Had I asked to check for additional comments in all the other PRs, I would have seen the -B rationale and avoided the confusion.

Apologies.

[wpmobilebot]

Version 8.18 has now entered code-freeze, so the milestone of this PR has been updated to 8.19.

[claude]

Claude finished @AliSoftware's task in 2m 56s —— View job

---

Re-review of 22c236c

• Diff the new commit against the previously reviewed state
• Verify the Gemfile.lock bump resolves the merge gate
• Check the lock is internally consistent after the bump
• Confirm the gem fix is load-bearing in this repo
• Confirm the script is unchanged since my last pass

---

One commit since d2a3ee1, and it's the merge gate closing: fastlane-plugin-wpmreleasetoolkit 14.11.1 → 14.11.2. git diff d2a3ee1 HEAD -- .buildkite/ is empty — the shell script is byte-identical to what I signed off on, so everything from my previous passes stands unchanged and I'm not re-arguing any of it.

The lock bump checks out

Gemfile.lock:169. 14.11.2 satisfies Gemfile:9's ~> 14.11, so no manifest change was needed. The plugin's own dependency block is untouched, consistent with a patch release. Every transitive bump still satisfies its constraints:

gem	new version	constrained by	ok
json	2.21.2	< 3.0.0 (Gemfile.lock:142), ~> 2.3 (:317)	✅
aws-partitions	1.1277.0	~> 1, >= 1.992.0 (:15)	✅
aws-sdk-s3	1.228.2	~> 1.197 (:119)	✅

aws-sdk-core stays at 3.254.0, which is what both aws-sdk-s3 1.228.2 and aws-sdk-kms still ask for. No resolution is disturbed.

The gem half is genuinely load-bearing here — and this repo is the worse place to get it wrong

Worth stating explicitly, because "picks up an upstream fix" reviews often aren't checked against the local call site. fastlane/Fastfile:487-490 is exactly the name-based lookup #763 addresses:

publish_github_release(
  repository: GITHUB_REPO,
  name: version_number
)

Reached from .buildkite/release-pipelines/update-rollouts.yml:38. And what follows it raises the stakes above the generic case — fastlane/Fastfile:496-499 tears down the release branch immediately after publishing:

remove_branch_protection(repository: GITHUB_REPO, branch: current_branch)
Fastlane::Helper::GitHelper.delete_remote_branch_if_exists!(current_branch)

So if the wrong same-named draft is published, the tag lands on the wrong commit and the branch that would let you see what happened is gone in the same lane. The script fix (right commit on the branch) and the gem fix (right draft published from it) really are two halves of one guarantee, and this repo exercises both.

One inconsistency in how #763 is described

The commit message says #763 makes publish_github_release "publish the most recently created GitHub Release when several share the same name". Your earlier comment on this PR described the same fix as "the tag-based lookup was preferring a leftover draft over the release that actually owns the tag". Those aren't the same selection rule — most-recently-created vs. owns-the-tag. Probably the review on #763 changed the approach and the commit message describes the earlier shape, but someone tracing AINFRA-2725 from this repo's history in six months only sees the commit message. Worth a one-line correction there if the tag-based description is the accurate one.

I could not check this myself: WebFetch isn't permitted on this runner and curl/gh api to rubygems.org and the toolkit repo were declined by the sandbox. So I've verified 14.11.2 is internally coherent in the lockfile, but not that it's published upstream or that it contains #763 — that part rests on your bundle update having actually resolved it.

Scope note, disclosed and fine

Three unrelated transitive gems plus BUNDLED WITH 4.0.16 → 4.0.17 rode along, which your commit message calls out. For comparison, the previous toolkit bump (31c4e40, dependabot) touched 3 lines and left BUNDLED WITH alone; this one touches 10. Both harmless:

• The three drive-bys are in dependabot's ruby-minor-and-patch group and would have been bumped by it anyway.
• Nothing pins bundler — no gem install bundler or BUNDLER_VERSION anywhere under .buildkite/ or .github/workflows/, and .ruby-version is 3.4.9 — so a patch-level BUNDLED WITH bump has nothing to conflict with.

The merge gate is now closed — the description hasn't caught up

Gemfile.lock:169 is 14.11.2, so the precondition your description makes a merge blocker is satisfied. But the > [!NOTE] **Please do not merge this PR yet.** block at the top is still there, and @mokagio explicitly withheld approval "until the release-toolkit update lands, to avoid accidental merges as warned in the description". Left as-is, the description is now the only thing holding the PR. Two small edits worth making before requesting re-review:

• Drop or strike the do-not-merge NOTE and say the bump has landed in 22c236c.
• The Testing instructions cover only the script ("No behaviour change on a fresh checkout") and don't mention the gem side at all. A line on what 14.11.2 changes for publish_gh_release would help whoever validates the next release.

Fix this →

Checked and clear

• Script untouched. git diff d2a3ee1 HEAD -- .buildkite/ is empty. FETCH_HEAD reset (line 38), the resolution superset (lines 20-29), and the deliberate git checkout without -B (line 33) all stand as previously reviewed. Still the only git branch manipulation anywhere under .buildkite/.
• No other pin to update. fastlane-plugin-wpmreleasetoolkit appears only in Gemfile/Gemfile.lock — nothing under .buildkite/, .github/, or scripts/ hardcodes a toolkit version, so the lock bump is the complete change.
• Security. Lockfile version bumps only; no new gems, no new sources, no source changes. rubygems.org remains the only source. No secrets or credentials in the diff.
• Performance. Not applicable; no runtime code changed.

I ran no builds or tests: the diff is a lockfile, and with no network on this runner bundle install / bundle lock --local can't resolve to confirm the lock. There's no Gradle or Kotlin surface here, and spotlessCheck doesn't cover Gemfile.lock or .buildkite/**. The real check remains the next release build.
· branch ainfra-2725/reset-release-branch-checkout

[AliSoftware]

Updated release-toolkit to 14.11.2, the version that shipped wordpress-mobile/release-toolkit#763 — so both halves of AINFRA-2725 are now in this PR and it is no longer blocked. I removed the do-not-merge note from the description accordingly.

That release makes publish_github_release publish the most recently created GitHub Release when several share the same name, instead of whichever one the GitHub API happened to list first. It also fixes the tag-based lookup used by upload_github_release_assets, which could otherwise resolve to a leftover draft rather than the release that actually owns the tag.

Done with a real bundle update fastlane-plugin-wpmreleasetoolkit, so the lock was properly re-resolved rather than hand-edited. Two side effects worth naming:

• A few unrelated transitive gems that had newer releases were refreshed along with it (aws-partitions, aws-sdk-s3, json, and googleauth in one repo).
• BUNDLED WITH moved to the current 4.0.17. I pinned bundler explicitly for the whole batch so every repo lands on the same version — otherwise one of them would have been silently downgraded.