fix: check out PR feature branch in pr-fixer workflow#880
fix: check out PR feature branch in pr-fixer workflow#880Gkrumbach07 wants to merge 1 commit intomainfrom
Conversation
The pr-fixer sessions now resolve the PR's head branch and pass it in the repos config so the runner clones the feature branch directly instead of starting on main. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughModifies the PR Fixer workflow to dynamically retrieve PR branch names using Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pr-fixer.yml (1)
198-216:⚠️ Potential issue | 🟠 Major
wait: 'false'removes the completion signal this workflow still depends on.
ambient-action@v0.0.2only populatessession-phasewhenwait: true, but the summary step below still reads this output (line 223). With this change, the matrix job goes green as soon as the session is created, regardless of whether the fixer later succeeds or fails, and the Phase field in the summary will always be blank. Either keep the action synchronous here or stop consuming completion outputs in the summary.🔧 If you need completion visibility
- wait: 'false' + wait: 'true'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/pr-fixer.yml around lines 198 - 216, The workflow sets the ambient-action step (id: session) with wait: 'false', but downstream summary logic still reads the action's completion output (session-phase); change either the action to run synchronously by setting wait: 'true' on the step with id "session" so session-phase is populated, or keep wait: 'false' and remove/stop consuming session-phase in the summary step (the step that reads outputs.session-phase) so the workflow no longer depends on a completion signal that isn't produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-fixer.yml:
- Around line 70-77: The manual-dispatch path uses the pr_branch step (id:
pr_branch) and currently calls gh pr view with --repo set to the base
${GITHUB_REPOSITORY}, which can cause the workflow to checkout/create branches
in the base repo for fork PRs; update the pr_branch logic to perform the same
cross-repo guard used elsewhere (the isCrossRepository check / steps.fork_check
result) or resolve the PR head repository URL from gh pr view and pass that repo
into --repo (or repos) when invoking gh; ensure you reference the PR number
input (steps.pr.outputs.number) and only use the base repo when the PR is not
cross-repository, otherwise use the head repo returned by gh pr view so the
branch name is resolved against the correct repository.
---
Outside diff comments:
In @.github/workflows/pr-fixer.yml:
- Around line 198-216: The workflow sets the ambient-action step (id: session)
with wait: 'false', but downstream summary logic still reads the action's
completion output (session-phase); change either the action to run synchronously
by setting wait: 'true' on the step with id "session" so session-phase is
populated, or keep wait: 'false' and remove/stop consuming session-phase in the
summary step (the step that reads outputs.session-phase) so the workflow no
longer depends on a completion signal that isn't produced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: afd5b196-4c74-422d-b86f-13e317c09247
📒 Files selected for processing (1)
.github/workflows/pr-fixer.yml
| - name: Get PR branch | ||
| if: steps.fork_check.outputs.skip != 'true' | ||
| id: pr_branch | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| BRANCH=$(gh pr view ${{ steps.pr.outputs.number }} --repo "${{ github.repository }}" --json headRefName --jq '.headRefName') | ||
| echo "branch=$BRANCH" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
Block fork PRs in the manual-dispatch path.
workflow_dispatch still skips the fork guard, but this change now feeds the PR’s headRefName into a repo URL that is always ${{ github.repository }}. For a fork PR, that targets the base repo with a branch name from the contributor’s fork; the runner’s checkout path falls back to git checkout -b <branch> when the branch is missing, so the fixer can create/push a same-named branch in the base repository instead of the PR head repo. Please run the same isCrossRepository check for manual dispatch too, or resolve the PR head repository URL and pass that into repos.
🔧 Minimal fix
- - name: Check PR is not a fork (issue_comment)
- if: github.event_name == 'issue_comment'
+ - name: Check PR is not a fork
+ if: github.event_name == 'issue_comment' || github.event_name == 'workflow_dispatch'Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pr-fixer.yml around lines 70 - 77, The manual-dispatch
path uses the pr_branch step (id: pr_branch) and currently calls gh pr view with
--repo set to the base ${GITHUB_REPOSITORY}, which can cause the workflow to
checkout/create branches in the base repo for fork PRs; update the pr_branch
logic to perform the same cross-repo guard used elsewhere (the isCrossRepository
check / steps.fork_check result) or resolve the PR head repository URL from gh
pr view and pass that repo into --repo (or repos) when invoking gh; ensure you
reference the PR number input (steps.pr.outputs.number) and only use the base
repo when the PR is not cross-repository, otherwise use the head repo returned
by gh pr view so the branch name is resolved against the correct repository.
🧪 CI InsightsHere's what we observed from your CI run for bc62973. 🟢 All jobs passed!But CI Insights is watching 👀 |
Review Queue — Not Ready to MergeThe manual workflow dispatch path is missing a fork guard, which could allow pushes to the wrong repository when triggered on fork PRs. Additionally, setting To unblock: Add the fork guard to the workflow_dispatch path (or resolve the PR head repository URL correctly), and either keep |
|
Closing — fork guard issue needs a broader rethink. |
1 participant
Summary
gh pr viewand pass it in thereposconfigmainTest plan
🤖 Generated with Claude Code