created separate files#50
Conversation
WalkthroughThe previous unified merge-conflict workflow was removed and replaced by two separate GitHub Actions: a PR-triggered detector that checks a single PR against its base, and a push-triggered detector that scans all open PRs after base-branch updates and reports per-PR check-runs. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub (Event)
participant Runner as Actions Runner
participant Git as git (local)
participant GHAPI as GitHub API / gh
GH->>Runner: trigger (pull_request_target or push)
Runner->>Git: checkout repo (fetch-depth=0)
alt pull_request_target (single PR)
Runner->>GHAPI: gh pr view (metadata)
Runner->>Git: fetch base branch & PR head refs
Runner->>Git: git merge-tree <base> <head>
Git-->>Runner: merge-tree result (OK / CONFLICT)
Runner->>GHAPI: optional check-run / console output
else push (multiple PRs)
Runner->>GHAPI: list open PRs for target branch
loop per PR
Runner->>Git: fetch PR head and base refs
Runner->>Git: git merge-tree <base> <head>
Git-->>Runner: merge-tree result
Runner->>GHAPI: create/update check-run with result
end
Runner->>GHAPI: post aggregated summary
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
@palisadoes |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
.github/workflows/push-merge-conflict-detector.yml (1)
24-25: Replace inline${{ }}expressions withenv:variables.Direct context interpolation into shell scripts is a GitHub Actions security anti-pattern. For this workflow the values are admin-controlled (branch name and repo slug), so the practical risk is low, but using
env:variables eliminates the pattern entirely and is consistent with the fix needed in the PR workflow.♻️ Proposed refactor
For the
get-prsstep:+ env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BASE_BRANCH: ${{ github.ref_name }} run: | + set -euo pipefail gh pr list \ - --base "${{ github.ref_name }}" \ + --base "$BASE_BRANCH" \For the
check-all-prsstep, add to the existingenv:block and update the twogh apicall lines:env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }}- gh api repos/${{ github.repository }}/check-runs \ + gh api "repos/${REPO}/check-runs" \Apply the same replacement on both the failure (line 71) and success (line 85) paths.
Also applies to: 71-72, 85-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push-merge-conflict-detector.yml around lines 24 - 25, Replace inline GitHub Actions expressions used inside shell commands with environment variables: add env entries (e.g., BRANCH_NAME, REPO_SLUG or similar) to the get-prs and check-all-prs steps and set them to the corresponding expressions (e.g., ${{ github.ref_name }}, ${{ github.repository }}) then update the two occurrences of gh pr list and both gh api call lines to reference those env vars (e.g., "$BRANCH_NAME" and "$REPO_SLUG") instead of using ${ { ... } } inline interpolation; update both the failure and success paths mentioned so all inline ${{ }} usages in the step are replaced.
🤖 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/pull-request-merge-conflict-detector.yml:
- Around line 3-5: The workflow currently triggers on every pull_request_target
activity; narrow it to the four PR event types required by adding a types: list
under the pull_request_target trigger. Modify the workflow root block that
contains "pull_request_target:" to include "types: [opened, synchronize,
reopened, ready_for_review]" (or the specific four events named in the PR
objectives) so the conflict detector runs only on those events.
- Around line 7-11: Add a job-level permissions block to the
detect-merge-conflict job to enforce least privilege: in the job named
detect-merge-conflict add a permissions map that grants only read access
required (e.g., contents: read and pull-requests: read) instead of the default
write token; update the job definition where runs-on is set so the token used by
this job is scoped to read-only access to repo contents and PR metadata.
- Around line 19-29: The run script currently interpolates GitHub context
expressions directly (e.g., `${{ github.event.pull_request.base.ref }}` and `${{
github.event.pull_request.number }}`) which enables script injection; move those
expressions into the job's env block (add env entries like BASE_REF: ${{
github.event.pull_request.base.ref }} and PR_NUMBER: ${{
github.event.pull_request.number }}) and update the script to read the safe
shell variables (e.g., use "$BASE_REF" and "$PR_NUMBER" in the run body) instead
of embedding `${{ }}` inside the run script; keep GH_TOKEN in env as-is and
remove any `${{ ... }}` usage from the run steps.
In @.github/workflows/push-merge-conflict-detector.yml:
- Around line 23-31: Add strict shell failure handling to the get-prs step by
prepending "set -euo pipefail" at the start of the run block so any failure in
the gh pr list command (or any subsequent pipe) aborts the job instead of
producing an empty pr_numbers.txt; keep references to the existing commands (gh
pr list, pr_numbers.txt, COUNT, and echo "count=$COUNT" >> $GITHUB_OUTPUT) so
behavior is unchanged except for immediate failure on errors.
- Around line 8-11: The detect-conflicts job needs an explicit job-level
permissions block so the GITHUB_TOKEN can create check-runs; add a permissions
section under the detect-conflicts job (referencing the job name
detect-conflicts) with checks: write (e.g., permissions: checks: write) to
ensure POST /repos/{owner}/{repo}/check-runs succeeds.
- Around line 55-64: The check currently treats only exit code 1 as a conflict,
so change the exit-code check around CODE (set from git merge-tree) to treat any
non-zero exit as a failure (use "-ne 0" instead of "-eq 1") so git errors (e.g.
128) are not considered clean; keep the existing grep for "CONFLICT" and still
set HAS_CONFLICT=true when either the grep finds "CONFLICT" in OUTPUT or CODE is
non-zero, referencing the git merge-tree call and the CODE and HAS_CONFLICT
variables in the diff.
---
Nitpick comments:
In @.github/workflows/push-merge-conflict-detector.yml:
- Around line 24-25: Replace inline GitHub Actions expressions used inside shell
commands with environment variables: add env entries (e.g., BRANCH_NAME,
REPO_SLUG or similar) to the get-prs and check-all-prs steps and set them to the
corresponding expressions (e.g., ${{ github.ref_name }}, ${{ github.repository
}}) then update the two occurrences of gh pr list and both gh api call lines to
reference those env vars (e.g., "$BRANCH_NAME" and "$REPO_SLUG") instead of
using ${ { ... } } inline interpolation; update both the failure and success
paths mentioned so all inline ${{ }} usages in the step are replaced.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/merge-conflict-detector.yml.github/workflows/pull-request-merge-conflict-detector.yml.github/workflows/push-merge-conflict-detector.yml
💤 Files with no reviewable changes (1)
- .github/workflows/merge-conflict-detector.yml
palisadoes
left a comment
There was a problem hiding this comment.
Please address these findings:
|
@coderabbitai review |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/push-merge-conflict-detector.yml (1)
74-99: Consider extracting duplicated check-run POST logic into a helper shell function.Only conclusion/title/summary differ between the two branches; extracting one function will reduce maintenance risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push-merge-conflict-detector.yml around lines 74 - 99, Duplicate gh api POST blocks for creating check-runs should be extracted into a helper shell function (e.g., post_check_run) to reduce duplication; implement a function that accepts parameters for conclusion, output title, and output summary (and uses CHECK_NAME and HEAD_SHA from the environment), call it in both branches instead of repeating the long gh api invocation, and set FAILED=true in the HAS_CONFLICT branch as before while keeping the echo messages for PR and variable names (HAS_CONFLICT, PR, CHECK_NAME, HEAD_SHA, FAILED) intact.
🤖 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/push-merge-conflict-detector.yml:
- Around line 51-99: The current conflict detector treats any merge-tree
conflict as a PR conflict and posts only a generic check-run; change logic to
(1) obtain the PR changed file list from gh (use the existing DATA/json fields
or call gh pr view "$PR" --json files to get files[]), (2) parse the raw OUTPUT
from git merge-tree to extract the list of conflicted file paths, (3) compute
the intersection between conflicted files and the PR file list and only set
HAS_CONFLICT=true (and FAILED=true) when that intersection is non-empty, and (4)
enrich the gh api repos/.../check-runs payload (the POST using
name="$CHECK_NAME", head_sha="$HEAD_SHA", etc.) to include a detailed output:
title, a summary and output[text] that contain PR title, base/head refs+SHAs
(use BASE_REF, BASE_SHA, HEAD_SHA and headRefName/title from DATA), total
changed files count, the list of conflicted PR files, and brief remediation
guidance; locate changes around the variables and commands OUTPUT, git
merge-tree, HAS_CONFLICT, HEAD_SHA, BASE_SHA and the gh api check-runs POST
blocks and replace the generic messages with the detailed payload built from the
intersected file list.
---
Nitpick comments:
In @.github/workflows/push-merge-conflict-detector.yml:
- Around line 74-99: Duplicate gh api POST blocks for creating check-runs should
be extracted into a helper shell function (e.g., post_check_run) to reduce
duplication; implement a function that accepts parameters for conclusion, output
title, and output summary (and uses CHECK_NAME and HEAD_SHA from the
environment), call it in both branches instead of repeating the long gh api
invocation, and set FAILED=true in the HAS_CONFLICT branch as before while
keeping the echo messages for PR and variable names (HAS_CONFLICT, PR,
CHECK_NAME, HEAD_SHA, FAILED) intact.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pull-request-merge-conflict-detector.yml.github/workflows/push-merge-conflict-detector.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pull-request-merge-conflict-detector.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/push-merge-conflict-detector.yml (1)
94-103: Deduplicate relevant conflict paths before counting/reporting.A file can appear multiple times in
merge-treeoutput; without dedupe,CONFLICT_COUNTcan be inflated.♻️ Proposed refactor
- RELEVANT_CONFLICTS=$(echo "$RELEVANT_CONFLICTS" | sed '/^$/d') + RELEVANT_CONFLICTS=$(printf '%s\n' "$RELEVANT_CONFLICTS" \ + | sed '/^$/d' \ + | sort -u) CONFLICT_COUNT=$(echo "$RELEVANT_CONFLICTS" | grep -c . || true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/push-merge-conflict-detector.yml around lines 94 - 103, RELEVANT_CONFLICTS can contain duplicate paths from the merge-tree output, inflating CONFLICT_COUNT; after building RELEVANT_CONFLICTS (from ALL_CONFLICTED_FILES) deduplicate it before counting and reporting by replacing the current RELEVANT_CONFLICTS processing with a step that sorts/uniques the list (so subsequent CONFLICT_COUNT=$(echo "$RELEVANT_CONFLICTS" | grep -c . || true) reflects unique files only), ensuring references to RELEVANT_CONFLICTS and CONFLICT_COUNT (and the source ALL_CONFLICTED_FILES / merge-tree generation) are updated accordingly.
🤖 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/push-merge-conflict-detector.yml:
- Around line 54-63: gh pr view --json files is truncating to 100 files so
PR_FILES (and therefore RELEVANT_CONFLICTS) can miss files; replace the files
extraction with a paginated REST call using "gh api" to list pull request files
(GET /repos/{owner}/{repo}/pulls/{pr}/files?per_page=100) and iterate pages to
collect all .path values into PR_FILES, then recompute TOTAL_FILES from that
full list; keep using gh pr view for HEAD_SHA, HEAD_REF, BASE_REF, TITLE but
remove files from that JSON usage and ensure any downstream variables (PR_FILES,
TOTAL_FILES, RELEVANT_CONFLICTS) use the fully-paginated file list.
---
Nitpick comments:
In @.github/workflows/push-merge-conflict-detector.yml:
- Around line 94-103: RELEVANT_CONFLICTS can contain duplicate paths from the
merge-tree output, inflating CONFLICT_COUNT; after building RELEVANT_CONFLICTS
(from ALL_CONFLICTED_FILES) deduplicate it before counting and reporting by
replacing the current RELEVANT_CONFLICTS processing with a step that
sorts/uniques the list (so subsequent CONFLICT_COUNT=$(echo
"$RELEVANT_CONFLICTS" | grep -c . || true) reflects unique files only), ensuring
references to RELEVANT_CONFLICTS and CONFLICT_COUNT (and the source
ALL_CONFLICTED_FILES / merge-tree generation) are updated accordingly.
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
palisadoes
left a comment
There was a problem hiding this comment.
Will all open PRs in the repository receive a merge conflict message if any of the open PRs in the repository have a merge conflict? If this is so, then the solution isn't what we need. We cannot have false positives as they cause confusion and eventually people will ignore them.
Yes it marks all of them if they have conflicting files. |
|
What actually do we need then? |
|
This pull request did not get any activity in the past 10 days and will be closed in 360 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. |
fixes: #33
Summary by CodeRabbit