-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: enhance conflict prediction workflow with PR comments #6758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: enhance conflict prediction workflow with PR comments #6758
Conversation
|
This pull request has conflicts, please rebase. |
|
See PastaPastaPasta#35; it'll update this comment over time as new runs occur. Only 1 comment will ever be posted |
WalkthroughThe updates enhance the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/handle_potential_conflicts.py(1 hunks).github/workflows/predict-conflicts.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to .github/** : Unless specifically prompted, avoid making changes to .github (GitHub workflows and configs)
Learnt from: kwvg
PR: dashpay/dash#6421
File: .github/workflows/build-depends.yml:80-85
Timestamp: 2025-02-17T10:21:55.551Z
Learning: In GitHub Actions workflow files, when using the `env` command with `DEP_OPTS` or similar variables containing space-delimited key-value pairs for environment variables, word splitting is desired behavior and the variables should not be quoted.
.github/workflows/predict-conflicts.yml (1)
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to .github/** : Unless specifically prompted, avoid making changes to .github (GitHub workflows and configs)
.github/workflows/handle_potential_conflicts.py (1)
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
🪛 actionlint (1.7.7)
.github/workflows/predict-conflicts.yml
31-31: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (9)
- GitHub Check: mac-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (8)
.github/workflows/handle_potential_conflicts.py (6)
22-33: Good error handling for missing dependencies.The enhanced import handling with clear error messages improves the developer experience.
34-55: Excellent refactoring of PR data fetching.The function now properly handles:
- Repository configuration via environment variable
- Network errors with requests.RequestException
- JSON parsing errors
- Invalid API responses
This makes the script more robust and testable on forks.
56-72: Well-implemented GitHub Actions output handler.The function correctly handles both single-line and multiline outputs using GitHub's delimiter syntax, with proper error handling.
78-93: Robust input validation added.Good defensive programming with clear error messages for missing required fields.
94-107: Proper error handling for main PR data.The code correctly validates the PR data structure before accessing nested fields.
166-181: Clean implementation of GitHub Actions outputs.The outputs are well-structured for the workflow to consume:
has_conflicts: Boolean for conditional stepsconflict_details: Markdown-formatted list for PR commentsconflicting_prs: Comma-separated list for reference.github/workflows/predict-conflicts.yml (2)
26-26: Proper step IDs for output references.Adding IDs enables the workflow to reference outputs from these steps in conditional logic.
Also applies to: 33-33
36-57: Well-designed PR comment system.The comment posting logic:
- Uses
message-idto prevent duplicate comments- Provides clear status messages for both conflict and no-conflict scenarios
- Automatically updates/replaces previous comments
- Modified handle_potential_conflicts.py to output structured data via GitHub Actions outputs - Added PR comment functionality to predict-conflicts.yml workflow - Comments are updateable (sticky) using message-id to prevent duplicates - Shows list of conflicting PRs with links when conflicts are detected - Removes comment when no conflicts exist - Improved error handling and validation in Python script Co-Authored-By: Claude <noreply@anthropic.com>
- Changed get_pr_json to return None on API errors instead of exiting - Added proper None checks before accessing PR data - Skip PRs that fail to fetch instead of crashing the entire workflow - This fixes the KeyError when PR API responses don't have expected fields
- Use GITHUB_REPOSITORY environment variable for API calls - Defaults to dashpay/dash if not set - Fixes testing on forked repositories like PastaPastaPasta/dash
- Print the exact URL being checked - Show more of the response when unexpected content is received - This will help diagnose why GitHub returns different responses
- Show total response length - Search for conflict text and show its position - Display context around the conflict message
- Look for 'Can't automatic' partial text to handle truncated responses - Also check for 'octicon octicon-x' which is the conflict indicator icon - This handles cases where the exact text format varies
- Remove verbose debug output for inputs and PR checking - Keep only essential error and warning messages - Streamline output for production use
- Use repository from environment variable for conflict PR URLs - Update checkout action from v3 to v4 for latest features
9fadf45 to
e0515b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/handle_potential_conflicts.py (1)
139-141: Eliminate redundant repository variable definition.The
repovariable is defined twice (lines 36 and 140), which is redundant and could lead to maintenance issues.Consider defining the repository variable once at the module level or passing it as a parameter:
+# Get repository from environment at module level +REPO = os.environ.get('GITHUB_REPOSITORY', 'dashpay/dash') def get_pr_json(pr_num): - # Get repository from environment or default to dashpay/dash - repo = os.environ.get('GITHUB_REPOSITORY', 'dashpay/dash') - try: - response = requests.get(f'https://api.github.com/repos/{repo}/pulls/{pr_num}') + response = requests.get(f'https://api.github.com/repos/{REPO}/pulls/{pr_num}')And update the usage in the main function:
- # Get repository from environment - repo = os.environ.get('GITHUB_REPOSITORY', 'dashpay/dash') - merge_check_url = f'https://github.com/{repo}/branches/pre_mergeable/{our_pr_label}...{conflict_pr_label}' + merge_check_url = f'https://github.com/{REPO}/branches/pre_mergeable/{our_pr_label}...{conflict_pr_label}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/handle_potential_conflicts.py(1 hunks).github/workflows/predict-conflicts.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/predict-conflicts.yml
🧰 Additional context used
📓 Path-based instructions (1)
.github/**
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to .github/** : Unless specifically prompted, avoid making changes to .github (GitHub workflows and configs)
.github/workflows/handle_potential_conflicts.py (2)
Learnt from: kwvg
PR: dashpay/dash#6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.
Learnt from: CR
PR: dashpay/dash#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-10T16:26:00.371Z
Learning: Applies to .github/** : Unless specifically prompted, avoid making changes to .github (GitHub workflows and configs)
⏰ 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). (9)
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
🔇 Additional comments (8)
.github/workflows/handle_potential_conflicts.py (8)
22-24: LGTM: Additional imports are appropriate for the enhanced functionality.The new imports (
os,json,uuid) are necessary for the GitHub Actions integration and environment variable handling.
28-32: LGTM: Excellent error handling for missing dependencies.The try-except block provides clear feedback when the
hjsonmodule is missing, which improves the user experience during setup.
35-54: LGTM: Robust error handling and environment variable integration.The function now properly handles HTTP errors, JSON parsing errors, and API error responses while dynamically detecting the repository from environment variables.
56-71: LGTM: Well-implemented GitHub Actions output handling.The function properly handles multiline values using delimiter syntax and includes appropriate error handling for missing environment variables.
78-104: LGTM: Comprehensive input validation and error handling.The enhanced validation ensures required fields are present and provides clear error messages, improving the robustness of the script.
112-138: LGTM: Improved conflict processing with better validation.The enhanced loop includes proper validation of conflict entries, skipping invalid or draft PRs, and provides informative warning messages.
150-162: LGTM: Improved conflict detection with better pattern matching.The enhanced logic handles partial text matching and icon detection, with appropriate warning messages for unexpected responses.
166-180: LGTM: Well-structured GitHub Actions integration.The output handling properly formats conflict details as markdown and sets all necessary outputs for the workflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR automates posting and updating GitHub PR comments when potential merge conflicts are detected.
- Enhances the workflow to validate conflicts, post sticky comments, and fail CI on detected conflicts.
- Updates the conflict-handling script to fetch PR data dynamically, parse mergeability, and set Actions outputs.
- Improves error handling for missing data and API failures.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/predict-conflicts.yml | Updated checkout action; added validate, comment-posting, and fail steps |
| .github/workflows/handle_potential_conflicts.py | Refactored input parsing, updated PR fetch logic, conflict detection, and output handling |
Comments suppressed due to low confidence (4)
.github/workflows/predict-conflicts.yml:34
- The run step is passing
$conflictswhich isn't defined in the shell. It should reference the output of thecheck_conflictsstep using${{ steps.check_conflicts.outputs.conflicts }}.
run: pip3 install hjson && .github/workflows/handle_potential_conflicts.py "$conflicts"
.github/workflows/predict-conflicts.yml:34
- The script is invoked directly without an interpreter. Either add a shebang and make it executable, or prefix the command with
python3to ensure it runs correctly.
run: pip3 install hjson && .github/workflows/handle_potential_conflicts.py "$conflicts"
.github/workflows/handle_potential_conflicts.py:19
- The module-level docstring is empty. Add a brief description of the script's purpose, inputs, and outputs to aid maintainability.
"""
.github/workflows/handle_potential_conflicts.py:112
- The conflict detection loop introduces significant logic without accompanying tests. Consider adding unit tests to verify behavior for various inputs, including skipped entries and mergeability cases.
for conflict in conflict_prs:
|
@PastaPastaPasta can you show example (screenshot or text comment) how it works now? |
This fixes the lint-python-utf8-encoding.py linter failure by explicitly specifying encoding='utf8' when opening the GITHUB_OUTPUT file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 538d88d
Summary
This PR enhances the conflict prediction workflow to automatically post comments on pull requests when potential merge conflicts are detected with other open PRs.
Changes
1. Enhanced Python Script (
handle_potential_conflicts.py)GITHUB_REPOSITORYenvironment variable (supports testing on forks)2. Updated Workflow (
predict-conflicts.yml)mshick/add-pr-comment@v2message-idto prevent duplicates3. Bug Fixes
Example Comment Output
When conflicts are detected:
When no conflicts exist:
Testing
This has been tested on the PastaPastaPasta/dash fork with multiple test PRs to verify:
Benefits