From 4d85f6bea506f84a29e6f4eb76dce58161b07dfb Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:26:54 +0200 Subject: [PATCH 1/2] fix(impl-review): retry the staging download instead of reviewing an empty directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The step swallowed a lost `gsutil cp` (`2>/dev/null || true`), the render check right after it then failed on nothing, and because that failure set no ai-review-failed label, no watchdog case picked the PR up — it waited for a manual re-dispatch (#11360 on 2026-09-05, #11678 on 2026-09-09; staging was complete both times). The download now tries three times with a short backoff, stops as soon as both theme renders are on disk, and keeps gsutil's stderr in a warning per failed attempt: a transfer blip heals in the same run, a genuinely empty staging folder still fails loudly at the render check. The babysit skill gets the gotcha (how to tell the two apart, the one-shot re-dispatch). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LfPAdJKa4JWzvsEUQZUuhs --- .claude/skills/babysit-pipeline/SKILL.md | 10 ++++++++++ .github/workflows/impl-review.yml | 17 ++++++++++++++++- .../impl-review-staging-download-retry.md | 10 ++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 changelog.d/impl-review-staging-download-retry.md diff --git a/.claude/skills/babysit-pipeline/SKILL.md b/.claude/skills/babysit-pipeline/SKILL.md index 1b47b7c0e44..e5672d9b681 100644 --- a/.claude/skills/babysit-pipeline/SKILL.md +++ b/.claude/skills/babysit-pipeline/SKILL.md @@ -259,6 +259,16 @@ 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 `Review: PR #N` run failing at "Verify both theme renders exist" + with staging complete is a lost download, not a missing render.** The + PR keeps no verdict label, so no watchdog case matches and the driver + waits an hour before calling the spec stalled. Check + `gs://anyplot-images/staging////` — 18 objects means + the generate/repair upload was fine — then `gh workflow run + impl-review.yml -f pr_number=N` once, only when no `Review: PR #N` + run is active. The download step retries since the 2026-09-09 fix; + a repeat on the same PR after that means the staging folder really is + empty and the pair needs a fresh generate. - **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 diff --git a/.github/workflows/impl-review.yml b/.github/workflows/impl-review.yml index 0bdc160b1f8..0477893c9a2 100644 --- a/.github/workflows/impl-review.yml +++ b/.github/workflows/impl-review.yml @@ -178,7 +178,22 @@ jobs: LANGUAGE: ${{ steps.lang.outputs.language }} run: | mkdir -p plot_images - gsutil -m cp "gs://anyplot-images/staging/${SPEC_ID}/${LANGUAGE}/${LIBRARY}/*" plot_images/ 2>/dev/null || true + # Retried: a single lost transfer used to be swallowed here (`2>/dev/null + # || true`), and the next step then failed on an empty directory with + # no label set — no watchdog case matches that, so the PR sat until + # someone re-dispatched the review by hand (#11360 on 2026-09-05, + # #11678 on 2026-09-09; staging was complete both times). Keep stderr: + # a transfer blip and a missing staging folder need different answers. + SRC="gs://anyplot-images/staging/${SPEC_ID}/${LANGUAGE}/${LIBRARY}/*" + for attempt in 1 2 3; do + if gsutil -m cp "$SRC" plot_images/ 2> /tmp/gsutil.err \ + && [ -f plot_images/plot-light.png ] && [ -f plot_images/plot-dark.png ]; then + break + fi + GERR=$(head -c 300 /tmp/gsutil.err | tr '\n' ' ') + echo "::warning::staging download incomplete (attempt ${attempt}/3): ${GERR:-no error output}" + [ "$attempt" -lt 3 ] && sleep $((attempt * 10)) + done ls -la plot_images/ - name: Verify both theme renders exist diff --git a/changelog.d/impl-review-staging-download-retry.md b/changelog.d/impl-review-staging-download-retry.md new file mode 100644 index 00000000000..277094a91fa --- /dev/null +++ b/changelog.d/impl-review-staging-download-retry.md @@ -0,0 +1,10 @@ +### Fixed + +- **`impl-review` retries the staging download instead of reviewing an empty directory** — the + step swallowed a lost `gsutil cp` (`2>/dev/null || true`), the render check right after it + then failed on nothing, and because that failure set no `ai-review-failed` label, no watchdog + case picked the PR up: it waited for a manual re-dispatch (#11360 on 2026-09-05, #11678 on + 2026-09-09, staging complete both times). The download now tries three times with a short + backoff, stops as soon as both theme renders are on disk, and keeps `gsutil`'s stderr in a + warning per failed attempt, so a transfer blip heals in the same run and a genuinely empty + staging folder still fails loudly at the render check. From 70a2aa882b8851f9e577b4bc465b6429aa3cf9c0 Mon Sep 17 00:00:00 2001 From: Markus Neusinger <2921697+MarkusNeusinger@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:27:16 +0200 Subject: [PATCH 2/2] chore(changelog): add PR reference (#11697) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LfPAdJKa4JWzvsEUQZUuhs --- changelog.d/impl-review-staging-download-retry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/impl-review-staging-download-retry.md b/changelog.d/impl-review-staging-download-retry.md index 277094a91fa..372d596e033 100644 --- a/changelog.d/impl-review-staging-download-retry.md +++ b/changelog.d/impl-review-staging-download-retry.md @@ -7,4 +7,4 @@ 2026-09-09, staging complete both times). The download now tries three times with a short backoff, stops as soon as both theme renders are on disk, and keeps `gsutil`'s stderr in a warning per failed attempt, so a transfer blip heals in the same run and a genuinely empty - staging folder still fails loudly at the render check. + staging folder still fails loudly at the render check. (#11697)