ci: harden release workflow inputs and tag detection#2430
Open
dunglas wants to merge 5 commits into
Open
Conversation
Cross-port of dunglas/mercure#1246 review feedback: - Validate the version input inside release.yaml as well, so a UI-triggered dispatch can't slip a non-semver string through. - Replace the fragile "(HTTP 404)" stderr grep with a matching-refs lookup that returns an empty array for absent tags, and extend the split-state guard so a mismatched caddy/v<version> on the resume path is caught up-front instead of inside create_tag. - Build the release tree from `git diff --name-only HEAD` so anything the PGO refresh or `go mod tidy` step touches (beyond the previously hardcoded three paths) is captured. - Restore the three-way behind/ahead/diverged diagnostic in release.sh that the rewrite collapsed into a single equality check.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the release tooling by validating release inputs earlier, making tag-state detection more robust, and improving release-commit contents and operator diagnostics.
Changes:
- Adds workflow-side SemVer validation and keeps dispatches restricted to
main. - Replaces stderr-based tag-missing detection with GitHub matching-ref lookups and split-state checks.
- Builds release tree entries from all tracked modified files and restores clearer local/remote branch state diagnostics in
release.sh.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
release.sh |
Reports whether local main is behind, ahead of, or diverged from origin/main before dispatching. |
.github/workflows/release.yaml |
Validates inputs, improves tag lookup/resume logic, and includes all tracked modified files in the API-created release commit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Trim explanatory comments to one or two lines each and drop the operator-side semver regex from release.sh — release.yaml validates it.
This was referenced May 16, 2026
Cross-port of dunglas/mercure#1246 follow-up: - Use matching-refs in create_tag so transient API failures aren't read as "tag absent". - Build the release tree from `git status` partitions so additions, deletions, and modifications round-trip correctly. - Clarify the "no file changes" error with operator guidance. - Check that git is installed in release.sh.
dunglas
added a commit
that referenced
this pull request
May 16, 2026
## Root cause `runner-php-8-3-31-trixie` on https://github.com/php/frankenphp/actions/runs/25961840516/job/76318473755 (PR #2430, `linux/amd64`) failed with: > curl: (2) no URL specified > tar: Child returned status 1 Two converging issues: 1. **`.github/workflows/docker.yaml` did not export `GITHUB_TOKEN` to the bake-action environment.** `docker-bake.hcl` declares `secret = [\"id=github-token,env=GITHUB_TOKEN\"]`, but the Build step's `env:` block only set `SHA`, `VERSION`, `PHP_VERSION`, `BASE_FINGERPRINT`. The secret mount resolved to empty, the watcher install fell into the unauthenticated branch, and parallel matrix jobs trivially hit GitHub's 60 req/hr unauth cap. `static.yaml`'s bake step already passes `GITHUB_TOKEN`; `docker.yaml` was the outlier. 2. **The watcher install pipeline silently swallows API errors.** `curl -s` (no `-f`) hides HTTP errors; `grep tarball_url | awk | sed | xargs curl -L | tar xz` produces empty output when the API response is anything other than a normal release JSON (rate limit message, 5xx, auth fail). `xargs curl -L` then runs with no URL, and `tar xz` gets empty stdin — yielding the opaque pair above with no signal as to why. ## Fix - `docker.yaml`: pass `GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}` to the bake Build step's env block (mirrors `static.yaml`). - `Dockerfile` and `alpine.Dockerfile`: install `jq`, rewrite the watcher install with `curl -fsSL` plus `jq -r '.tarball_url // empty'` and an explicit empty-URL check that prints a useful error. Heredoc form for readability; logic kept POSIX so it works under both `bash` (Debian) and `ash` (Alpine). Cmake/build flags and final `ldconfig` step are unchanged.
dunglas
added a commit
that referenced
this pull request
May 16, 2026
## Root cause `TestHotReload` was failing on https://github.com/php/frankenphp/actions/runs/25961840516/job/76318473726 (`linux/arm/v7`, dispatched from #2430) with: > context deadline exceeded (Client.Timeout or context cancellation while reading body) `caddytest.NewTester` initialises `tester.Client.Timeout = 5s` (`Default.TestRequestTimeout`). That budget covers the entire roundtrip including body reads. On an emulated armv7 runner the chain `os.WriteFile` → e-dant/watcher event → `mercure` publish → SSE chunk delivery doesn't complete in 5 s, so the streaming `resp.Body.Read` is killed before the marker `index.php` is seen. The job died at exactly 5.03 s. ## Fix Set `tester.Client.Timeout = 0` for this test. The test already cancels the read via `context.WithCancel` once the marker appears in the buffer, and Go's `-timeout` provides the outer deadline — the 5 s inner cap was redundant. The other failing job on that run (`linux/amd64`, e-dant/watcher cmake build) is unrelated to this PR.
Cross-port of dunglas/mercure#1246 follow-up: - Verify the resumed commit actually contains the expected caddy/go.mod frankenphp pin and a non-trivial PGO profile before re-tagging or dispatching downstream builds. - Use --no-renames so renames decompose into add+delete and both halves land in the API tree mutation. - Preserve each file's existing mode (executable bit) when building tree entries instead of hardcoding 100644. - Scope the concurrency group per version so a pending environment approval doesn't block dispatches for a different version.
Cross-port of dunglas/mercure#1246 follow-up: - Match the frankenphp require entry in caddy/go.mod under both block and single-line forms so a future `go mod tidy` reformat doesn't fail the resume verification. - Detect a release-shaped main HEAD without tags as resumable, covering the case where a previous run pushed the commit but failed before create_tag. - Guard against main advancing between checkout and the commit step by asserting parent_sha equals the original checkout SHA before building the API tree. - release.sh: restore set -o errtrace so the ERR trap fires inside any future helpers, and fetch tags so the operator sees existing release tags before dispatching.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cross-port of review feedback from dunglas/mercure#1246, applied to the changes that this workflow shares structurally with Mercure's.
Summary
versioninput insiderelease.yaml. The semver regex inrelease.shonly protects operators using the script; a UI-triggered dispatch could otherwise propagatelatest,1.2, or any string containing/straight intogo get @v...,sed, and tag refs.(HTTP 404)stderr grep with agit/matching-refs/tags/...lookup that returns an empty array for absent tags. Distinguishing "tag missing" from rate-limit / 5xx / auth failures no longer depends on the exact wording ofgh's error messages. The split-state guard now also fires symmetrically: a mismatchedcaddy/v<version>on the resume path is caught before any writes.git diff --name-only HEADso the commit captures every file the PGO refresh orgo mod tidystep touches, rather than the previously hardcoded three paths.release.shso the operator knows whether togit pull,git push, or reconcile a divergence.