docs: drop the "leg" metaphor, correct SR-1's PR lookup - #93
Merged
Conversation
Two related cleanups.
"Trusted leg" and "build leg" were my own coinage and mean nothing to 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 I now state the
property rather than encode it in a name.
SR-1 told the implementer to resolve the PR from `head_sha` via
`GET /repos/{repo}/commits/{head_sha}/pulls`. That is wrong, and following it
cost a production outage: the endpoint returns EMPTY for a fork PR's head
commit while working correctly for a same-repo one, so it passes every test
reachable before the workflow reaches the default branch and fails for the
only case this design exists for. `workflow_run.pull_requests` is fork-blind
the same way, which is exactly 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,
while the PR number, the run id and the head branch are base-repo facts and
are not. Kept the wrong version described rather than deleted, since the next
implementer will otherwise reach for the same endpoint. docs/ci-setup.md gets
the short form.
Renamed the SR-1 anchor to match; all cross-references updated and checked.
fengmk2
added a commit
to voidzero-dev/vite-plus
that referenced
this pull request
Aug 10, 2026
Two fixes to `main`, both found by the first real runs of the publishing workflow. It could not be exercised before merge, because `workflow_run` only 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, and `commit.a7180fa85c06fad48` is on the bridge: ``` commit.a7180fa85c06fad48 | pr: .../pull/2328 | at: 2026-08-10T07:35:31.617Z ``` ## 1. Fork PRs could not be resolved at all #2391 (from `liangmiQwQ`) failed in `authorize` with `no open PR of voidzero-dev/vite-plus has head c7e51be…` while that PR was open with exactly that head. `listPullRequestsAssociatedWithCommit` returns **empty** for a fork PR's head commit. Confirmed against the live API: | commit | result | | --- | --- | | #2387 head (same-repo) | returns `#2387` | | #2391 head (fork) | **empty** | So it worked for every case reachable before merge and failed for the only case this feature exists for. `workflow_run.pull_requests` is 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": ``` fork PR 2391 (real failure) -> OK: #2391 labeled=true fork=true stale head -> FAIL: PR #2391 now at c7e51be, built 0000000 no such branch -> FAIL: no open PR from liangmiQwQ:does-not-exist ``` 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.get` re-check returns correct state, head and labels for #2391, and the artifact download and the `listWorkflowRunArtifacts` precondition both see that run's 148MB `bridge-packages`. ## 2. The Docker gha cache broke the image push ``` #14 exporting to GitHub Actions Cache #14 ERROR: error writing layer blob: failed to reserve cache #13 exporting to image ... CANCELED ``` 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: write` on the one job that installs and executes the preview package, which is the job SR-5 says to keep unprivileged, and this was the only `type=gha` usage 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.yml` is named "Publish preview build" and no longer publishes anything. Renaming it is the real fix, but the publishing workflow matches it by `name:`, 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.
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
1 participant
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 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:
Following that cost a production outage.
listPullRequestsAssociatedWithCommitreturns empty for a fork PR's head commit while working correctly for a same-repo one: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_requestsis 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:
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.mdgets 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).