Skip to content

ci(deploy-neep): fetch fork PR heads via refs/pull/<N>/head#3953

Merged
majestyotbr merged 1 commit into
mainfrom
beats/deploy-neep-fork-pr-checkout
May 9, 2026
Merged

ci(deploy-neep): fetch fork PR heads via refs/pull/<N>/head#3953
majestyotbr merged 1 commit into
mainfrom
beats/deploy-neep-fork-pr-checkout

Conversation

@beats-dh

@beats-dh beats-dh commented May 9, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #3951 and #3952. The deploy on #3945 (comment) failed at [1/7] Checking out branch because the PR's head branch Add-linux-release-arm-CMake-preset only exists in the contributor's fork:

=== [1/7] Checking out branch: Add-linux-release-arm-CMake-preset ===
From https://github.com/opentibiabr/canary
   6dd0bd534..85c2475aa  main       -> origin/main
Switched to a new branch 'Add-linux-release-arm-CMake-preset'
fatal: ambiguous argument 'origin/Add-linux-release-arm-CMake-preset': unknown revision or path not in the working tree.

The script ran git fetch origin (upstream opentibiabr/canary) followed by git reset --hard origin/$BRANCH, which can't resolve a fork's branch.

GitHub maintains a refs/pull/<N>/head ref on the upstream repo for every PR — same-repo or fork — so we can fetch and reset against that ref instead.

Behaviour

Actual

/deploy on a PR opened from a fork aborts at the checkout step.

Expected

/deploy works for both same-repo and fork PRs.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

What changed

  • New shell vars HAS_PR and PR_NUMBER plumbed into the remote script from steps.setup.outputs.
  • Checkout block now branches:
    • PR context (HAS_PR=true and PR_NUMBER set): git fetch origin "+refs/pull/${PR_NUMBER}/head:refs/remotes/pr/${PR_NUMBER}", then git checkout -B "$BRANCH" "refs/remotes/pr/${PR_NUMBER}", then git reset --hard "refs/remotes/pr/${PR_NUMBER}". Works for fork PRs and same-repo PRs alike.
    • No PR context (workflow_dispatch on a known branch): unchanged — git checkout "$BRANCH" || git checkout -b "$BRANCH" then git reset --hard "origin/$BRANCH".
  • The $BRANCH name is still used as the local branch name on the VPS and in the MOTD.

Security note: the existing author_association gate (OWNER/MEMBER/COLLABORATOR only) still applies, so only trusted maintainers can trigger a fork-code deploy via /deploy.

How Has This Been Tested

  • /deploy on a same-repo PR — verify checkout still works through the new PR-ref path.
  • /deploy on a fork PR — verify the deploy proceeds past the checkout step (the original symptom).
  • workflow_dispatch on a non-PR branch — verify the fallback path still works.

Test Configuration:

  • Server Version: any
  • Client: n/a (CI workflow)
  • Operating System: Ubuntu (runner) + remote VPS

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I checked the PR checks reports
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Notes for reviewers

A separate, cosmetic issue is still visible in the same comment: every block of output is duplicated (e.g. the === [1/7] ... block, and even drone-ssh's own ✅ Successfully executed commands to all hosts. banner). The drone-ssh banner is emitted locally by appleboy/ssh-action, not by our remote script — its appearance in outputs.stdout confirms the duplication is in the action's capture_stdout mechanism rather than in our log file. Worth addressing as a follow-up; not in scope here.

Summary by CodeRabbit

  • Chores
    • Updated deployment workflow to enable triggering deployments for pull requests via issue comments, including support for pull requests from forked repositories.

Review Change Stack

The previous checkout assumed the PR branch existed on `origin` and did
`git reset --hard origin/$BRANCH`, which fails for PRs from forks. The
deploy aborted with `fatal: ambiguous argument 'origin/<branch>'` — see
#3945 (deploy comment).

When a PR is detected, fetch the special `refs/pull/<N>/head` ref that
GitHub maintains for every PR (same-repo or fork) and reset to it.
Fall back to the original origin-branch path for `workflow_dispatch`
(no PR context).
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98c7304d-facb-4019-ae40-5aa8159aaf0b

📥 Commits

Reviewing files that changed from the base of the PR and between 85c2475 and 468ec24.

📒 Files selected for processing (1)
  • .github/workflows/deploy-neep.yml

📝 Walkthrough

Walkthrough

The Neep deployment workflow's remote SSH script is updated to support checking out pull request heads when deployment is triggered by issue comments. It conditionally fetches refs/pull/<PR_NUMBER>/head and checks out from that ref if PR context is available; otherwise it defaults to the original branch-based checkout behavior.

Changes

PR-Aware Deployment Checkout

Layer / File(s) Summary
PR-Aware Git Checkout Logic
.github/workflows/deploy-neep.yml
The remote deploy script exports HAS_PR and PR_NUMBER from workflow outputs and adds conditional checkout: when PR context is present, fetches and checks out the PR head ref; otherwise checks out the target branch from origin/$BRANCH.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • opentibiabr/canary#3951: Both PRs modify the same deploy-neep GitHub Actions remote deploy script checkout and SSH remote steps.
  • opentibiabr/canary#3873: Both PRs modify GitHub Actions checkout logic to ensure PR head refs are fetched and checked out correctly.

Suggested labels

codex

Suggested reviewers

  • majestyotbr

Poem

🐰 A rabbit hops through CI flows,
Pulling PRs where the workflow goes,
Fork or branch, the deploy now knows,
Which ref to fetch—how the script grows!
Conditional checkout bliss! 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: supporting fork PR head checkout via refs/pull//head in the deploy-neep workflow, which directly addresses the PR's primary objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch beats/deploy-neep-fork-pr-checkout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

sonarqubecloud Bot commented May 9, 2026

Copy link
Copy Markdown

@majestyotbr majestyotbr merged commit 3847077 into main May 9, 2026
17 checks passed
@majestyotbr majestyotbr deleted the beats/deploy-neep-fork-pr-checkout branch May 9, 2026 18:06
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