feat/added pr conflict detection gha #6369
Conversation
|
To generate Unit Tests for this PR, please click here. |
Our Pull Request Approval ProcessThis PR will be reviewed according to our: Your PR may be automatically closed if:
Thanks for contributing! |
WalkthroughAdds a new GitHub Actions job Changes
Sequence DiagramssequenceDiagram
actor Trigger
participant WF as GitHub Actions Workflow
participant Checkout as Actions/checkout
participant GH as GitHub CLI
participant Git as Git (fetch, merge-base, merge-tree)
participant PR as Pull Request
Trigger->>WF: pull_request_target event
WF->>Checkout: checkout PR head (fetch-depth: 0)
WF->>GH: list PR changed files
WF->>Git: fetch base branch & compute merge-base
WF->>Git: run git merge-tree (base..head)
Git-->>WF: merge-tree output (conflict candidates)
WF->>WF: parse & filter conflicts to PR file list
alt Conflicts Found
WF->>PR: post summary / exit 1
else No Conflicts
WF->>PR: exit 0
end
sequenceDiagram
actor Trigger
participant WF as GitHub Actions Workflow
participant Checkout as Actions/checkout
participant GH as GitHub CLI
participant Git as Git (fetch, merge-base, merge-tree)
participant PRs as Open PRs
Trigger->>WF: push event (base branch updated)
WF->>Checkout: checkout repo (fetch-depth: 0)
WF->>GH: query open PRs targeting base
loop for each open PR
WF->>GH: get PR details & changed files
WF->>Git: compute merge-base with base
WF->>Git: run git merge-tree
Git-->>WF: merge-tree output
WF->>WF: parse & filter to PR files
alt Conflicts Detected
WF->>PRs: post comment with conflict details & resolution steps
end
end
alt Any Conflicts Found
WF->>WF: exit 1
else No Conflicts
WF->>WF: exit 0
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
🤖 Fix all issues with AI agents
In @.github/workflows/pr-conflict-detection.yml:
- Around line 254-261: The single-PR parsing logic must be changed to match the
multi-PR block: only extract files from "CONFLICT" markers instead of also using
"Auto-merging". Update the earlier pipeline that writes to
pr_${pr_number}_conflicts.txt to filter MERGE_OUTPUT with grep "^CONFLICT" and
the same sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/'
transformation, then dedupe/sort and write to pr_${pr_number}_conflicts.txt
(same approach used in the shown block) so both checks behave identically.
- Around line 133-135: The current conflict detection greps MERGE_OUTPUT for
"Auto-merging" which produces false positives because "Auto-merging <file>" is
emitted for successful merges; update the pipeline to only detect real conflicts
by removing the "Auto-merging" pattern from the grep (i.e., stop using grep -E
"^\+<<<<<|^Auto-merging") and instead only search MERGE_OUTPUT for conflict
indicators such as CONFLICT or the existing "^\+<<<<<" before appending
filenames to all_conflicts.txt; ensure any reference to MERGE_OUTPUT processing
and the file all_conflicts.txt is adjusted accordingly so only true conflicts
are recorded.
- Around line 287-302: The PR comment block contains a duplicated "gh pr comment
${pr_number} --body" line and an improperly closed multi-line string plus
missing control-structure terminators; remove the duplicate invocation, ensure
the heredoc/quoted multi-line body for gh pr comment ${pr_number} is a single
correctly opened and closed string, add the missing fi to close the if block
that guards the conflict-reporting logic, add the missing done to close the
while loop that iterates PRs, and ensure the script ends cleanly (e.g., proper
exit or end-of-file) so the YAML parses and the workflow can run.
- Around line 79-83: Add the PR title to the step's environment and reference
the env var in the script instead of embedding the pull request title directly;
specifically, in the "Check single PR for conflicts" step (the step with env:
GH_TOKEN) add an env entry like PR_TITLE: ${{ github.event.pull_request.title }}
and then update the script at the location where the PR title is used (around
the existing usage at line 172) to read from $PR_TITLE rather than injecting the
title inline.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr-conflict-detection.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/pr-conflict-detection.yml
28-28: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
83-83: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Generate and Validate Documentation / autodocs
🔇 Additional comments (3)
.github/workflows/pr-conflict-detection.yml (3)
3-13: Appropriate trigger and permission configuration.Using
pull_request_targetis correct here since you need write access to post comments on PRs from forks. The permissions are appropriately scoped for the workflow's requirements.
48-77: LGTM!The PR fetching logic is correct and handles the edge case of no open PRs appropriately.
21-25: Verify no untrusted code execution from PR checkout.Using
pull_request_targetwithref: ${{ github.event.pull_request.head.sha }}checks out potentially untrusted PR code. This workflow appears safe since it only runsgitandghcommands without executing scripts from the repo. Ensure no future modifications add build/test steps that would execute PR code.
| - name: Check single PR for conflicts | ||
| if: github.event_name == 'pull_request_target' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | |
There was a problem hiding this comment.
Script injection vulnerability via PR title (same issue as logging step).
The PR title is used again at line 172 within the script. Apply the same fix by passing it through an environment variable.
🔒 Proposed fix
- name: Check single PR for conflicts
if: github.event_name == 'pull_request_target'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ PR_TITLE: ${{ github.event.pull_request.title }}
run: |Then at line 172, replace:
- echo "[RESULT] PR #${{ github.event.pull_request.number }}: ${{ github.event.pull_request.title }}"
+ echo "[RESULT] PR #${{ github.event.pull_request.number }}: ${PR_TITLE}"🧰 Tools
🪛 actionlint (1.7.10)
83-83: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
In @.github/workflows/pr-conflict-detection.yml around lines 79 - 83, Add the PR
title to the step's environment and reference the env var in the script instead
of embedding the pull request title directly; specifically, in the "Check single
PR for conflicts" step (the step with env: GH_TOKEN) add an env entry like
PR_TITLE: ${{ github.event.pull_request.title }} and then update the script at
the location where the PR title is used (around the existing usage at line 172)
to read from $PR_TITLE rather than injecting the title inline.
| # Also check for "changed in both" indicators | ||
| echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \ | ||
| grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true |
There was a problem hiding this comment.
Potential false positives from Auto-merging detection.
Auto-merging <file> is logged by git for files that were automatically merged successfully without conflicts. Including these lines will flag files as conflicted even when they merged cleanly. Only CONFLICT lines indicate actual conflicts.
🐛 Proposed fix - remove Auto-merging detection
# Look for CONFLICT markers
echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \
sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \
grep -v "^CONFLICT" >> all_conflicts.txt || true
- # Also check for "changed in both" indicators
- echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \
- grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Also check for "changed in both" indicators | |
| echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \ | |
| grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true |
🤖 Prompt for AI Agents
In @.github/workflows/pr-conflict-detection.yml around lines 133 - 135, The
current conflict detection greps MERGE_OUTPUT for "Auto-merging" which produces
false positives because "Auto-merging <file>" is emitted for successful merges;
update the pipeline to only detect real conflicts by removing the "Auto-merging"
pattern from the grep (i.e., stop using grep -E "^\+<<<<<|^Auto-merging") and
instead only search MERGE_OUTPUT for conflict indicators such as CONFLICT or the
existing "^\+<<<<<" before appending filenames to all_conflicts.txt; ensure any
reference to MERGE_OUTPUT processing and the file all_conflicts.txt is adjusted
accordingly so only true conflicts are recorded.
| # Parse conflicts | ||
| touch "pr_${pr_number}_conflicts.txt" | ||
| echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \ | ||
| sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \ | ||
| grep -v "^CONFLICT" >> "pr_${pr_number}_conflicts.txt" || true | ||
| sort -u "pr_${pr_number}_conflicts.txt" | grep -v "^$" > "pr_${pr_number}_conflicts_unique.txt" || touch "pr_${pr_number}_conflicts_unique.txt" | ||
| mv "pr_${pr_number}_conflicts_unique.txt" "pr_${pr_number}_conflicts.txt" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent conflict parsing between single PR and all-PR checks.
This section correctly only extracts from CONFLICT markers, but the single PR check (lines 129-135) also includes Auto-merging lines which can cause false positives. Align both to use only the CONFLICT marker approach.
🤖 Prompt for AI Agents
In @.github/workflows/pr-conflict-detection.yml around lines 254 - 261, The
single-PR parsing logic must be changed to match the multi-PR block: only
extract files from "CONFLICT" markers instead of also using "Auto-merging".
Update the earlier pipeline that writes to pr_${pr_number}_conflicts.txt to
filter MERGE_OUTPUT with grep "^CONFLICT" and the same sed -E 's/^CONFLICT
\([^)]+\): Merge conflict in (.+)$/\1/' transformation, then dedupe/sort and
write to pr_${pr_number}_conflicts.txt (same approach used in the shown block)
so both checks behave identically.
| gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected** | ||
| gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected** | ||
| This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged. | ||
| **Conflicted files:** | ||
| $(cat "pr_${pr_number}_filtered.txt" | sed 's/^/- /') | ||
| **Action required:** Please update your branch to resolve these conflicts: | ||
| \`\`\`bash | ||
| git fetch origin | ||
| git checkout ${PR_HEAD_REF} | ||
| git merge origin/${PR_BASE_REF} | ||
| # Resolve conflicts, then: | ||
| git push | ||
| \`\`\`" |
There was a problem hiding this comment.
Critical syntax errors: duplicate line and missing control structure closures.
- Lines 287-288 are duplicated - this is a copy-paste error
- The file appears truncated - missing
fito close theifat line 278, missingdoneto close thewhileloop at line 212, and no proper script termination
This workflow will fail YAML parsing and won't execute.
🐛 Proposed fix structure
# Post comment on PR
- gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected**
gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected**
This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged.
**Conflicted files:**
$(cat "pr_${pr_number}_filtered.txt" | sed 's/^/- /')
**Action required:** Please update your branch to resolve these conflicts:
\`\`\`bash
git fetch origin
git checkout ${PR_HEAD_REF}
git merge origin/${PR_BASE_REF}
# Resolve conflicts, then:
git push
\`\`\`"
+ else
+ echo "[CHECK-ALL] ✅ PR #${pr_number} has no conflicts"
+ fi
+ done < pr_numbers.txt
+
+ echo "[CHECK-ALL] ========================================"
+ echo "[CHECK-ALL] SUMMARY: ${FAILED_COUNT} PR(s) with conflicts"
+ if [ "${FAILED_COUNT}" -gt 0 ]; then
+ echo "[CHECK-ALL] Failed PRs: ${FAILED_PRS}"
+ exit 1
+ fi
+ echo "[CHECK-ALL] ========================================"🤖 Prompt for AI Agents
In @.github/workflows/pr-conflict-detection.yml around lines 287 - 302, The PR
comment block contains a duplicated "gh pr comment ${pr_number} --body" line and
an improperly closed multi-line string plus missing control-structure
terminators; remove the duplicate invocation, ensure the heredoc/quoted
multi-line body for gh pr comment ${pr_number} is a single correctly opened and
closed string, add the missing fi to close the if block that guards the
conflict-reporting logic, add the missing done to close the while loop that
iterates PRs, and ensure the script ends cleanly (e.g., proper exit or
end-of-file) so the YAML parses and the workflow can run.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6369 +/- ##
========================================
Coverage 96.74% 96.74%
========================================
Files 456 456
Lines 16598 16598
Branches 5118 5195 +77
========================================
Hits 16057 16057
Misses 528 528
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
palisadoes
left a comment
palisadoes
left a comment
There was a problem hiding this comment.
-
The YAML will need to be added to this file, either within the file or referenced from it.
.github/workflows/pull-request-target.yml -
That way the failure of the pull request target workflow is guaranteed.
not as a separate file:
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @.github/workflows/pull-request-target.yml:
- Around line 82-86: The workflow step "Check single PR for conflicts" injects
the PR title directly into the shell run block creating a script-injection risk;
fix by adding PR_TITLE to the step's env block (set PR_TITLE: ${{
github.event.pull_request.title }}) and reference the env var inside the run
script instead of interpolating the raw title, ensuring any use of the title in
the run block reads from $PR_TITLE and not from unescaped GitHub context.
- Around line 19-22: Add a concurrency stanza to the detect-merge-conflicts job
so parallel runs are deduplicated; specifically, in the detect-merge-conflicts
job (named "Detect Merge Conflicts in PRs") add a concurrency block with a
stable group identifier (e.g., use github.workflow combined with the PR
identifier or branch ref such as github.ref or github.event.pull_request.number)
and set cancel-in-progress: true to ensure any in-progress run is cancelled when
a new push triggers the workflow.
- Around line 204-305: The loop detects conflicts but never fails the job; after
the while loop that reads pr_numbers.txt, add a check that if FAILED_COUNT (or
FAILED_PRS) is greater than 0 then print a concise summary (e.g., list
FAILED_PRS and FAILED_COUNT) and exit with a non‑zero status (exit 1) so the
workflow fails on push when conflicts are found; ensure this check runs only in
the push-context branch of the job that uses FAILED_COUNT/FAILED_PRS.
- Around line 289-305: The PR comment body passed to gh pr comment ${pr_number}
--body contains leading indentation that will render in the GitHub comment;
remove that by replacing the indented inline multiline string with a dedented
heredoc or by passing a left-aligned body file so all lines (the warning, code
block and the $(cat "pr_${pr_number}_filtered.txt" | sed ...) list) start at
column 0; update the call site (gh pr comment ${pr_number} --body) to use the
non-indented heredoc or --body-file approach so the rendered comment has no
extra leading whitespace.
- Around line 257-265: The batch conflict parsing only extracts "CONFLICT" lines
from MERGE_OUTPUT, causing inconsistency with the single-PR path that also
extracts "Auto-merging" file lines; update the batch block that writes
"pr_${pr_number}_conflicts.txt" to parse both CONFLICT and Auto-merging patterns
from ${MERGE_OUTPUT} (same extraction/normalization logic used in the single-PR
code), ensuring filenames from "Auto-merging ..." are captured and deduplicated
before sorting into "pr_${pr_number}_conflicts.txt".
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request-target.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/pull-request-target.yml
31-31: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
86-86: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
.github/workflows/pull-request-target.yml (2)
3-12: LGTM - Permissions and triggers are appropriately scoped.The use of
pull_request_targetis appropriate for this conflict detection use case since it needs to run against the base branch context. Permissions follow least-privilege principle with only read access to contents and write access to pull-requests (for comments) and checks.
51-81: LGTM - PR discovery logic is correct.The step properly queries open PRs targeting the base branch and sets the
has_prsoutput for conditional execution of subsequent steps.
| detect-merge-conflicts: | ||
| name: Detect Merge Conflicts in PRs | ||
| runs-on: ubuntu-latest | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding concurrency control to prevent duplicate comments.
Multiple rapid pushes to develop could trigger parallel workflow runs, each posting comments on the same conflicted PRs. Adding a concurrency group would cancel in-progress runs when a new push occurs.
♻️ Proposed enhancement
detect-merge-conflicts:
name: Detect Merge Conflicts in PRs
runs-on: ubuntu-latest
+ concurrency:
+ group: conflict-check-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true
steps:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| detect-merge-conflicts: | |
| name: Detect Merge Conflicts in PRs | |
| runs-on: ubuntu-latest | |
| detect-merge-conflicts: | |
| name: Detect Merge Conflicts in PRs | |
| runs-on: ubuntu-latest | |
| concurrency: | |
| group: conflict-check-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }} | |
| cancel-in-progress: true | |
🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 19 - 22, Add a
concurrency stanza to the detect-merge-conflicts job so parallel runs are
deduplicated; specifically, in the detect-merge-conflicts job (named "Detect
Merge Conflicts in PRs") add a concurrency block with a stable group identifier
(e.g., use github.workflow combined with the PR identifier or branch ref such as
github.ref or github.event.pull_request.number) and set cancel-in-progress: true
to ensure any in-progress run is cancelled when a new push triggers the
workflow.
| # Parse conflicts from merge-tree output | ||
| touch all_conflicts.txt | ||
| # Look for CONFLICT markers | ||
| echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \ | ||
| sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \ | ||
| grep -v "^CONFLICT" >> all_conflicts.txt || true | ||
| # Also check for "changed in both" indicators | ||
| echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \ | ||
| grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true | ||
| # Remove duplicates and empty lines | ||
| sort -u all_conflicts.txt | grep -v "^$" > all_conflicts_unique.txt || touch all_conflicts_unique.txt | ||
| mv all_conflicts_unique.txt all_conflicts.txt |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
git merge-tree output format CONFLICT markers different types
💡 Result:
Short answer — format and marker types
-
git-merge-tree overall output: three sections — OID of toplevel tree, a Conflicted-file-info list (lines: " ") and an Informational-messages block. Use --name-only or -z to change quoting/NUL termination. [1]
-
Informational messages (human form) show conflict notices like "Auto-merging " or "CONFLICT (rename/delete): …". With -z they are structured records: NULNULNUL. is a stable short string such as "Auto-merging", "CONFLICT (rename/delete)", "CONFLICT (submodule lacks merge base)", "CONFLICT (binary)". These messages are intended for humans (non-stable text without -z). [1][2][3]
-
Conflict markers embedded in the toplevel tree file blobs:
-
Default (merge style):
...their side...
<<<<<<< (ours)
...your side...(other)
- diff3 style (merge.conflictStyle = diff3) adds the original/base section with a ||||||| marker between original and =======. [3]
-
-
Important parsing guidance:
- Do not try to infer conflict types only from the Conflicted-file-info lines; use the Informational-messages section (or -z structured records). Many logical conflict kinds exist (modify/delete, rename conflicts, binary, submodule, file/dir), and mappings can be many-to-many. [1][2]
- git-merge-tree documentation (OUTPUT, -z and informational messages). [1]
- manpage / Ubuntu docs explaining -z record format and example conflict-type strings. [2]
- git-merge / merge.conflictStyle docs describing <<<<<<< / ||||||| / ======= / >>>>>>> markers. [3]
- Normal (no conflicts): exit code 0. [1]
- Conflicts: exit code 1. [1]
- Error (could not start/complete): exit code ≠ 0 and ≠ 1 (unspecified). [1]
- When using --stdin: the process returns 0 for both clean and conflicted merges; a non-0/≠1 means some merges could not be completed. [1][2]
- Clean merge: single line containing the OID of the toplevel tree. [2]
- Conflicted merge: three parts in order:
- OID of toplevel tree (represents what would be checked out; conflicted files may contain conflict markers). [2]
- Conflicted file info: lines of " " (or just filenames with --name-only). [2]
- Informational messages: a blank line then free-form messages such as "Auto-merging " or "CONFLICT …" (these are not stable strings for machine parsing). [2][3]
- -z switches most separators/newlines to NULs and changes the informational-message encoding (records: NULNULNUL). [2]
- --name-only omits mode/object/stage and lists only filenames. [2]
- --[no-]messages controls whether the informational messages are emitted. [2]
- The OID is always emitted; when exit ≠ 0 the printed tree output contains more than just the tree OID. [1][2]
-
git-merge-tree outputs a toplevel tree OID; when there are conflicts it also emits a "Conflicted file info" section (mode, object, stage, filename) and an "Informational messages" block containing human-readable messages such as "CONFLICT (rename/delete): …" or "CONFLICT (binary)". These free-form messages are for humans and not stable for scripting; use the Conflicted file info section (stages) to determine file-level conflict state. [1][2]
-
Typical conflict message forms you’ll see in the informational block:
- "CONFLICT (rename/delete): renamed… but deleted in…"
- "Auto-merging "
These describe rename/delete, modify/delete, rename/rename, etc., but exact wording may change across Git versions. [1][2]
-
Practical notes:
- Do not try to discover conflict types by scanning the resultant tree; parse the Conflicted file info (stages) instead. Some tree-level conflicts (modify/delete, rename/delete, directory–file, rename/rename permutations) are not representable by simple conflict markers and require manual resolution. [1]
- Modify/delete (one side modified a file, the other deleted it) and rename/delete (one side renamed, the other deleted) are “tree/high-level” conflicts that generally require manual decision (keep modified/renamed version or accept deletion). Tools like git mergetool present choices (created vs deleted). [1][3]
- If you need machine-readable output, use git-merge-tree with -z and inspect the conflicted-file entries (modes/objects/stages); don’t rely on the human messages block. [1][2]
- git-merge-tree man page (output, conflicted file info, informational messages). [1]
- git-merge-tree examples / details showing "CONFLICT (rename/delete)" messaging. [2]
- Discussion / practical advice about modify/delete and rename/delete conflicts (resolving manually). [3]
- Check exit code:
git merge-treereturns exit code 1 when conflicts exist - Parse structured output: Use
git merge-tree -zto get machine-readable records with stable conflict-type strings - Examine conflicted-file info: The Conflicted-file-info section (mode/object/stage) directly indicates conflict state; stages 2 or 3 indicate conflicts
Zero-terminated / machine-friendly variants
Notes
Sources: Git documentation (git-merge-tree manpage) and upstream Git docs. [1][2][3]
🌐 Web query:
git merge-tree rename delete modify delete conflict messages documentation💡 Result:
Summary — how git-merge-tree reports rename/delete and modify/delete conflicts, and how to interpret them:
References
Use git merge-tree exit code or structured output instead of parsing informational messages.
The current approach of parsing the Informational-messages block (lines 132-138) is fundamentally unreliable. According to Git documentation, these messages are human-readable, non-stable strings intended for display only—not machine parsing. The exact wording varies across Git versions, making patterns like
grep "^CONFLICT"andgrep -oP "(?<=Auto-merging )\S+"fragile.Additionally, including
Auto-mergingfiles introduces false positives since auto-merged files aren't necessarily conflicted.Instead, use one of these more reliable approaches:
Sources
🌐 Web query:
git merge-tree exit code conflicts auto-merging output format
💡 Result:
Summary — exit codes
Output format (default)
| - name: Check all open PRs for conflicts | ||
| if: github.event_name == 'push' && steps.get-prs.outputs.has_prs == 'true' | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| echo "[CHECK-ALL] ========================================" | ||
| echo "[CHECK-ALL] Checking all open PRs for conflicts after merge to ${{ github.ref_name }}" | ||
| FAILED_PRS="" | ||
| FAILED_COUNT=0 | ||
| while IFS= read -r pr_number; do | ||
| echo "[CHECK-ALL] ----------------------------------------" | ||
| echo "[CHECK-ALL] Checking PR #${pr_number}" | ||
| # Get PR details | ||
| PR_DATA=$(gh pr view ${pr_number} --json headRefName,headRefOid,baseRefName,baseRefOid,title) | ||
| PR_TITLE=$(echo "${PR_DATA}" | jq -r '.title') | ||
| PR_HEAD_REF=$(echo "${PR_DATA}" | jq -r '.headRefName') | ||
| PR_HEAD_SHA=$(echo "${PR_DATA}" | jq -r '.headRefOid') | ||
| PR_BASE_REF=$(echo "${PR_DATA}" | jq -r '.baseRefName') | ||
| PR_BASE_SHA=$(echo "${PR_DATA}" | jq -r '.baseRefOid') | ||
| echo "[CHECK-ALL] PR #${pr_number}: ${PR_TITLE}" | ||
| echo "[CHECK-ALL] Base: ${PR_BASE_REF} @ ${PR_BASE_SHA}" | ||
| echo "[CHECK-ALL] Head: ${PR_HEAD_REF} @ ${PR_HEAD_SHA}" | ||
| # Fetch branches | ||
| git fetch origin ${PR_BASE_REF}:refs/remotes/origin/${PR_BASE_REF} 2>/dev/null || true | ||
| git fetch origin ${PR_HEAD_REF}:refs/remotes/origin/${PR_HEAD_REF} 2>/dev/null || true | ||
| # Get PR files | ||
| gh pr view ${pr_number} --json files --jq '.files[].path' > "pr_${pr_number}_files.txt" | ||
| FILE_COUNT=$(wc -l < "pr_${pr_number}_files.txt" | tr -d ' ') | ||
| if [ "${FILE_COUNT}" -eq 0 ]; then | ||
| echo "[CHECK-ALL] No files changed - skipping" | ||
| continue | ||
| fi | ||
| # Calculate merge-base | ||
| MERGE_BASE=$(git merge-base origin/${PR_BASE_REF} origin/${PR_HEAD_REF} 2>/dev/null || echo "") | ||
| if [ -z "${MERGE_BASE}" ]; then | ||
| echo "[CHECK-ALL] ⚠️ Could not calculate merge-base - skipping" | ||
| continue | ||
| fi | ||
| # Run merge-tree | ||
| set +e | ||
| MERGE_OUTPUT=$(git merge-tree ${MERGE_BASE} origin/${PR_BASE_REF} origin/${PR_HEAD_REF} 2>&1) | ||
| set -e | ||
| # Parse conflicts | ||
| touch "pr_${pr_number}_conflicts.txt" | ||
| echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \ | ||
| sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \ | ||
| grep -v "^CONFLICT" >> "pr_${pr_number}_conflicts.txt" || true | ||
| sort -u "pr_${pr_number}_conflicts.txt" | grep -v "^$" > "pr_${pr_number}_conflicts_unique.txt" || touch "pr_${pr_number}_conflicts_unique.txt" | ||
| mv "pr_${pr_number}_conflicts_unique.txt" "pr_${pr_number}_conflicts.txt" | ||
| # Filter to PR files | ||
| touch "pr_${pr_number}_filtered.txt" | ||
| if [ -f "pr_${pr_number}_conflicts.txt" ] && [ -s "pr_${pr_number}_conflicts.txt" ]; then | ||
| while IFS= read -r conflict_file; do | ||
| if grep -Fxq "${conflict_file}" "pr_${pr_number}_files.txt"; then | ||
| echo "${conflict_file}" >> "pr_${pr_number}_filtered.txt" | ||
| fi | ||
| done < "pr_${pr_number}_conflicts.txt" | ||
| fi | ||
| CONFLICT_COUNT=0 | ||
| if [ -f "pr_${pr_number}_filtered.txt" ] && [ -s "pr_${pr_number}_filtered.txt" ]; then | ||
| CONFLICT_COUNT=$(wc -l < "pr_${pr_number}_filtered.txt" | tr -d ' ') | ||
| fi | ||
| if [ "${CONFLICT_COUNT}" -gt 0 ]; then | ||
| echo "[CHECK-ALL] ❌ PR #${pr_number} has ${CONFLICT_COUNT} conflicted file(s)" | ||
| cat "pr_${pr_number}_filtered.txt" | while read file; do | ||
| echo "[CHECK-ALL] ⚠️ ${file}" | ||
| done | ||
| FAILED_PRS="${FAILED_PRS}${pr_number}," | ||
| FAILED_COUNT=$((FAILED_COUNT + 1)) | ||
| # Post comment on PR2 | ||
| gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected** | ||
| This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged. | ||
| **Conflicted files:** | ||
| $(cat "pr_${pr_number}_filtered.txt" | sed 's/^/- /') | ||
| **Action required:** Please update your branch to resolve these conflicts: | ||
| \`\`\`bash | ||
| git fetch origin | ||
| git checkout ${PR_HEAD_REF} | ||
| git merge origin/${PR_BASE_REF} | ||
| # Resolve conflicts, then: | ||
| git push | ||
| \`\`\`" | ||
| fi |
There was a problem hiding this comment.
Missing failure exit after detecting conflicts in push event.
The script tracks FAILED_COUNT but never uses it to exit with a non-zero status. After the loop completes, the script exits with 0 regardless of whether conflicts were found. The workflow will always appear as passed even when PRs have conflicts.
🐛 Proposed fix: Add exit status check after loop
Add after line 305 (after the done < pr_numbers.txt which closes the while loop):
fi
done < pr_numbers.txt
+
+ echo "[CHECK-ALL] ========================================"
+ echo "[CHECK-ALL] SUMMARY: ${FAILED_COUNT} PR(s) have merge conflicts"
+ echo "[CHECK-ALL] ========================================"
+
+ if [ "${FAILED_COUNT}" -gt 0 ]; then
+ echo "[CHECK-ALL] ❌ Workflow failed due to merge conflicts"
+ exit 1
+ else
+ echo "[CHECK-ALL] ✅ All PRs are conflict-free"
+ exit 0
+ fi🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 204 - 305, The loop
detects conflicts but never fails the job; after the while loop that reads
pr_numbers.txt, add a check that if FAILED_COUNT (or FAILED_PRS) is greater than
0 then print a concise summary (e.g., list FAILED_PRS and FAILED_COUNT) and exit
with a non‑zero status (exit 1) so the workflow fails on push when conflicts are
found; ensure this check runs only in the push-context branch of the job that
uses FAILED_COUNT/FAILED_PRS.
| # Post comment on PR2 | ||
| gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected** | ||
| This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged. | ||
| **Conflicted files:** | ||
| $(cat "pr_${pr_number}_filtered.txt" | sed 's/^/- /') | ||
| **Action required:** Please update your branch to resolve these conflicts: | ||
| \`\`\`bash | ||
| git fetch origin | ||
| git checkout ${PR_HEAD_REF} | ||
| git merge origin/${PR_BASE_REF} | ||
| # Resolve conflicts, then: | ||
| git push | ||
| \`\`\`" | ||
| fi |
There was a problem hiding this comment.
Comment body has inconsistent indentation.
The heredoc content starting at line 291 has leading whitespace that will appear in the rendered PR comment, causing irregular formatting. This affects readability for contributors.
♻️ Proposed fix: Remove leading whitespace from heredoc
# Post comment on PR
- gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected**
-
- This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged.
-
- **Conflicted files:**
- $(cat "pr_${pr_number}_filtered.txt" | sed 's/^/- /')
-
- **Action required:** Please update your branch to resolve these conflicts:
- \`\`\`bash
- git fetch origin
- git checkout ${PR_HEAD_REF}
- git merge origin/${PR_BASE_REF}
- # Resolve conflicts, then:
- git push
- \`\`\`"
+ gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected**
+
+This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged.
+
+**Conflicted files:**
+$(cat "pr_${pr_number}_filtered.txt" | sed 's/^/- /')
+
+**Action required:** Please update your branch to resolve these conflicts:
+\`\`\`bash
+git fetch origin
+git checkout ${PR_HEAD_REF}
+git merge origin/${PR_BASE_REF}
+# Resolve conflicts, then:
+git push
+\`\`\`"
fi🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 289 - 305, The PR
comment body passed to gh pr comment ${pr_number} --body contains leading
indentation that will render in the GitHub comment; remove that by replacing the
indented inline multiline string with a dedented heredoc or by passing a
left-aligned body file so all lines (the warning, code block and the $(cat
"pr_${pr_number}_filtered.txt" | sed ...) list) start at column 0; update the
call site (gh pr comment ${pr_number} --body) to use the non-indented heredoc or
--body-file approach so the rendered comment has no extra leading whitespace.
|
@palisadoes |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/workflows/pull-request-target.yml:
- Around line 257-264: The single-PR conflict check is including "Auto-merging"
lines while this multi-PR block parses only lines starting with "CONFLICT";
update the single-PR check to use the same parsing pipeline as the shown block:
read from MERGE_OUTPUT, grep only "^CONFLICT", use sed -E 's/^CONFLICT
\([^)]+\): Merge conflict in (.+)$/\1/' to extract file paths, filter empty
lines, sort -u and write to the pr_<number>_conflicts file; remove any grep or
pattern that matches "Auto-merging" so both checks behave consistently.
- Line 289: Remove or clarify the leftover debug comment "Post comment on PR2"
in the GitHub Actions workflow: either delete that line entirely or replace it
with a meaningful description of the step’s purpose so the job/comment
accurately reflects its behavior and isn't confusing to future readers.
- Around line 30-49: The inline script in the "Log environment" step directly
interpolates the user-controlled PR title (github.event.pull_request.title),
which allows shell injection; change the step to pass untrusted inputs via
environment variables (e.g., set env: PR_TITLE: ${{
github.event.pull_request.title }}) and reference the variable inside the script
(use "$PR_TITLE") instead of embedding the expression inline; ensure any other
PR fields that could be user-controlled are passed via env vars as well and
always quoted when used in the shell.
- Around line 136-138: The current conflict detection greps for "Auto-merging"
and a mistaken "^\+<<<<<" pattern, causing false positives; update the logic to
drop the "Auto-merging" match and use the correct merge conflict marker pattern
(e.g., "^<<<<<<<") against MERGE_OUTPUT before appending filenames to
all_conflicts.txt so only real conflict indicators are captured.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request-target.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/pull-request-target.yml
31-31: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
86-86: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Test Application (Shard 10)
- GitHub Check: Test Application (Shard 2)
- GitHub Check: Test Application (Shard 12)
- GitHub Check: Test Application (Shard 6)
- GitHub Check: Test Application (Shard 8)
- GitHub Check: Test Application (Shard 9)
- GitHub Check: Test Application (Shard 11)
- GitHub Check: Test Application (Shard 7)
- GitHub Check: Test Application (Shard 3)
- GitHub Check: Test Application (Shard 4)
- GitHub Check: Test Application (Shard 5)
- GitHub Check: Test Application (Shard 1)
🔇 Additional comments (4)
.github/workflows/pull-request-target.yml (4)
24-28: Security consideration: Checkout of PR head withpull_request_target.The workflow uses
pull_request_targetand checks outgithub.event.pull_request.head.sha, which is the PR author's potentially untrusted code. While the workflow only runs git/gh CLI commands and doesn't execute any code from the checkout, ensure this pattern remains safe as the workflow evolves.The current implementation is acceptable since:
- No scripts from the checked-out code are executed
- Only git metadata operations are performed
51-80: LGTM!The step correctly fetches open PRs targeting the base branch and handles the empty case appropriately. The use of
gh pr listwith JSON output andjqfor parsing is clean.
204-304: Missing workflow failure when conflicts are detected on push.The "Check all open PRs for conflicts" step tracks
FAILED_COUNTbut never uses it to fail the workflow. The variablesFAILED_PRSandFAILED_COUNTare computed but the step always exits successfully, even when conflicts are found. Consider whether this is intentional (comment-only notification) or if the workflow should also report a summary/failure.If the intent is to also produce a summary or fail when any PR has conflicts:
💡 Suggested addition at the end of the step
+ else + echo "[CHECK-ALL] ✅ PR #${pr_number} has no conflicts" fi done < pr_numbers.txt + echo "[CHECK-ALL] ========================================" + echo "[CHECK-ALL] SUMMARY: ${FAILED_COUNT} PR(s) with conflicts" + if [ "${FAILED_COUNT}" -gt 0 ]; then + echo "[CHECK-ALL] PRs with conflicts: ${FAILED_PRS}" + fi + echo "[CHECK-ALL] ========================================"
290-304: > Likely an incorrect or invalid review comment.
| # Parse conflicts | ||
| touch "pr_${pr_number}_conflicts.txt" | ||
| echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \ | ||
| sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \ | ||
| grep -v "^CONFLICT" >> "pr_${pr_number}_conflicts.txt" || true | ||
| sort -u "pr_${pr_number}_conflicts.txt" | grep -v "^$" > "pr_${pr_number}_conflicts_unique.txt" || touch "pr_${pr_number}_conflicts_unique.txt" | ||
| mv "pr_${pr_number}_conflicts_unique.txt" "pr_${pr_number}_conflicts.txt" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Conflict parsing is correct here but inconsistent with single PR check.
This step correctly parses only CONFLICT lines. However, the "Check single PR for conflicts" step (lines 136-138) incorrectly includes Auto-merging lines. Align both implementations for consistency.
🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 257 - 264, The
single-PR conflict check is including "Auto-merging" lines while this multi-PR
block parses only lines starting with "CONFLICT"; update the single-PR check to
use the same parsing pipeline as the shown block: read from MERGE_OUTPUT, grep
only "^CONFLICT", use sed -E 's/^CONFLICT \([^)]+\): Merge conflict in
(.+)$/\1/' to extract file paths, filter empty lines, sort -u and write to the
pr_<number>_conflicts file; remove any grep or pattern that matches
"Auto-merging" so both checks behave consistently.
|
@coderabbitai all suggestions have been fixed. |
|
@yashgoyal0110: Thank you for addressing the suggestions! I'll review the changes now, focusing on significant issues while ignoring minor fixes. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/workflows/pull-request-target.yml:
- Around line 268-270: The current grep that looks for "Auto-merging" yields
false positives by flagging successful auto-merged files; remove that check and
instead detect real conflicts by matching Git's CONFLICT output—replace the
second grep with something like grepping MERGE_OUTPUT for "CONFLICT" / "Merge
conflict in" and extract filenames (e.g. use grep -oP "(?<=Merge conflict in
)\S+" >> "pr_${pr_number}_conflicts.txt") while keeping the existing check for
conflict markers (the +<<<<</^<<<<<<< extraction) so only true conflicts are
recorded.
- Around line 199-202: Replace direct interpolation of
github.event.pull_request.head.ref/base.ref in the shell script with environment
variables to avoid script-injection risk: add env entries (e.g., PR_HEAD_REF and
PR_BASE_REF) for ${{ github.event.pull_request.head.ref }} and ${{
github.event.pull_request.base.ref }} in the job/step, then update the
echo/merge lines that reference head.ref and base.ref to use those env vars
(ensuring they are quoted/escaped when used) instead of interpolating the event
values directly.
- Around line 38-46: The PR title is being injected directly into the shell via
PR_TITLE='${{ github.event.pull_request.title }}', creating a command injection
risk; instead write the title into the Actions environment and use a quoted
variable expansion. Replace the direct assignment with echo "PR_TITLE=${{
github.event.pull_request.title }}" >> $GITHUB_ENV (or equivalent GITHUB_ENV
handling), then print it using printf "[SETUP] PR Title: %s\n" "$PR_TITLE" (note
the double quotes around $PR_TITLE) to avoid word-splitting and execution; keep
the other echo lines unchanged.
- Around line 137-139: The current logic appends filenames matched by
Auto-merging to all_conflicts.txt causing false positives; remove the
Auto-merging check and only capture actual conflict indicators (e.g., the +<<<<<
conflict marker or explicit CONFLICT lines) from MERGE_OUTPUT, i.e., stop
grepping for "Auto-merging" and update the pipeline that writes to
all_conflicts.txt to only extract filenames when real conflict markers are
present (use the existing /^\+<<<<</ or other explicit CONFLICT patterns on
MERGE_OUTPUT instead of Auto-merging).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request-target.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/pull-request-target.yml
31-31: "github.event.pull_request.title" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
87-87: "github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Performs linting, formatting, type-checking, unused file detection, checking for different source...
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
.github/workflows/pull-request-target.yml (3)
3-12: LGTM on trigger configuration and permissions.The
pull_request_targettrigger with the specified event types aligns with the requirements from issue #6364, andpushto develop enables re-checking open PRs after merges. Permissions are appropriately scoped for reading contents and writing PR comments/checks.
24-28: Checkout configuration is appropriate for the use case.Full history fetch is required for
git merge-treeoperations. While checking outpull_request.head.shawithpull_request_targetnormally raises security concerns, this workflow only runs git/gh CLI commands and doesn't execute any code from the checked-out repository, making this safe.
52-81: LGTM!The step correctly fetches open PRs targeting the base branch using GitHub CLI and sets the appropriate output for conditional execution of the batch check step.
| if [ "${{ github.event_name }}" = "pull_request_target" ]; then | ||
| echo "[SETUP] PR Number: ${{ github.event.pull_request.number }}" | ||
| PR_TITLE='${{ github.event.pull_request.title }}' | ||
| printf "[SETUP] PR Title: %s\n" "${PR_TITLE}" | ||
| echo "[SETUP] Base Branch: ${{ github.event.pull_request.base.ref }}" | ||
| echo "[SETUP] Base SHA: ${{ github.event.pull_request.base.sha }}" | ||
| echo "[SETUP] Head Branch: ${{ github.event.pull_request.head.ref }}" | ||
| echo "[SETUP] Head SHA: ${{ github.event.pull_request.head.sha }}" | ||
| fi |
There was a problem hiding this comment.
Script injection vulnerability via PR title.
The PR title is directly interpolated into the shell script, which allows an attacker to inject arbitrary shell commands through a malicious PR title (e.g., '; malicious_command #).
Pass untrusted inputs through environment variables instead:
🔒 Proposed fix
- name: Log environment
+ env:
+ PR_TITLE: ${{ github.event.pull_request.title }}
run: |
echo "[SETUP] ========================================"
echo "[SETUP] Workflow triggered by: ${{ github.event_name }}"
# ... other lines ...
if [ "${{ github.event_name }}" = "pull_request_target" ]; then
echo "[SETUP] PR Number: ${{ github.event.pull_request.number }}"
- PR_TITLE='${{ github.event.pull_request.title }}'
- printf "[SETUP] PR Title: %s\n" "${PR_TITLE}"
+ printf "[SETUP] PR Title: %s\n" "${PR_TITLE}"
# ... rest of the block ...
fi🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 38 - 46, The PR title
is being injected directly into the shell via PR_TITLE='${{
github.event.pull_request.title }}', creating a command injection risk; instead
write the title into the Actions environment and use a quoted variable
expansion. Replace the direct assignment with echo "PR_TITLE=${{
github.event.pull_request.title }}" >> $GITHUB_ENV (or equivalent GITHUB_ENV
handling), then print it using printf "[SETUP] PR Title: %s\n" "$PR_TITLE" (note
the double quotes around $PR_TITLE) to avoid word-splitting and execution; keep
the other echo lines unchanged.
| # Also check for "changed in both" indicators | ||
| echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \ | ||
| grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true |
There was a problem hiding this comment.
False positive detection from Auto-merging lines.
Auto-merging in git merge-tree output indicates that git is attempting to merge a file, not that a conflict occurred. Successful auto-merges will not produce a CONFLICT line. Grepping for Auto-merging will incorrectly flag files that merged cleanly.
Remove this logic to avoid false positives:
🐛 Proposed fix
# Look for CONFLICT markers
echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \
sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \
grep -v "^CONFLICT" >> all_conflicts.txt || true
-
- # Also check for "changed in both" indicators
- echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \
- grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Also check for "changed in both" indicators | |
| echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \ | |
| grep -oP "(?<=Auto-merging )\S+" >> all_conflicts.txt || true |
🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 137 - 139, The
current logic appends filenames matched by Auto-merging to all_conflicts.txt
causing false positives; remove the Auto-merging check and only capture actual
conflict indicators (e.g., the +<<<<< conflict marker or explicit CONFLICT
lines) from MERGE_OUTPUT, i.e., stop grepping for "Auto-merging" and update the
pipeline that writes to all_conflicts.txt to only extract filenames when real
conflict markers are present (use the existing /^\+<<<<</ or other explicit
CONFLICT patterns on MERGE_OUTPUT instead of Auto-merging).
| echo "[RESULT] Please update your branch to resolve these conflicts:" | ||
| echo "[RESULT] git fetch origin" | ||
| echo "[RESULT] git checkout ${{ github.event.pull_request.head.ref }}" | ||
| echo "[RESULT] git merge origin/${{ github.event.pull_request.base.ref }}" |
There was a problem hiding this comment.
Script injection risk with head.ref in shell script.
While this is inside an echo statement making exploitation harder, directly interpolating github.event.pull_request.head.ref in shell scripts is flagged by static analysis as a security risk. A crafted branch name could potentially break the script logic.
Pass it through an environment variable:
🔒 Proposed fix
Update the step's env section:
- name: Check single PR for conflicts
if: github.event_name == 'pull_request_target'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ PR_HEAD_REF: ${{ github.event.pull_request.head.ref }}
+ PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
run: |Then update the references in the script:
- echo "[RESULT] git checkout ${{ github.event.pull_request.head.ref }}"
- echo "[RESULT] git merge origin/${{ github.event.pull_request.base.ref }}"
+ echo "[RESULT] git checkout ${PR_HEAD_REF}"
+ echo "[RESULT] git merge origin/${PR_BASE_REF}"🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 199 - 202, Replace
direct interpolation of github.event.pull_request.head.ref/base.ref in the shell
script with environment variables to avoid script-injection risk: add env
entries (e.g., PR_HEAD_REF and PR_BASE_REF) for ${{
github.event.pull_request.head.ref }} and ${{ github.event.pull_request.base.ref
}} in the job/step, then update the echo/merge lines that reference head.ref and
base.ref to use those env vars (ensuring they are quoted/escaped when used)
instead of interpolating the event values directly.
| # Also check for "changed in both" indicators (consistent with single PR check) | ||
| echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \ | ||
| grep -oP "(?<=Auto-merging )\S+" >> "pr_${pr_number}_conflicts.txt" || true |
There was a problem hiding this comment.
Same false positive issue with Auto-merging detection.
Consistent with the earlier comment, this grep for Auto-merging will cause false positives by flagging successfully auto-merged files as conflicts.
🐛 Proposed fix
# Look for CONFLICT markers
echo "${MERGE_OUTPUT}" | grep "^CONFLICT" | \
sed -E 's/^CONFLICT \([^)]+\): Merge conflict in (.+)$/\1/' | \
grep -v "^CONFLICT" >> "pr_${pr_number}_conflicts.txt" || true
-
- # Also check for "changed in both" indicators (consistent with single PR check)
- echo "${MERGE_OUTPUT}" | grep -E "^\+<<<<<|^Auto-merging" | \
- grep -oP "(?<=Auto-merging )\S+" >> "pr_${pr_number}_conflicts.txt" || true🤖 Prompt for AI Agents
In @.github/workflows/pull-request-target.yml around lines 268 - 270, The
current grep that looks for "Auto-merging" yields false positives by flagging
successful auto-merged files; remove that check and instead detect real
conflicts by matching Git's CONFLICT output—replace the second grep with
something like grepping MERGE_OUTPUT for "CONFLICT" / "Merge conflict in" and
extract filenames (e.g. use grep -oP "(?<=Merge conflict in )\S+" >>
"pr_${pr_number}_conflicts.txt") while keeping the existing check for conflict
markers (the +<<<<</^<<<<<<< extraction) so only true conflicts are recorded.
| gh pr comment ${pr_number} --body "⚠️ **Merge Conflict Detected** | ||
| This PR has merge conflicts with the base branch \`${PR_BASE_REF}\` after recent changes were merged. | ||
| **Conflicted files:** | ||
| ${CONFLICT_LIST} | ||
| **Action required:** Please update your branch to resolve these conflicts: | ||
| \`\`\`bash | ||
| git fetch origin | ||
| git checkout ${PR_HEAD_REF} | ||
| git merge origin/${PR_BASE_REF} | ||
| # Resolve conflicts, then: | ||
| git push | ||
| \`\`\`" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, find the workflow file and examine the context around lines 302-316
find . -name "pull-request-target.yml" -path "*/.github/workflows/*"Repository: PalisadoesFoundation/talawa-admin
Length of output: 120
🏁 Script executed:
# Once we have the file, let's read it to understand the full context
wc -l .github/workflows/pull-request-target.ymlRepository: PalisadoesFoundation/talawa-admin
Length of output: 122
🏁 Script executed:
# Read the relevant section and surrounding context to understand how conflicts are detected and comments are posted
sed -n '280,330p' .github/workflows/pull-request-target.yml | cat -nRepository: PalisadoesFoundation/talawa-admin
Length of output: 2637
🏁 Script executed:
# Check the full structure of the conflict detection and comment posting logic
sed -n '200,350p' .github/workflows/pull-request-target.yml | cat -nRepository: PalisadoesFoundation/talawa-admin
Length of output: 7280
🏁 Script executed:
# Search for patterns in other workflows that handle duplicate comments or check for existing comments
rg "gh pr comment" .github/workflows/ -A 10 -B 5Repository: PalisadoesFoundation/talawa-admin
Length of output: 1442
🏁 Script executed:
# Check if there are any other references to duplicate prevention or comment updates
rg -i "duplicate|existing.*comment|update.*comment" .github/workflows/Repository: PalisadoesFoundation/talawa-admin
Length of output: 227
🏁 Script executed:
# Check if there's any unique identifier or pattern matching for existing comments in the workflow
rg "CONFLICT_LIST|Merge Conflict Detected" .github/workflows/pull-request-target.yml -B 5 -A 5Repository: PalisadoesFoundation/talawa-admin
Length of output: 892
🏁 Script executed:
# Check the entire workflow step to understand the trigger conditions
sed -n '1,50p' .github/workflows/pull-request-target.yml | cat -nRepository: PalisadoesFoundation/talawa-admin
Length of output: 2231
🏁 Script executed:
# Look for any comment body patterns that might help identify duplicates
rg -i "comment.*body|query|jq.*comment" .github/workflows/Repository: PalisadoesFoundation/talawa-admin
Length of output: 538
🏁 Script executed:
# Check git log or recent changes to see if duplicate-comment handling was considered
git log --oneline -20 -- .github/workflows/pull-request-target.yml 2>/dev/null | head -20Repository: PalisadoesFoundation/talawa-admin
Length of output: 106
Consider preventing duplicate comments on already-conflicted PRs.
The code unconditionally posts a new conflict comment whenever conflicts are detected (gh pr comment inside the if [ "${CONFLICT_COUNT}" -gt 0 ] block). Since this workflow runs on every push to the develop branch, multiple workflow executions will post duplicate comments on the same PR. There is no mechanism to check for or update an existing conflict comment before posting a new one.
Prevent duplicate notifications by either checking for an existing conflict comment before posting, or updating the existing comment instead of creating a new one.
|
Have you tried this on your home repo with two separate PRs? |
@palisadoes 
I have tested, on my personal repo. working fine on base branch push
|
2 participants
What kind of change does this PR introduce?
feature
fixes: #6364
Summary
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.