Skip to content

Commit 6d66533

Browse files
committed
Comment the ordering invariants, not the incident
Both ordering comments narrated the bug being prevented ("the step that previously failed", "the failure mode where we pushed a merge against a bogus target and crashed"). Restate them as the rules a fresh reader needs: the base check runs before any mutation because a diverged base makes the recorded target stale, and the label drops last so any earlier failure stays resumable.
1 parent 9884e0b commit 6d66533

1 file changed

Lines changed: 8 additions & 6 deletions

File tree

update-pr-stack.sh

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,9 @@ continue_after_resolution() {
250250
# The base we left the PR on while waiting for conflict resolution was the
251251
# merged parent branch. If it no longer matches, a human retargeted the PR
252252
# (e.g. straight onto the integration branch); we are no longer the authority
253-
# on its base, so we step back without touching the branch. Doing this BEFORE
254-
# any mutation is what stops the failure mode where we pushed a merge built
255-
# against a bogus target and then crashed.
253+
# on its base, so we step back without touching the branch. This runs before
254+
# any mutation: once the base diverges, the recorded target is stale and a
255+
# merge built against it would be wrong.
256256
local CURRENT_BASE
257257
CURRENT_BASE=$(gh pr view "$PR_BRANCH" --json baseRefName --jq '.baseRefName')
258258
if [[ "$CURRENT_BASE" != "$RECORDED_BASE" ]]; then
@@ -285,9 +285,11 @@ continue_after_resolution() {
285285
return 1
286286
fi
287287

288-
# Order matters: push the cleaned-up head, then retarget the base, and only
289-
# then drop the label. The retarget is the step that previously failed; if
290-
# anything here fails the label stays, so the next push resumes.
288+
# Drop the label last: it is what re-triggers this action, so while any
289+
# earlier step can still fail it must stay on to let the next push resume.
290+
# Push the cleaned-up head before retargeting so the head already contains
291+
# NEW_TARGET when the base flips to it, keeping the PR mergeable (GitHub
292+
# suppresses CI on a PR that conflicts with its base).
291293
log_cmd git push origin "$PR_BRANCH"
292294
log_cmd gh pr edit "$PR_BRANCH" --base "$NEW_TARGET"
293295
log_cmd gh pr edit "$PR_BRANCH" --remove-label "$CONFLICT_LABEL"

0 commit comments

Comments
 (0)