Skip to content

Fix misleading fork detection for issue_comment and other minimal-PR events#26144

Merged
dsyme merged 4 commits intomainfrom
fix/defer-fork-detection-for-minimal-pr
Apr 14, 2026
Merged

Fix misleading fork detection for issue_comment and other minimal-PR events#26144
dsyme merged 4 commits intomainfrom
fix/defer-fork-detection-for-minimal-pr

Conversation

@dsyme
Copy link
Copy Markdown
Collaborator

@dsyme dsyme commented Apr 14, 2026

Problem

For non-pull_request events (e.g. issue_comment, pull_request_review), the event payload only contains a minimal PR object ({number, state}) without head/base data. detectForkPR() was called on this minimal object, which incorrectly reported "head repository deleted (was likely a fork)" and emitted a misleading fork warning (⚠️ Fork PR detected).

Solution

  • logPRContext: Reports fork status as "unknown" when pullRequest.head is missing, instead of calling detectForkPR on incomplete data
  • fetchPRDetails: Now returns the full PR object alongside commitCount/headRef
  • main() else branch: Fetches full PR data first, then resolves fork status accurately using detectForkPR(fullPR) — logs "Is fork PR (from API): ..." when resolved

Tests

  • 2 new tests verify the minimal-PR scenario (non-fork and fork cases via issue_comment with payload.issue.pull_request)
  • Updated race condition test mocks to include full PR data
  • All 49 tests pass

For non-pull_request events (e.g. issue_comment), the event payload
only contains a minimal PR object ({number, state}) without head/base
data. Previously, detectForkPR was called on this minimal object,
which incorrectly reported 'head repository deleted (was likely a fork)'
and emitted a misleading fork warning.

Now:
- logPRContext reports fork status as 'unknown' when head data is missing
- fetchPRDetails returns the full PR object from the API
- Fork detection runs on the full API data in the else branch
- Fork warning is only emitted when fork status is truly confirmed

Also uses strict isFork === false check for the fast path to avoid
treating null (unknown) as non-fork.
Copilot AI review requested due to automatic review settings April 14, 2026 02:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes misleading fork detection for minimal PR payload events (e.g., issue_comment) by treating fork status as unknown until full PR details are fetched from the GitHub API.

Changes:

  • Update logPRContext to avoid calling fork detection when PR payload lacks full details, logging fork status as unknown.
  • Extend fetchPRDetails to return the full PR object along with commitCount/headRef, and re-resolve fork status from API data.
  • Add/adjust tests to cover minimal-PR payload scenarios and update mocks to include head/base repo data.
Show a summary per file
File Description
actions/setup/js/checkout_pr_branch.cjs Defers fork detection for minimal PR payloads and re-evaluates fork status from API-fetched full PR data.
actions/setup/js/checkout_pr_branch.test.cjs Adds coverage for minimal PR payload events and updates mocks to match new API data expectations.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

dsyme and others added 3 commits April 14, 2026 12:08
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dsyme dsyme merged commit 9f16c6f into main Apr 14, 2026
54 of 55 checks passed
@dsyme dsyme deleted the fix/defer-fork-detection-for-minimal-pr branch April 14, 2026 02:32
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