feat: fork-PR comments via pull_request + workflow_run split (+ checkout@v7 docs hardening)#142
Merged
Merged
Conversation
…nt workflow for checkout@v7 Invert the PR-check recommendation: the default is now a single `ci check` step in the existing pull_request workflow (forks get the check but no comment), with pull_request_target demoted to an opt-in "Commenting on fork PRs" section. Apply the actions/checkout@v7 fork-checkout change (enforced now, backported to floating tags on July 16, 2026): bump checkouts to @v7 and add allow-unsafe-pr-checkout: true on the read-only ./pr checkout in both the recommended workflow and our own dogfooding bumpy-check.yaml. Without this the fork-comment workflow would start failing once the backport lands. - docs/github-actions.md: restructure + flag fix + 3rd security rule - docs/prereleases.md: generalize the now-non-default check trigger note - .github/workflows/bumpy-check.yaml: @v7 + flag on the fork job
theoephraim
force-pushed
the
claude/adoring-buck-35a83a
branch
from
99b86bd to
9a81e9b
Compare
Add a safer way to comment on fork PRs than the privileged pull_request_target workflow: run `ci check` in the normal (unprivileged) pull_request workflow, and post the comment from a separate workflow_run job whose privileged half never touches fork code or files. CLI: - `ci check --emit-comment <dir>` renders the comment to <dir>/comment.md (seeds an empty file so the artifact is always present) instead of only posting it. - new `ci comment --body-file <path>` posts a pre-rendered body. It resolves the target PR from the TRUSTED workflow_run event (head_sha → PR lookup), never from the (fork-influenced) artifact, and treats the body as data. Sets GH_REPO from GITHUB_REPOSITORY so it runs without a checkout. Docs: make the pull_request + workflow_run split the recommended fork-comment setup; demote the single-file pull_request_target workflow to an alternative. Dogfood: replace the pull_request_target bumpy-check.yaml (two jobs) with a single pull_request check that emits + uploads the artifact, plus a new bumpy-comment.yaml (workflow_run) that builds main's bumpy and posts. Tests: resolveTargetPrNumber derives the PR from the event head_sha (and ignores a hostile PR ref in the body); ci comment no-ops on a missing/empty body. Adds a minor bump for @varlock/bumpy.
|
The changes in this PR will be included in the next version bump.
|
theoephraim
changed the title
…split - cli.md: document ci check --emit-comment and add a ci comment section - github-actions.md: note the workflow_run default-branch gotcha, and tighten the untrusted-artifact wording (rendered from PR bump files / may execute fork code) and a stale 'next section' pointer
…p optional - github-actions.md: remove the entire 'Alternative: pull_request_target' subsection (workflow + security essentials + registry-redirect details + CodeQL notes). The pull_request + workflow_run split is now the only documented way to comment on fork PRs. - Lead the fork-comment section with a bold 'this is optional — only for fork PR comments' callout; drop the now-stale 'precautions below' phrasing from the PR check section. - cwd.ts: the legacy pull_request_target guardrail now steers users to the pull_request + workflow_run setup first, with --cwd kept as a stay-on-it escape hatch.
theoephraim
added a commit
that referenced
this pull request
Jun 26, 2026
…ollision The #142 squash-merge dropped this rename. On main, the "Bumpy Check" workflow's job is named `check`, which collides with the `check` job in ci.yaml — two workflows emit the same `check` status context, making the required-check match ambiguous. Rename it to `bumpy-check` so the bump-file check has its own distinct status context. The branch-protection ruleset should require `check` (CI tests) + `bumpy-check`, replacing the now-defunct `check-local` (the old pull_request_target job, removed in #142).
theoephraim
added a commit
that referenced
this pull request
Jun 26, 2026
…on) (#144) ## What Re-applies the one commit the [#142](#142) squash-merge dropped: renaming the **Bumpy Check** workflow's job from `check` to `bumpy-check`. ## Why On `main` right now, two PR workflows both emit a status context named `check`: - `CI / check` (the test suite, in `ci.yaml`) - `Bumpy Check / check` (the bump-file check, in `bumpy-check.yaml`) A required status check named `check` is therefore ambiguous. Renaming the bump-file job to `bumpy-check` gives it a distinct context. ##⚠️ Needs a branch-protection (ruleset) change too The "Protect main" ruleset currently requires **`check`** and **`check-local`**. Since #142 merged, `check-local` no longer exists (it was the old `pull_request_target` job) → it's now a **phantom required check that blocks every PR**, including this one. Required checks should become **`check`** (CI tests) + **`bumpy-check`** (this PR's renamed job). The `comment` job in `bumpy-comment.yaml` (the `workflow_run` poster) should **not** be required. This PR stays blocked until the ruleset drops `check-local`, so the ruleset edit and this merge go together.
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.
What
Reworks how bumpy gets a release-plan comment onto fork PRs. Two parts:
pull_requestcheck as the default; apply the July-16actions/checkout@v7change (allow-unsafe-pr-checkouton the read-only./prcheckout).pull_request+workflow_runsplit where the privileged half never touches fork code or files — added after we audited the residual risks of the single-filepull_request_targetapproach.Why the split
A fork's
pull_requesttoken is read-only at issuance (enforced server-side — REST/GraphQL/gh/curlall 403, no secrets), so the comment must be posted from a privileged base-repo run. Theworkflow_runsplit is the only design where the privileged job has no untrusted checkout, no package manager run against fork config, and reads no fork file — it just posts pre-rendered text. That removes theallow-unsafe-pr-checkoutfragility, the symlink read-surface, the registry-redirect surface, and the CodeQL dismissal that thepull_request_targetworkflow carries.CLI changes (
@varlock/bumpy, minor)ci check --emit-comment <dir>— renders the comment to<dir>/comment.md(seeds an empty file so the artifact is always present) instead of only posting it. Still posts directly when it has a write token (same-repo PRs).ci comment --body-file <path>— posts a pre-rendered body. Security-critical: it resolves the target PR from the trustedworkflow_runevent (head_sha→ PR lookup), never from the artifact (which is fork-influenced), and treats the body as data. SetsGH_REPOfromGITHUB_REPOSITORYso it runs without a checkout. No-ops on a missing/empty body.User-facing setup
The fork-comment setup is optional — only needed to show the comment on PRs from forks (the check itself already runs red/green on forks). Add
--emit-comment+upload-artifactto your existingpull_requestCI, and drop in a smallworkflow_runposter. Docs:Commenting on fork PRs. Thepull_request_targetapproach has been removed from the docs — the split is the only documented fork-comment path.Dogfood
Replaced our
pull_request_targetbumpy-check.yaml(two jobs) with a singlepull_requestcheck that emits + uploads, plus a newbumpy-comment.yaml(workflow_run) that builds main's bumpy and posts.Tests / verification
test/core/ci-comment.test.ts:resolveTargetPrNumberderives the PR from the eventhead_sha(and ignores a hostile#999in the body);ci commentno-ops on missing/empty body. Full suite: 389 pass, typecheck + lint + fmt clean.--emit-comment(writes a real renderedcomment.md) andci comment(no-op + error paths) end-to-end.On this PR, the new dogfood
bumpy-comment.yaml(which builds main's bumpy) will fail, becauseci commentdoesn't exist onmainuntil this merges. That's cosmetic — it's the poster workflow, not the PR check, and this PR's own comment still posts directly frombumpy-check.yaml(same-repo write token). It goes green automatically once merged.