Run the docker workflow after release - #104

@claude review

Claude finished @carlos-granados's task in 1m 47s —— View job

Code Review — PR #104

• Load PR context
• Read .github/workflows/release.yml diff
• Cross-check against .github/workflows/docker.yml
• Post review feedback

Overall this is a well-designed change with excellent inline documentation. The comment on the new step is genuinely load-bearing — it explains why an explicit gh workflow run is needed rather than relying on release: published, and mirrors the same reasoning already established in prepare-release.yml → release.yml. Below are the substantive findings, ordered by importance.

1. Manual workflow_dispatch for an older tag will publish Docker images for the wrong release (functional, non-obvious)

release.yml already supports workflow_dispatch with an arbitrary tag input (lines 14–18), so a maintainer can re-release, say, 0.1.5 while 0.2.3 is the current latest. In that case the new step dispatches docker.yml with only --ref main -f target=prod — no tag is passed through.

docker.yml (lines 91–110) then resolves what to build like this:

REF="$EVENT_TAG"                                  # empty on workflow_dispatch
if [ -z "$REF" ]; then
    REF="$(gh release view ... --jq .tagName)"    # falls back to "latest published"
fi

So a re-release of 0.1.5 will silently trigger a Docker build of 0.2.3 (or whatever gh release view returns as latest) instead of the tag that was actually just released.

The push: tags path is unaffected — that path always releases the newest tag anyway. This only bites the workflow_dispatch path.

Two ways to address:

• Cheapest: add a note to the comment that manual dispatches for non-latest tags won't publish matching Docker images (accept the limitation).
• Cleanest: add a ref input to docker.yml's workflow_dispatch and pass -f ref=${{ env.RELEASE_REF }} here, then have the resolve job prefer that input.

Fix this →

2. Release job is marked failed even though the GitHub release itself succeeded (operational)

The retry loop exit 1s on failure, but by that point softprops/action-gh-release@v3 above has already published the release. Result: the release exists and is visible, but the workflow shows a red X, which is easy to misread as "the release failed — needs rerunning".

The current error message does say "rerun it manually from the Actions UI with target=prod" which mitigates this — but consider changing the failure to a warning + ::notice:: (or continue-on-error: true on the step) so the release job stays green and there's just a Docker-specific followup. That matches the actual state: the release shipped, only the downstream mirror didn't.

Not a blocker — depends on whether you prefer noisy failure (forces attention) or accurate status.

3. Nit — token choice

secrets.GITHUB_TOKEN is the right call here since permissions: actions: write is set at the workflow level; no PAT needed. Just calling this out because it's easy to over-think and reach for a PAT.

4. Nit — retry sleep with set -e

[ "$attempt" -lt 3 ] && sleep 5

GitHub Actions runs the default shell with set -eo pipefail. This particular idiom is safe because the failing [ is on the left of &&, which is explicitly excepted from set -e in bash. Just noting so a future reader doesn't rewrite it — it's fine as-is.

Approval summary

Change is correct, comments explain the non-obvious pieces, and it fills a real gap (the anti-recursion trap around GITHUB_TOKEN → release: published). Finding #1 is the only one I'd argue for addressing before merge, and even that can be deferred if workflow_dispatch re-releases for old tags aren't a real workflow you use.
• fix/run-docker-workflow-after-release

@claude updated, review again

Claude finished @carlos-granados's task in 59s —— View job

Re-review — PR #104

• Load latest PR context
• Read updated .github/workflows/release.yml and docker.yml
• Verify prior findings addressed
• Post review feedback

Both substantive findings from the first pass are addressed. Summary:

Finding #1 (manual dispatch for older tag) — resolved via docs

Rather than plumbing a ref input through docker.yml, the comment now explicitly calls the limitation out and directs the reader to dispatch docker.yml by hand for the older-tag re-release case:

re-releasing an older tag via this workflow's own workflow_dispatch (e.g. rebuilding 0.1.5 while 0.2.3 is latest) will publish Docker images for whatever is currently latest, not the tag just released. Fine for the normal case (releasing the newest tag) — if you ever need to re-release an older tag's Docker images specifically, dispatch docker.yml by hand instead.

That's the cheapest of the two options I suggested and a reasonable call — the extra plumbing wasn't worth it for a rare, always-manual path that has an obvious escape hatch. Good.

Finding #2 (failed step marks release as failed) — fixed

The retry loop no longer exit 1s on exhaustion; it emits ::warning:: and falls off the end (exit 0). The message is precise about what actually failed:

echo "::warning::Failed to dispatch docker.yml after 3 attempts — the release itself published fine, but rerun docker.yml manually from the Actions UI with target=prod to publish matching Docker images"

release.yml:844-847. Job stays green, the actual state (release published, mirror didn't kick off) is conveyed via the warning annotation, and the rerun instruction is right there. Matches what I'd hoped for.

Other verified bits

• permissions: actions: write is added at workflow level (release.yml:22) — required for gh workflow run. GITHUB_TOKEN is sufficient; no PAT.
• if: github.repository == 'php-debugger/php-debugger' keeps forks from attempting a dispatch they can't complete. Consistent with the fork-gating pattern already in docker.yml.
• --ref main is correct — docker.yml's prod guard (docker.yml:86) requires refs/heads/main anyway, so dispatching from anywhere else would just fail the guard.
• The [ "$attempt" -lt 3 ] && sleep 5 idiom is still fine under set -e (failing [ is on the left of &&, which is excepted).

Approval

LGTM — ship it. The comment block is exceptionally load-bearing (anti-recursion, target=prod, latest-vs-specific-tag, warn-don't-fail rationale) and will save the next reader from re-deriving all of this from GitHub Actions docs.
• fix/run-docker-workflow-after-release

@claude did some changes after copilot review, do a new review

Claude finished @carlos-granados's task in 1m 22s —— View job

Re-review — PR #104 (post-Copilot)

• Load latest PR state and Copilot review context
• Read updated .github/workflows/release.yml
• Verify responses to Copilot findings
• Post review feedback

Only one substantive change since my last approval: workflow-level permissions were tightened to least-privilege in response to Copilot's first comment. Verified in df31602.

Copilot Finding #1 (workflow-level actions: write too broad) — addressed cleanly

release.yml:26-27 is now permissions: contents: read at the workflow level, and release.yml:793-795 grants contents: write + actions: write only on the release job — the only job that publishes and dispatches. The build matrix jobs (validate/build-macos/build-linux/build-windows/build-php-*) now run with contents: read and no actions: permission at all, which is the least-privilege shape Copilot asked for.

Comment block at release.yml:21-25 documents the split clearly — future readers won't need to re-derive why the write perms are on the job rather than the workflow.

Copilot Finding #2 (build jobs need actions: write for upload-artifact) — correctly rejected

Your rebuttal is right. actions/upload-artifact@v6 and actions/download-artifact@v6 for same-run artifacts authenticate via the runner-injected ACTIONS_RUNTIME_TOKEN, which is independent of the permissions: block. The actions: permission only gates GITHUB_TOKEN-authenticated cross-run/cross-repo artifact access (e.g. dawidd6/action-download-artifact pulling from a prior run). Since every upload/download in this file is within the same run, no actions: permission is needed on the matrix jobs.

Repo history corroborates: pre-PR release.yml had only contents: write (no actions: at all) and the 0.1.0 / 0.2.0 matrix uploads worked. Copilot's suggestion would have added dead permissions.

Copilot's earlier tag-passthrough suggestion — already declined, still fine

Matches option A from my first pass. Documented in the load-bearing comment at release.yml:836-842. If a maintainer ever needs to re-release an older tag's Docker images, gh workflow run docker.yml -f target=prod from the CLI is the obvious escape hatch and it's called out inline.

One trivial nit (non-blocking)

release.yml:19-20 — there are two consecutive blank lines between the end of the on: block and the permissions comment. Cosmetic only; leave it or drop one, doesn't matter.

Approval

LGTM — ship it. Least-privilege change is correct, and the Copilot findings that would have introduced dead perms or cross-run auth confusion were correctly declined with sound reasoning.
• fix/run-docker-workflow-after-release