From 500b2a7526c3143e24cfc501b1b9321aebb8fa79 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sat, 5 Sep 2026 03:32:58 +0200 Subject: [PATCH 1/3] fix(impl-merge): let a re-dispatch finish the bookkeeping of an already-merged PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge step has long treated "already merged" as success and continued to promotion, labels, issue close and the Postgres sync — the whole point of dispatching the workflow again after a run that crashed post-merge. It never got there: the completeness check before it fetched the PR branch, and `gh pr merge --delete-branch` had removed that branch at the merge, so the re-run died on `couldn't find remote ref`. Seen on #11295 (2026-09-05): the GCP auth step failed 5 s after the squash, the images stayed in staging while the metadata on main already pointed at production, and the re-dispatch could not repair it. The check now reads the files from origin/main when the PR is merged, which is where the squash commit put them, and no longer tries to close a merged PR when they are absent. The babysit skill gets the gotcha and the status-cycle rule (compare landed pairs against production images). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LfPAdJKa4JWzvsEUQZUuhs --- .claude/skills/babysit-pipeline/SKILL.md | 13 +++++++ .github/workflows/impl-merge.yml | 37 +++++++++++++++---- .../impl-merge-redispatch-merged-pr.md | 12 ++++++ 3 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 changelog.d/impl-merge-redispatch-merged-pr.md diff --git a/.claude/skills/babysit-pipeline/SKILL.md b/.claude/skills/babysit-pipeline/SKILL.md index 3a4b4306b5e..1b47b7c0e44 100644 --- a/.claude/skills/babysit-pipeline/SKILL.md +++ b/.claude/skills/babysit-pipeline/SKILL.md @@ -259,6 +259,19 @@ nothing failed. put the spec on `rescue_specs.txt`: the driver's dispatch auto-closes every open PR of the pair, so a regeneration fired while a repair is mid-flight throws that work away. +- **A `Merge: PR #N` run that fails AFTER "Merge PR to main" leaves a + silent hole: the squash is on main (metadata pointing at production + URLs, the driver counts the pair as done) but the images are still in + staging, the `impl::done` label, issue close and Postgres sync + never ran.** Seen on #11295 (2026-09-05, GCP auth step failed 5 s + after the squash). Read the failed run's step list — a failure at or + after `Authenticate to GCP` with `Merge PR to main` green is this + case — then `gh workflow run impl-merge.yml -f pr_number=N`: the + workflow treats an already-merged PR as success and runs the + post-merge steps. Never promote the images by hand. Every status + cycle should compare the pairs landed on main against + `gs://anyplot-images/plots////plot-light.png`; a + merge failure in the run list is the trigger to look. - **`Merge: PR #N` failing five times with "Head branch is out of date" while `mergeable` stays `UNKNOWN` is a stuck PR object, not a branch problem.** Seen on #10850 (2026-09-01): `update-branch` had diff --git a/.github/workflows/impl-merge.yml b/.github/workflows/impl-merge.yml index 006b8a13ca0..f8aa0daab47 100644 --- a/.github/workflows/impl-merge.yml +++ b/.github/workflows/impl-merge.yml @@ -166,17 +166,38 @@ jobs: EXT: ${{ steps.extract.outputs.ext }} BRANCH: ${{ steps.check.outputs.branch }} PR_NUM: ${{ steps.check.outputs.pr_number }} + REPOSITORY: ${{ github.repository }} run: | - # Fetch the PR branch to check its contents - git fetch origin "$BRANCH" - IMPL_FILE="plots/${SPEC_ID}/implementations/${LANGUAGE}/${LIBRARY}${EXT}" META_FILE="plots/${SPEC_ID}/metadata/${LANGUAGE}/${LIBRARY}.yaml" - # Check if implementation file exists on the PR branch - if ! git show "origin/${BRANCH}:${IMPL_FILE}" &>/dev/null; then - echo "::error::Implementation file missing on branch: ${IMPL_FILE}" + # A re-dispatch for a PR that is already merged has no branch to + # fetch: `gh pr merge --delete-branch` removed it. That is exactly + # the run this dispatch exists for — the first one merged and then + # crashed before the post-merge bookkeeping (PR #11295: the GCP auth + # step failed 5 s after the squash, so the images stayed in staging + # while the metadata on main already pointed at production). Read + # the files from main in that case, which is where the squash + # commit put them, so the run reaches promotion, labels and sync. + STATE=$(gh pr view "$PR_NUM" --repo "$REPOSITORY" --json state -q .state 2>/dev/null || echo "") + if [ "$STATE" = "MERGED" ]; then + echo "::notice::PR #${PR_NUM} is already merged — validating against origin/main" + git fetch origin main + REF="origin/main" + else + # Fetch the PR branch to check its contents + git fetch origin "$BRANCH" + REF="origin/${BRANCH}" + fi + + # Check if implementation file exists on the PR branch (or on main) + if ! git show "${REF}:${IMPL_FILE}" &>/dev/null; then + echo "::error::Implementation file missing on ${REF}: ${IMPL_FILE}" echo "::error::This indicates an incomplete generation - metadata exists but implementation is missing" + if [ "$STATE" = "MERGED" ]; then + echo "::error::PR is already merged — nothing to close; the implementation needs a regeneration" + exit 1 + fi echo "::error::Closing PR to prevent partial merge" gh pr close "$PR_NUM" --comment "**Merge blocked:** Implementation file \`${IMPL_FILE}\` is missing from this PR branch. Only metadata was found. This indicates an incomplete code generation. Please regenerate using \`generate:${LIBRARY}\` label." @@ -185,8 +206,8 @@ jobs: fi # Check if metadata file exists - if ! git show "origin/${BRANCH}:${META_FILE}" &>/dev/null; then - echo "::warning::Metadata file missing on branch: ${META_FILE}" + if ! git show "${REF}:${META_FILE}" &>/dev/null; then + echo "::warning::Metadata file missing on ${REF}: ${META_FILE}" echo "::warning::This is unusual but not blocking - metadata will be created on next review" fi diff --git a/changelog.d/impl-merge-redispatch-merged-pr.md b/changelog.d/impl-merge-redispatch-merged-pr.md new file mode 100644 index 00000000000..b4c632430c0 --- /dev/null +++ b/changelog.d/impl-merge-redispatch-merged-pr.md @@ -0,0 +1,12 @@ +### Fixed + +- **A re-dispatched `impl-merge` run can finish the bookkeeping of a PR that is already merged** — + the merge step has long treated "already merged" as success and continued to promotion, + labels, issue close and the Postgres sync, which is the whole point of dispatching the + workflow again after a run that crashed post-merge. It never got there: the completeness + check before it fetched the PR branch, and `gh pr merge --delete-branch` had removed that + branch at the merge, so the re-run died on `couldn't find remote ref`. Seen on #11295 + (2026-09-05): the GCP auth step failed 5 s after the squash, the images stayed in staging + while the metadata on main already pointed at production, and the re-dispatch could not + repair it. The check now reads the files from `origin/main` when the PR is merged — where + the squash commit put them — and no longer tries to close a merged PR when they are absent. From 7cd7b17f10bea2370ae57e397173aef2f4f47caf Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sat, 5 Sep 2026 03:33:25 +0200 Subject: [PATCH 2/3] chore(changelog): add PR reference (#11307) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LfPAdJKa4JWzvsEUQZUuhs --- changelog.d/impl-merge-redispatch-merged-pr.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/impl-merge-redispatch-merged-pr.md b/changelog.d/impl-merge-redispatch-merged-pr.md index b4c632430c0..2748b820f67 100644 --- a/changelog.d/impl-merge-redispatch-merged-pr.md +++ b/changelog.d/impl-merge-redispatch-merged-pr.md @@ -9,4 +9,4 @@ (2026-09-05): the GCP auth step failed 5 s after the squash, the images stayed in staging while the metadata on main already pointed at production, and the re-dispatch could not repair it. The check now reads the files from `origin/main` when the PR is merged — where - the squash commit put them — and no longer tries to close a merged PR when they are absent. + the squash commit put them — and no longer tries to close a merged PR when they are absent. (#11307) From 4a67cd4e78bbff4bf70a45706f7f723a5afeafd0 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Sat, 5 Sep 2026 03:47:37 +0200 Subject: [PATCH 3/3] fix(impl-merge): retry the PR-state probe before choosing the validation ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #11307: a transient failure of the new state lookup read as "not merged" and fell through to the branch fetch — the exact failure the lookup exists to avoid. Same 3x/linear-backoff shape as the merge step's probe; unknown after three tries stops the run instead of guessing the ref. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LfPAdJKa4JWzvsEUQZUuhs --- .github/workflows/impl-merge.yml | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/impl-merge.yml b/.github/workflows/impl-merge.yml index f8aa0daab47..e48e25553c5 100644 --- a/.github/workflows/impl-merge.yml +++ b/.github/workflows/impl-merge.yml @@ -179,7 +179,27 @@ jobs: # while the metadata on main already pointed at production). Read # the files from main in that case, which is where the squash # commit put them, so the run reaches promotion, labels and sync. - STATE=$(gh pr view "$PR_NUM" --repo "$REPOSITORY" --json state -q .state 2>/dev/null || echo "") + # Retried like the other state probes in this workflow: a blip here + # read as "not merged" would fall through to the branch fetch and + # reproduce the very failure this branch of the logic exists for. + # Unknown after three tries is a hard stop — re-dispatching later is + # cheap, guessing the ref is not. + STATE="" + for probe in 1 2 3; do + if STATE=$(gh pr view "$PR_NUM" --repo "$REPOSITORY" --json state -q .state 2> /tmp/state.err); then + break + fi + SERR=$(head -c 500 /tmp/state.err | tr '\n' ' ') + echo "::warning::gh pr view (state) failed (probe ${probe}/3): ${SERR}" + STATE="" + if [ "$probe" -lt 3 ]; then + sleep $((probe * 5)) + fi + done + if [ -z "$STATE" ]; then + echo "::error::Could not determine the state of PR #${PR_NUM} after 3 attempts — not guessing which ref to validate; re-dispatch when the API is back" + exit 1 + fi if [ "$STATE" = "MERGED" ]; then echo "::notice::PR #${PR_NUM} is already merged — validating against origin/main" git fetch origin main