Repository navigation
build: wait 2 days before using a new PHP build - #309
Conversation
The update-php workflow picked up cli-php-builds releases within a day of an upstream release (e.g. zlib 1.4). It now uses the latest release published at least COOLDOWN_DAYS days ago (default 2), so that problems with new upstream releases can be found first. Manual runs of the workflow can set cooldown_days to 0 to skip it, e.g. for a security fix. A pinned release newer than the cooldown allows (from a manual update) is treated as up to date, instead of as an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnUBGwz87gguQW2fBMdvwS
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 1 warning · 🔵 2 minor points
🔍 Full review · 3 files reviewed
Verification
- On scheduled runs
inputs.cooldown_daysis empty, so${COOLDOWN_DAYS:-2}falls back to the default of 2 days. - The
^[0-9]+$check rejects decimal or negative values from the number input before they reachjq --argjson. - gh's
publishedAtvalues use theYYYY-MM-DDTHH:MM:SSZform, which jq'sfromdateiso8601accepts, and the comparison withnow - days*86400is correct. - CLAUDE.md now gives the new-minor-version steps as
COOLDOWN_DAYS=0 scripts/update-php.sh, so a manual run is not blocked when the build was published that day.
Nothing tests the script or the workflow: no test file is added, and no CI job or Makefile target runs scripts/update-php.sh. The cooldown logic is only exercised when the scheduled update-php workflow runs.
Review details
- Commit: 53bcd6f
- Model: claude-opus-5-5
Review 1 of 10 for this pull request · View the full run
CLI buildsBuilt from c7ef17e (merged into the base branch). Downloads need a GitHub login, and expire after 7 days.
Or download one with the GitHub CLI, e.g.: gh run download 42723759821 -R upsun/cli -n cli-linux-amd64
chmod +x upsun
./upsun --versionOn macOS, if you downloaded it with a browser, remove the quarantine attribute before running it: |
- A scheduled run no longer replaces a newer build on the update branch (e.g. from a run that skipped the cooldown for a security fix) with an older one: the workflow compares the PHP version and build, not the commit title. - A pinned release that is no longer listed (deleted, or marked as a pre-release) is an error again, rather than reported as up to date. - Failures of gh or jq are no longer hidden by `|| true`, which was meant only for grep finding no match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PnUBGwz87gguQW2fBMdvwS
There was a problem hiding this comment.
Note
Reviewed — No blocking findings · 🔵 1 minor point
🔍 Full review · 3 files reviewed
🔵 Minor point
.github/workflows/update-php.yml:61— The 'Check the existing branch' step skips the push whenever theautomated/update-phpbranch's Makefile pins a build thatsort -Vranks the same as the new one or higher. It does not check whether a PR is still open for that branch. Example: build 5 was pushed withcooldown_days: 0, turned out to be bad, its PR was closed without merging and its release deleted, while main still pins build 3. When build 4 passes the cooldown, the script proposes it, but this check sees that the branch pins build 5 and setsup_to_date. No PR is opened until a build above 5 is published. The old title comparison would have pushed build 4.
Verification
releases=$(gh ...)and thejqassignments run underset -e, so a failed command stops the script; only thegrepis wrapped in|| true.- When the pinned tag sorts newer than the newest release past the cooldown, the script exits 0 only if that tag is in
all_tags, and otherwise exits 1. sort -Vorders version-build strings like8.4.5-10after8.4.5-9, and8.4.10-1after8.4.9-1, so the branch comparison ranks builds correctly.- On scheduled runs,
inputs.cooldown_daysis empty, so${COOLDOWN_DAYS:-2}uses the default of 2.
Nothing in the diff tests scripts/update-php.sh or the workflow. They only run when update-php runs on its schedule or by hand (workflow_dispatch).
Review details
- Commit: c7ef17e
- Model: claude-opus-5-5
Review 2 of 10 for this pull request · View the full run
|
On the minor point about a closed PR's branch ( 🤖 Addressed by Claude Code Generated by Claude Code |
The update-php workflow picked up cli-php-builds releases within a day of
an upstream release (e.g. zlib 1.4). It now uses the latest release
published at least COOLDOWN_DAYS days ago (default 2), so that problems
with new upstream releases can be found first. Manual runs of the workflow
can set cooldown_days to 0 to skip it, e.g. for a security fix.
A pinned release newer than the cooldown allows (from a manual update) is
treated as up to date, instead of as an error.
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01PnUBGwz87gguQW2fBMdvwS