Skip to content

Commit caf5b79

Browse files
committed
ci(shard-timings): give the write-back step every variable it expands, and rehearse it on pull_request
The step that pushes the refresh branch and opens the PR expanded RUN_COUNT and RUNS in its commit message under set -u, but its env: exported only GH_TOKEN, RUN_ID and HEAD_SHA, so the scheduled refresh died at git commit after the dataset had been computed. Export all four generate outputs, as the compose step already does. The pull_request dry run was a separate, mutually exclusive step, so the script that writes never ran before a merge. Fold the two into one step that runs on every event with one env: block; the branch, add and commit run for real on the runner, and every act that leaves it (git push, gh pr create, gh pr view, the label write and read-back) goes through a single outward() switch driven by DRY_RUN. A variable missing from env: now reds the pull_request run that dropped it. The script refuses a DRY_RUN other than true/false, and refuses false on a pull_request run. Claude-Session: https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr Co-authored-by: Claude <noreply@anthropic.com>
1 parent fdeeea0 commit caf5b79

1 file changed

Lines changed: 108 additions & 39 deletions

File tree

‎.github/workflows/shard-timings-refresh.yml‎

Lines changed: 108 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,14 @@ on:
139139
# until the next sweep.
140140
#
141141
# On a `pull_request` run everything executes — selection, download,
142-
# regeneration, the coverage check, the byte comparison and the partitioner's
143-
# verdict — so the transport and the flags are proven on a real runner rather
144-
# than argued about. The WRITE is what is skipped: no branch is pushed, no PR
145-
# is opened, no label is written, and the body that would have been posted is
146-
# rendered to the run's step summary instead.
142+
# regeneration, the coverage check, the byte comparison, the partitioner's
143+
# verdict, and the write step's own script with the write step's own `env:`,
144+
# up to and including the local commit — so the transport, the flags and the
145+
# write step's variables are proven on a real runner rather than argued
146+
# about. Only the acts that leave the runner are switched off, behind the one
147+
# `DRY_RUN` switch in that step: no branch is pushed, no PR is opened, no
148+
# label is written, and the body that would have been posted is rendered to
149+
# the run's step summary instead.
147150
pull_request:
148151
paths:
149152
- '.github/workflows/shard-timings-refresh.yml'
@@ -517,8 +520,9 @@ jobs:
517520
518521
# Composition is separated from the WRITE on purpose: a `pull_request` run
519522
# of this lane must exercise the body-building — the shard durations, the
520-
# bins, the conditional blocks — without pushing anything. Both the write
521-
# step and the dry-run step below consume this file.
523+
# bins, the conditional blocks — without pushing anything. The write step
524+
# below consumes this file on every event: as the PR body when it writes,
525+
# and as the step summary on a `pull_request` dry run.
522526
- name: Compose the pull request body
523527
if: steps.compare.outputs.changed == 'true'
524528
env:
@@ -631,16 +635,81 @@ jobs:
631635
} > "$RUNNER_TEMP/pr-body.md"
632636
echo "Composed a $(wc -l < "$RUNNER_TEMP/pr-body.md")-line pull request body."
633637
634-
# The WRITE. Skipped on a `pull_request` run of this lane: a PR that only
635-
# edits this workflow must never push a bot branch or open a second PR.
636-
- name: Push the refresh branch and open the pull request
637-
if: steps.compare.outputs.changed == 'true' && github.event_name != 'pull_request'
638+
# The WRITE — and, on a `pull_request` run of this lane, its rehearsal.
639+
#
640+
# ONE STEP, ONE `env:`, ONE SCRIPT, ON EVERY EVENT (#18341). The write and
641+
# its dry run used to be two steps behind mutually exclusive `if:`s
642+
# (`github.event_name != 'pull_request'` and `== 'pull_request'`), so no
643+
# pull request ever executed the script that writes. Its first execution
644+
# was scheduled run 34810389734: the dataset was computed, and the step
645+
# then died at `git commit` with `RUN_COUNT: unbound variable`, because
646+
# this step's `env:` exported three of the five variables its script
647+
# expands. Under `set -u` a key missing from `env:` blows up only on the
648+
# leg that runs, and the rehearsal ran the other leg.
649+
#
650+
# So a `pull_request` run now executes THIS script with THIS `env:`. The
651+
# branch, the `git add` and the commit — whose message expands every
652+
# provenance variable — happen for real on the runner, with the repo's own
653+
# commit hook, so a variable missing from the block below reds the pull
654+
# request that dropped it. Only the acts that leave the runner are
655+
# switched, and all of them are switched in ONE place: `outward`, which on
656+
# a dry run prints the command instead of running it. The shell expands a
657+
# command's arguments before `outward` is entered, so `set -u` judges every
658+
# argument on both legs alike.
659+
#
660+
# ⛔ Never call `git push`, `gh` or any other network act outside
661+
# `outward`, and never split this step back into an event-gated pair:
662+
# either one reopens a leg that no pull request can exercise.
663+
- name: Push the refresh branch and open the pull request (dry run on pull_request)
664+
if: steps.compare.outputs.changed == 'true'
638665
env:
666+
# The switch: 'true' on a `pull_request` run and only there. The script
667+
# refuses any other spelling, and refuses 'false' on a `pull_request`
668+
# run, so no edit to this line can make a pull request push.
669+
DRY_RUN: ${{ github.event_name == 'pull_request' }}
639670
GH_TOKEN: ${{ secrets.RELEASE_PUSH_TOKEN || github.token }}
671+
# Every variable the script expands that the runner does not provide.
672+
# The commit message is the dataset's provenance and names all four:
673+
# ⛔ never drop one from the message to get a green run.
640674
RUN_ID: ${{ steps.generate.outputs.run_id }}
675+
RUNS: ${{ steps.generate.outputs.runs }}
676+
RUN_COUNT: ${{ steps.generate.outputs.run_count }}
641677
HEAD_SHA: ${{ steps.generate.outputs.head_sha }}
642678
run: |
643679
set -euo pipefail
680+
681+
case "$DRY_RUN" in
682+
true) DRY_TAG='(dry run, not executed) ' ;;
683+
false) DRY_TAG='' ;;
684+
*)
685+
echo "::error::DRY_RUN must be 'true' or 'false', got '$DRY_RUN'. Nothing was pushed."
686+
exit 1
687+
;;
688+
esac
689+
if [ "$GITHUB_EVENT_NAME" = 'pull_request' ] && [ "$DRY_RUN" != 'true' ]; then
690+
echo "::error::DRY_RUN is '$DRY_RUN' on a pull_request run. A pull_request run of this lane never pushes a branch, opens a PR or writes a label. Nothing was pushed."
691+
exit 1
692+
fi
693+
694+
# outward [--stand-in TEXT] COMMAND...
695+
# The one dry-run switch. Live, it runs COMMAND. Dry, it prints COMMAND
696+
# to the log instead, and prints TEXT on stdout where the live call's
697+
# output would have been, so every line after it runs on a value of
698+
# the same shape.
699+
outward() {
700+
local stand_in=''
701+
if [ "$1" = '--stand-in' ]; then
702+
stand_in="$2"
703+
shift 2
704+
fi
705+
if [ "$DRY_RUN" = 'true' ]; then
706+
{ printf 'DRY RUN, not executed:'; printf ' %q' "$@"; printf '\n'; } >&2
707+
if [ -n "$stand_in" ]; then printf '%s\n' "$stand_in"; fi
708+
return 0
709+
fi
710+
"$@"
711+
}
712+
644713
BRANCH="claude/shard-timings-refresh-$RUN_ID"
645714
git config user.name 'github-actions[bot]'
646715
git config user.email '41898282+github-actions[bot]@users.noreply.github.com'
@@ -651,57 +720,57 @@ jobs:
651720
git commit \
652721
-m "chore(ci): refresh the Test Core shard-timings dataset" \
653722
-m "Regenerated by .github/workflows/shard-timings-refresh.yml from the test-core-run-summary artifacts of $RUN_COUNT accumulated run(s) ($RUNS), newest $RUN_ID at $HEAD_SHA. Generated, never hand-edited."
654-
git push origin "$BRANCH"
723+
outward git push origin "$BRANCH"
655724
656-
PR_URL=$(gh pr create --base main --head "$BRANCH" \
725+
PR_URL=$(outward --stand-in "https://github.com/$GITHUB_REPOSITORY/pull/0" \
726+
gh pr create --base main --head "$BRANCH" \
657727
--title "chore(ci): refresh the Test Core shard-timings dataset" \
658728
--body-file "$RUNNER_TEMP/pr-body.md")
659-
echo "Opened $PR_URL" | tee -a "$GITHUB_STEP_SUMMARY"
729+
echo "${DRY_TAG}Opened $PR_URL" | tee -a "$GITHUB_STEP_SUMMARY"
660730
661731
# ADDITIVE label write only. A whole-set PUT replaces the PR's labels
662732
# and destroys any that land in between — measured on this repo, one
663733
# second wide (see pr-automation.yml's header). POST names only what it
664734
# adds, so no interleaving can lose another writer's label.
665-
PR_NUMBER=$(gh pr view "$PR_URL" --json number --jq .number)
666-
gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \
735+
PR_NUMBER=$(outward --stand-in 0 gh pr view "$PR_URL" --json number --jq .number)
736+
outward gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \
667737
-f "labels[]=skip-changeset" > /dev/null
668738
669739
# Read back, because an additive write is necessary and not sufficient:
670740
# a concurrent whole-set PUT from another workflow can still strip the
671741
# label after a successful POST. `skip-changeset` is this PR's exemption
672742
# from the changeset gate — it publishes nothing — so losing it turns
673743
# the gate red on a PR that legitimately has no changeset.
674-
if gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \
744+
if outward --stand-in skip-changeset gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \
675745
| grep -qxF 'skip-changeset'; then
676-
echo "skip-changeset confirmed on PR #$PR_NUMBER."
746+
echo "${DRY_TAG}skip-changeset confirmed on PR #$PR_NUMBER."
677747
else
678748
echo "::warning::skip-changeset did not survive the write on PR #$PR_NUMBER (a concurrent whole-set label PUT strips it). Re-applying once."
679-
gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \
749+
outward gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" \
680750
-f "labels[]=skip-changeset" > /dev/null
681-
gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \
751+
outward gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/labels" --jq '.[].name' \
682752
| grep -qxF 'skip-changeset' \
683753
|| echo "::error::skip-changeset is still absent from PR #$PR_NUMBER; the changeset gate will demand a changeset this PR legitimately has none of. Apply the label by hand."
684754
fi
685755
686-
# The dry-run half of the `pull_request` posture. Everything above ran for
687-
# real; this renders the body that WOULD have been posted, so a reviewer of
688-
# a change to this lane sees the actual output rather than the diff of the
689-
# code that produces it.
690-
- name: Dry run — the pull request this would have opened
691-
if: steps.compare.outputs.changed == 'true' && github.event_name == 'pull_request'
692-
run: |
693-
{
694-
echo "### Shard timings: dry run (no branch pushed, no PR opened, no label written)"
695-
echo
696-
echo "This is a \`pull_request\` run of the refresh lane itself. The run selection, the"
697-
echo "artifact download, the regeneration, the coverage check and the partitioner's verdict"
698-
echo "all executed for real; only the write was skipped. The body below is what a scheduled"
699-
echo "run would have posted."
700-
echo
701-
echo "---"
702-
echo
703-
cat "$RUNNER_TEMP/pr-body.md"
704-
} >> "$GITHUB_STEP_SUMMARY"
756+
# The dry-run half of the `pull_request` posture: render the body that
757+
# WOULD have been posted, so a reviewer of a change to this lane sees the
758+
# actual output rather than the diff of the code that produces it.
759+
if [ "$DRY_RUN" = 'true' ]; then
760+
{
761+
echo "### Shard timings: dry run (no branch pushed, no PR opened, no label written)"
762+
echo
763+
echo "This is a \`pull_request\` run of the refresh lane itself. The run selection, the"
764+
echo "artifact download, the regeneration, the coverage check, the partitioner's verdict"
765+
echo "and the write step's own script, up to and including its local commit, all"
766+
echo "executed for real; only the push, the PR and the label write were skipped. The body"
767+
echo "below is what a scheduled run would have posted."
768+
echo
769+
echo "---"
770+
echo
771+
cat "$RUNNER_TEMP/pr-body.md"
772+
} >> "$GITHUB_STEP_SUMMARY"
773+
fi
705774
706775
- name: Say what happened when nothing changed
707776
if: steps.compare.outputs.changed == 'false'

0 commit comments

Comments
 (0)