Skip to content

ci: post type-diff comment via workflow_run so fork PRs work - #409

Merged
TooTallNate merged 2 commits into
mainfrom
type-diff-fork-prs
Jul 6, 2026
Merged

ci: post type-diff comment via workflow_run so fork PRs work#409
TooTallNate merged 2 commits into
mainfrom
type-diff-fork-prs

Conversation

@TooTallNate

Copy link
Copy Markdown
Owner

Problem

The Runtime Type Diff check fails on every fork-originated PR (seen on #404, #405) with:

RequestError [HttpError]: Resource not accessible by integration
POST /repos/TooTallNate/nx.js/issues/<n>/comments → 403

The type build + diff itself succeeds — only the sticky-comment posting fails. On the pull_request event, fork PRs get a read-only GITHUB_TOKEN; the workflow-level permissions: pull-requests: write cannot elevate past that cap.

Fix

Standard two-stage workflow_run pattern:

  • type-diff.yml (pull_request, contents: read only): builds base + PR types, diffs, writes the diff to the job summary, uploads it as the type-diff artifact. No API writes, so it can never 403.
  • type-diff-comment.yml (new, workflow_run on "Type Diff" completion, pull-requests: write + actions: read): downloads the artifact from the triggering run and posts/updates the sticky comment. Works for fork PRs because workflow_run workflows execute from the default branch with a base-repo token.

Security notes

The artifact is produced by a workflow that built untrusted fork code, so the comment workflow treats it as attacker-controlled:

  • PR number is resolved from the workflow_run payload / head SHA lookup — never from artifact contents (which could otherwise target an arbitrary PR).
  • The diff text is embedded using a code fence longer than any backtick run in the content, so it can't escape the fence and inject markdown into a bot-authored comment.
  • The comment workflow never checks out or executes code from the triggering ref.

Notes

  • workflow_run.pull_requests is empty for fork PRs, so the PR is resolved via listPullRequestsAssociatedWithCommit(head_sha) with an exact head.sha match.
  • The pull_request job also writes the diff to $GITHUB_STEP_SUMMARY, so the result is visible on the run page even before/without the comment.
  • No changeset: workflow-only change, no published package affected.
  • type-diff-comment.yml only takes effect once merged to main (workflow_run workflows run from the default branch). After merge, re-running the check on runtime: defer scheme resolution to globalThis.fetch in Image/Audio/Video #404 will use the updated pull_request workflow via the refreshed merge ref.

The Type Diff workflow posted its sticky PR comment directly from the
pull_request job, but the pull_request GITHUB_TOKEN is read-only for
fork-originated PRs (the workflow-level 'permissions:' cannot elevate
past that cap), so issues.createComment failed with 403
"Resource not accessible by integration" on every fork PR (e.g. #404).

Split into the standard two-stage pattern:

- type-diff.yml (pull_request, contents:read only): builds base + PR
  types, writes the diff to the job summary, and uploads it as the
  'type-diff' artifact. No API writes.
- type-diff-comment.yml (workflow_run, pull-requests:write): downloads
  the artifact from the triggering run and posts/updates the sticky
  comment. The PR number is resolved from the run payload / head SHA
  (never from artifact contents), and the diff is embedded with a
  code fence longer than any backtick run in the content, since the
  artifact is produced from untrusted fork code.
Copilot AI review requested due to automatic review settings July 6, 2026 07:58
@changeset-bot

changeset-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: f09006f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nx-js Ready Ready Preview, Comment Jul 6, 2026 8:06am

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the “Runtime Type Diff” GitHub Actions check to use a two-workflow workflow_run pattern so fork-originated PRs can still receive an automated sticky “type diff” comment without hitting GITHUB_TOKEN permission limits.

Changes:

  • Refactors .github/workflows/type-diff.yml to only build + diff types, write the diff to the job summary, and upload it as an artifact (no API writes).
  • Adds .github/workflows/type-diff-comment.yml to download the artifact on workflow_run completion and create/update the sticky PR comment with elevated permissions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/type-diff.yml Removes PR-comment posting from the pull_request workflow; generates diff + uploads artifact for a follow-up workflow.
.github/workflows/type-diff-comment.yml New workflow_run workflow that downloads the diff artifact and posts/updates the sticky PR comment using a write-capable token.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/type-diff.yml
Comment thread .github/workflows/type-diff-comment.yml
Comment thread .github/workflows/type-diff-comment.yml Outdated
- Escape-proof the job summary code fence the same way as the PR
  comment (fence longer than any backtick run in the untrusted diff)
- Defensively match merge_commit_sha in addition to head.sha when
  resolving the PR from the workflow_run head SHA
- Paginate issues.listComments when locating the sticky comment so a
  busy PR can't cause a duplicate
@TooTallNate
TooTallNate merged commit 16464ca into main Jul 6, 2026
6 checks passed
@TooTallNate
TooTallNate deleted the type-diff-fork-prs branch July 6, 2026 08:07
TooTallNate added a commit that referenced this pull request Jul 6, 2026
Follow-up to #409. The first live fork-PR run (on #404, after syncing it
with main) failed in the comment workflow with:

```
##[error]Unable to resolve PR for head SHA f638034
```

Root cause: `listPullRequestsAssociatedWithCommit` returns an **empty
list when the base repo is queried with a commit that only exists in a
fork** — verified directly:

- `GET /repos/TooTallNate/nx.js/commits/f638034.../pulls` → `[]`
- `GET /repos/natureglass/nx.js_extended/commits/f638034.../pulls` → PR
#404

So the fallback failed for exactly the fork PRs this workflow exists to
serve (same-repo PRs never reach the fallback, since
`workflow_run.pull_requests` is populated for them).

## Fix

Query the `workflow_run.head_repository` (the fork) for the commit→PR
association, and filter results to PRs that:
- target this repo (`base.repo.full_name` match), and
- have the run's exact head SHA (`head.sha` or `merge_commit_sha`)

The association data comes from GitHub's API and is authoritative — a
fork cannot fabricate a PR association pointing at an arbitrary PR, so
the comment-targeting security property from #409 is preserved.

After merging, re-running the "Type Diff" check on #404 will trigger a
fresh comment run using this fixed workflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants