ci: fix the publishing workflow after its first real runs - #2397
Merged
Conversation
Preview publishing is broken on main for fork PRs, which is the case the whole split exists for. First real fork PR (#2391 from liangmiQwQ) failed with "no open PR of voidzero-dev/vite-plus has head c7e51be" while that PR was open with exactly that head. Cause: listPullRequestsAssociatedWithCommit returns EMPTY for a fork PR's head commit. Verified against the live API — it returns #2387 for a same-repo head and nothing for #2391's. So the lookup worked for every case I could test and failed for the only case that matters. `workflow_run.pull_requests` is empty for forks too, which is what sent me to the commit endpoint in the first place. I swapped one fork-blind source for another and could not have caught it before merge, since workflow_run cannot fire until the file is on the default branch. Now resolves via `pulls?state=open&head=<head_owner>:<head_branch>`, both GitHub-signed payload fields, so this is as trustworthy as the sha was. The head-sha match is a separate step so the failure says which of the two happened: no such PR, or the PR moved on since the build. Checked against the live API: #2391 resolves and is labeled, a stale head reports the PR and both shas, and an unknown branch reports no PR.
✅ Deploy Preview for viteplus-preview canceled.
|
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Collaborator
|
Thx ❤️ |
The Docker preview job now fails outright: #14 exporting to GitHub Actions Cache #14 ERROR: error writing layer blob: failed to reserve cache #13 exporting to image ... CANCELED ERROR: failed to build: failed to solve: error writing layer blob The cache export is fatal to the build, so it cancelled the image push. My optimization broke the job it was meant to speed up, and the npm preview for PR #2328 published fine while its Docker image did not. Reverting rather than fixing it. Making it work would need `actions: write` on the one job that installs and executes the preview package, which is the job SR-5 says to keep as unprivileged as possible, and this was the only `type=gha` usage in the repo so there was no working precedent to copy. The benefit was 60-90s of apt on a path that already waits on a human approval measured in minutes to days, so it was buying almost nothing. `ignore-error=true` would keep the build green but the export would keep failing, leaving a dead directive and a stack trace in every log.
fengmk2
changed the title
wan9chi
approved these changes
Aug 10, 2026
"Trusted leg" and "build leg" were my own coinage and mean nothing to a reader who was not in the design conversation. Replaced throughout with plain descriptions of what each file does: the build workflow and the publishing workflow. Where trust mattered I now state the property instead of encoding it in a name, so "TRUSTED LEG" became "It is the only place a bridge credential exists" and "BUILD LEG (untrusted)" became "It holds no secrets and no OIDC permission, so it is safe to run for a pull request from a fork". Also added a line the naming badly needed: publish-preview.yml is called "Publish preview build" and no longer publishes anything, so it now says so outright. Renaming it would be better but the publishing workflow matches it by NAME, so that has to be a coordinated change. Left the one pre-existing "arm64 QEMU leg", where leg means a matrix leg and is the normal term.
fengmk2
changed the title
fengmk2
added a commit
to voidzero-dev/pkg-pr-registry-bridge
that referenced
this pull request
Aug 10, 2026
Two related cleanups, both prompted by voidzero-dev/vite-plus#2397. ## "leg" was jargon I invented "Trusted leg" and "build leg" carry no meaning for a reader who was not in the design conversation. Replaced throughout the RFC, docs, action and worker comments with plain descriptions: **the build workflow** and **the publishing workflow**. Where trust was the point, the property is now stated rather than encoded in a name. Left one untouched: a pre-existing "arm64 QEMU leg", where leg means a matrix leg and is the normal term. ## SR-1 named an endpoint that does not work for forks This is the more important half. SR-1 said: > resolve the PR number from `head_sha` via `GET /repos/{repo}/commits/{head_sha}/pulls` Following that cost a production outage. `listPullRequestsAssociatedWithCommit` returns **empty** for a fork PR's head commit while working correctly for a same-repo one: | commit | result | | --- | --- | | same-repo PR head | returns the PR | | fork PR head | **empty** | So it passes every test reachable before the workflow is on the default branch, and fails for the only case this whole design exists for. `workflow_run.pull_requests` is fork-blind in the same way, which is precisely what makes the commit endpoint look like the alternative. SR-1 now says to resolve by **head repository and branch**, to check the head sha separately so the failure distinguishes "no such PR" from "the PR moved on", and records the general rule: > Anything keyed on a fork's commit is suspect; the PR number, the run id, and the head branch are all base-repo facts and are not. I kept the wrong version described rather than silently deleting it, because the next implementer will otherwise reach for the same endpoint for the same reason I did. `docs/ci-setup.md` gets the short form. The SR-1 anchor is renamed to match; all cross-references updated and verified to resolve. 206 tests pass, typecheck clean, no action-bundle drift (comments are stripped).
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
3 participants
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.
Two fixes to
main, both found by the first real runs of the publishing workflow. It could not be exercised before merge, becauseworkflow_runonly fires for workflow files already on the default branch.First, the good news: the design works. PR #2328 published end to end through the new path with an OIDC token, no admin token involved.
authorize,Pkg Preview, and the sticky comment all succeeded, andcommit.a7180fa85c06fad48is on the bridge:1. Fork PRs could not be resolved at all
#2391 (from
liangmiQwQ) failed inauthorizewithno open PR of voidzero-dev/vite-plus has head c7e51be…while that PR was open with exactly that head.listPullRequestsAssociatedWithCommitreturns empty for a fork PR's head commit. Confirmed against the live API:#2387So it worked for every case reachable before merge and failed for the only case this feature exists for.
workflow_run.pull_requestsis empty for forks too, which is what sent me to the commit endpoint originally — I swapped one fork-blind source for another.Now resolves via
pulls?state=open&head=<head_owner>:<head_branch>, both GitHub-signed payload fields. The head-sha match is a separate step so the message distinguishes "no such PR" from "the PR moved on":I re-checked the rest of the publishing workflow for the same blind spot. Everything else keys off the PR number or the run id, which are base-repo objects and fork-safe: the post-approval
pulls.getre-check returns correct state, head and labels for #2391, and the artifact download and thelistWorkflowRunArtifactsprecondition both see that run's 148MBbridge-packages.2. The Docker gha cache broke the image push
The cache export is fatal to the build, so it cancelled the push. I added this in the cleanup pass; it broke the job it was meant to speed up, and #2328's npm preview published while its Docker image did not.
Reverted rather than repaired. Making it work needs
actions: writeon the one job that installs and executes the preview package, which is the job SR-5 says to keep unprivileged, and this was the onlytype=ghausage in the repo so there was no working precedent. It was saving 60-90s of apt on a path that already waits on a human approval measured in minutes to days.3. Terminology
"Trusted leg" and "build leg" were my own coinage and meant nothing to a reader who was not in the design conversation. The two workflows are now described as the build workflow and the publishing workflow, and where trust was the point the property is stated rather than encoded in a name.
This also surfaced something worth fixing later:
publish-preview.ymlis named "Publish preview build" and no longer publishes anything. Renaming it is the real fix, but the publishing workflow matches it byname:, so that has to be a coordinated change. The header says so outright for now.The same terminology fix for the RFC and bridge docs is voidzero-dev/pkg-pr-registry-bridge#93, which also corrects SR-1 for the fork-blind endpoint above.
After merging
Re-label #2391 to get the first genuine fork preview.