ci: route refreshed visual baselines through a PR instead of a direct push

[bpamiri]

Closes #3283

Summary

Dispatching Actions → Refresh visual baselines completes the expensive rebuild + screenshot + --update work, then dies at the final step: the workflow pushes the refreshed PNG(s) straight back to the dispatched branch (git push origin "HEAD:$BRANCH"), which the develop branch ruleset now rejects with GH013: Changes must be made through a pull request.

This converts the final step to the PR + auto-merge pattern already proven in the sibling refresh-packages-baseline.yml:

• .github/workflows/refresh-visual-baselines.yml — the "Commit and push refreshed baselines" step becomes "Open refresh PR and enable auto-merge": it now invokes the new helper instead of pushing directly, and the job permissions: block gains pull-requests: write (a contents: write-only token fails the gh pr calls silently). Doc comments and the step summary updated to describe the PR flow.
• tools/gh-open-refresh-baseline-pr.sh (new) — commits the refreshed baseline(s) onto a throwaway chore/refresh-baseline-<sites>-<run_id> branch, gh pr create against the dispatched branch with a conventional, commitlint-safe title (chore(web): refresh visual baseline(s) (<sites>), ≤100 chars), then gh pr merge --auto --squash --delete-branch. If repo auto-merge is disabled it logs and leaves the PR open for a manual merge instead of failing the job. All inputs arrive via env (never spliced into a shell string), preserving the workflow's Actions-injection-safe posture. Passes shellcheck.
• vendor/wheels/tests/specs/cli/RefreshVisualBaselinesPrPushSpec.cfc (new) — static-content regression spec (same pattern as BotResolveConflictsLoopSafeSpec) pinning: no git push … HEAD:<target> in the workflow or helper, pull-requests: write present, gh pr create + gh pr merge --auto present, and the workflow actually invoking the helper.
• changelog.d/3283-refresh-baselines-pr-push.fixed.md — changelog fragment.

This applies the TDD-verified patch the wheels-bot held in its comment on #3283 — the bot's GitHub App lacks the workflows permission required to push changes to workflow files, so it could not open this PR itself. Pushed here with human credentials.

Test plan / status

TDD red → green, verified against a Lucee 7 + SQLite server serving this exact working tree:

• Red (spec applied, fix not yet applied): wheels.tests.specs.cli.RefreshVisualBaselinesPrPushSpec → 0 pass / 4 fail / 0 error, each failure citing the intended invariant (direct push present, missing pull-requests: write, no PR flow, missing helper).
• Green (fix applied): same bundle → 4 pass / 0 fail / 0 error.
• Full cli area (directory=wheels.tests.specs.cli): 21 bundles, 131 pass, 0 fail, 0 error — no regressions.
• Workflow YAML parses (ruby psych); helper passes shellcheck and bash -n.

Extra-scrutiny areas (sensitive: CI workflow change)

• Touches .github/workflows/ — the changed step only runs on manual workflow_dispatch, and the job token gains pull-requests: write (scoped to this job only; contents: write is unchanged).
• The helper enables auto-merge on the baseline-refresh PRs it opens (mirroring refresh-packages-baseline.yml); required checks on the target branch still gate the actual merge. Repo auto-merge is currently known to toggle off after some admin PATCH operations — the helper degrades gracefully to leaving the PR open.
• Not exercisable end-to-end except via a real workflow_dispatch run; suggest dispatching Refresh visual baselines (sites=guides) once merged to confirm the PR flow (the runner's gh/GITHUB_TOKEN path can only be proven live).

Cross-engine notes

No framework runtime code changed. The only CFML added is a static file-content spec (string/regex/fileRead only, ## correctly escaped in strings) — no closures-as-named-args, no reserved-scope names, no tag wrappers. Compiles and runs green on Lucee 7; nothing engine-specific in the spec surface.

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer

TL;DR — This PR fixes #3283: the Refresh visual baselines workflow used to git push origin "HEAD:$BRANCH" at the end, which the develop ruleset now rejects with GH013: Changes must be made through a pull request. It re-routes the refresh through a throwaway chore/refresh-baseline-* branch → gh pr create → gh pr merge --auto, extracted into tools/gh-open-refresh-baseline-pr.sh, grants the job pull-requests: write, and pins the invariant with a static regression spec. The change is well-scoped, TDD-verified (red 4-fail → green 4-pass), and mirrors the proven refresh-packages-baseline.yml pattern. Verdict: comment — no blocking issue; one operational caveat on the auto-merge step worth a maintainer's eyes before relying on it landing unattended.

Correctness

• tools/gh-open-refresh-baseline-pr.sh:96 — auto-merge may be enabled but never fire, because the refresh PR's required checks won't run. The PR is opened with GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}, and GitHub's infinite-loop guard suppresses workflow triggers on GITHUB_TOKEN-authored PRs. The sibling workflow this PR mirrors documents exactly this and deliberately does not auto-merge because of it:

  .github/workflows/refresh-packages-baseline.yml: "CI gotcha: PRs opened with GITHUB_TOKEN don't trigger other workflow runs (GitHub's infinite-loop guard). Reviewers should eyeball the single-PNG diff."

  visual-regression lives in web-deploy.yml, which fires on pull_request → develop for web/** paths — the refresh PR touches web/tests/visual-baselines/*.png, so it matches the path filter but the run is suppressed by the token. If the target branch's protection requires that check, gh pr merge "$pr_url" --auto enables auto-merge but the PR then waits forever on a check that never runs. The helper's if gh pr merge … ; then … else … only catches the case where the command itself errors (auto-merge disabled repo-wide), not the enabled-but-stuck case — so the "left open for manual merge" log line won't print, and a human has to notice the wedged PR on their own.

  This is non-blocking: the primary bug (the GH013 crash) is genuinely fixed, and the worst case degrades to the same manual-merge fallback the PR body already anticipates. But the stated goal ("auto-merges once the required checks pass … lands unattended") likely won't hold when the target branch has required checks. Consider either (a) documenting in the helper/step summary that a maintainer must merge manually if the refresh PR's checks don't auto-run under GITHUB_TOKEN, or (b) matching the sibling and dropping --auto entirely, given both share the token limitation. At minimum, verify against a target branch with the real required-check set during the suggested live workflow_dispatch run.

Cross-engine

No framework runtime code changed. The one CFML file added (RefreshVisualBaselinesPrPushSpec.cfc) is a static-content spec — fileRead + reFindNoCase only, ##3283 correctly escaped in string literals (## → literal #), // comments left un-escaped as allowed. No closures-as-named-args, no reserved-scope names, no tag wrappers. Extends wheels.WheelsTest with BDD describe/it. Clean.

Tests

vendor/wheels/tests/specs/cli/RefreshVisualBaselinesPrPushSpec.cfc pins the four invariants that matter — no residual git push … HEAD:<target> in either the workflow or the helper, pull-requests: write present, gh pr create + gh pr merge --auto present, and the workflow actually invoking the helper. Genuine TDD (red 4-fail → green 4-pass reported), and the spec asserts on the fix's observable contract rather than incidental text. Good coverage for a step whose live side effects (git + gh) can't be unit-exercised.

Commits

ci: route refreshed visual baselines through a PR instead of a direct push — valid type (ci), no scope, subject not ALL-CAPS, ≤100 chars. Conforms to commitlint.config.js.

Docs

• Changelog fragment changelog.d/3283-refresh-baselines-pr-push.fixed.md uses a valid type (fixed) and complete bullet text — correct per the fragment system (no direct CHANGELOG.md edit). Minor: the commit itself is ci: (internal workflow plumbing, not user-facing), so a changelog fragment is arguably optional here — but erring toward documenting is harmless, not a defect.
• Workflow header comments and the step summary are updated to describe the new PR flow accurately. Nice.

[bpamiri]

Addressed the reviewer's auto-merge caveat (and a few adjacent findings) in bba7eed:

• Auto-merge dropped — the reviewer (and refresh-packages-baseline.yml's header) is right: the fallback PR is authored by GITHUB_TOKEN, whose PRs never fire pull_request workflows, so the required checks sit Expected forever and gh pr merge --auto would wedge silently. The fallback PR is now deliberately left for a maintainer, exactly like the sibling workflow.

• Direct push restored as a guarded fast path — the workflow header's documented flow is dispatching on the PR's source branch, which still accepts direct pushes. The helper now commits once and tries git push origin HEAD:$TARGET_BRANCH first (identical to pre-refresh-visual-baselines workflow direct-push is rejected by the develop ruleset #3283 behavior); only a ruleset rejection (GH013 on develop) falls through to the PR flow. No extra PR round-trip for the primary use case.

• Honest messaging — the helper reports delivery=push|pr + pr_url via GITHUB_OUTPUT, and the step summary now says plainly that the fallback PR cannot land unattended: close/reopen it to trigger CI, or eyeball the PNG diff and merge.

• Re-run safety — throwaway branch name now includes RUN_ATTEMPT, so re-running a failed job (same run_id) can't collide with attempt 1's leftover branch.

• Spec hardened — comment lines are stripped before matching so only executable gh pr create satisfies it; every direct push in the helper must be if-guarded; gh pr merge --auto is now forbidden (pinning the wedge); RUN_ATTEMPT suffix pinned. Verified red against both the original bug and the previous iteration of this PR; 5/5 green on Lucee 7 + SQLite, cli area 131 pass.

• Before relying on this on a real baseline refresh: one live workflow_dispatch run against a feature branch (expect direct push) and one against develop (expect fallback PR + summary instructions).

[wheels-bot]

Wheels Bot — Reviewer

TL;DR — This PR fixes #3283: the Refresh visual baselines workflow used to git push origin "HEAD:$BRANCH" as its final step, which the develop ruleset rejects with GH013: Changes must be made through a pull request, wasting the whole rebuild+screenshot run. Delivery now lives in tools/gh-open-refresh-baseline-pr.sh: the direct push is kept as an if-guarded fast path (unprotected feature branches, the documented dispatch flow), and a ruleset rejection falls through to a throwaway chore/refresh-baseline-* branch + gh pr create left for a maintainer. The job gains pull-requests: write, and a static regression spec pins the invariants. This head (bba7eed4) also resolves my prior review's only concern by dropping the inert gh pr merge --auto step. The change is well-scoped, TDD-verified, injection-safe, and mirrors the proven refresh-packages-baseline.yml pattern. Verdict: approve.

Resolved from prior review (465d4406)

• Auto-merge that could never fire — fixed. My earlier comment flagged that gh pr merge … --auto on a GITHUB_TOKEN-authored PR would wedge, since GitHub suppresses pull_request triggers on such PRs so required checks never run. The commit ci: keep guarded direct baseline push, drop inert auto-merge on the PR fallback removes it entirely — verified: a grep for gh pr merge / --auto over tools/gh-open-refresh-baseline-pr.sh and .github/workflows/refresh-visual-baselines.yml returns nothing, and the spec now actively pins its absence (RefreshVisualBaselinesPrPushSpec.cfc:120 asserts the combined helper+workflow contains no gh pr merge … --auto). The fallback PR is now correctly left for a maintainer, matching the sibling workflow.

Correctness

Clean. The fast path if git push origin "HEAD:${TARGET_BRANCH}"; then … exit 0; fi (gh-open-refresh-baseline-pr.sh:100) is inside an if condition, so under set -euo pipefail a rejection does not trip errexit — it falls through to the PR path as intended. RUN_ATTEMPT is folded into the throwaway branch name (chore/refresh-baseline-${SITES}-${RUN_ID}-${RUN_ATTEMPT}) so a re-run of a failed job does not collide non-fast-forward with the branch left by attempt 1 — a real edge case, correctly handled and spec-pinned. The step-summary branches (push / pr / changed-but-undelivered / no-drift) cover all delivery output states.

Security

No injection surface. All inputs arrive via env and reach git/gh as single argv elements (--title, --body-file, never a shell-expanded command string); SITES is constrained to the workflow choice list and RUN_ID/RUN_ATTEMPT are numeric. The one word-split (git add -- $ADD_PATHS, gh-open-refresh-baseline-pr.sh:74) is intentional, shellcheck-annotated, and workflow-controlled. pull-requests: write is scoped to this single job; contents: write is unchanged.

Cross-engine

No framework runtime code changed. The one CFML file (RefreshVisualBaselinesPrPushSpec.cfc) is a static-content spec — fileRead + reFindNoCase/reFind only, issue number correctly escaped in string literals, the stripCommentLines closure assigned to a var before use (not passed as a constructor named arg). Extends wheels.WheelsTest with BDD describe/it. Reported green on Lucee 7 + SQLite. Clean.

Tests

vendor/wheels/tests/specs/cli/RefreshVisualBaselinesPrPushSpec.cfc follows the established static-check prior art (BotResolveConflictsLoopSafeSpec, expandPath("/wheels/../..")) and pins the invariants that matter: no git push in the workflow, the workflow invokes the helper, pull-requests: write present, every helper git push … HEAD: is if-guarded, an executable gh pr create exists, no auto-merge, and the branch name carries both RUN_ID and RUN_ATTEMPT. It strips comment lines first so a comment mentioning gh pr create cannot satisfy the assertions — the right rigor for a step whose live git/gh side effects cannot be unit-exercised. Genuine red→green TDD reported.

Commits

Both conform to commitlint.config.js: valid ci type, no scope, subjects not ALL-CAPS and ≤100 chars, messages explain the "why."

Docs (non-blocking nits)

• Stale PR description. The Summary and Extra-scrutiny sections still describe gh pr merge --auto --squash --delete-branch and "The helper enables auto-merge on the baseline-refresh PRs it opens" — behavior removed in the final commit. The shipped helper opens the PR and leaves it for a maintainer with no auto-merge (and the spec now forbids auto-merge). Worth updating the description so a maintainer reading it is not misled; no code impact.
• ci: change with a changelog fragment. changelog.d/3283-refresh-baselines-pr-push.fixed.md is well-formed (fixed type, complete bullet), but this is internal workflow plumbing dispatched only by maintainers, so a user-facing fragment is arguably optional here. Harmless — erring toward documenting is fine.