-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Auto-merge non-breaking vendor updates with improved messaging #3059
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
…ates Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
|
@copilot Please fix the problem (https://github.com/cmderdev/cmder/actions/runs/20217095726/job/58031954039#step:9:54) |
Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
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 enhances the vendor dependency update workflow with intelligent semantic versioning analysis, improved messaging with emojis, and automatic merging capabilities for non-breaking changes. The goal is to reduce manual intervention for safe updates while maintaining proper review processes for potentially breaking changes.
Key changes:
- Added semantic version parsing to detect major/minor/patch updates across both PowerShell script and GitHub Actions workflow
- Implemented auto-merge feature that directly pushes minor/patch updates to master, bypassing PR creation for safe changes
- Enhanced user-facing messages with context-aware formatting (single vs. multiple dependencies) and visual emoji indicators (🔥 for major, 🚀 for minor, ⬆️ for patch)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| scripts/update.ps1 | Adds semantic version analysis logic, exports breaking change flags and update details to GitHub Actions environment |
| .github/workflows/vendor.yml | Implements auto-merge workflow step, improves PR title/body formatting with conditional logic for single vs. multiple dependencies, adds collapsible changelog sections for major updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Auto-merge minor updates | ||
| if: env.COUNT_UPDATED > 0 && env.HAS_BREAKING_CHANGES != 'True' | ||
| shell: pwsh | ||
| run: | | ||
| try { | ||
| echo "### 🚀 Auto-merging Updates" >> $env:GITHUB_STEP_SUMMARY | ||
| echo "" >> $env:GITHUB_STEP_SUMMARY | ||
| echo "Attempting to automatically merge non-breaking changes to master..." >> $env:GITHUB_STEP_SUMMARY | ||
| git config --global user.name "github-actions[bot]" | ||
| git config --global user.email "github-actions[bot]@users.noreply.github.com" | ||
| # Commit the changes | ||
| git add vendor/sources.json | ||
| $commitResult = git commit -m "⬆️ Update dependencies ($env:LIST_UPDATED)" | ||
| # Push directly to master | ||
| git push origin HEAD:master |
Copilot
AI
Dec 15, 2025
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.
The auto-merge feature directly pushes to the master branch without any approval or security checks. This could be a security risk if a malicious dependency update is introduced. Consider adding validation steps before auto-merging, such as running tests or security scans, even for minor/patch updates.
| # If semantic versioning fails, treat as unknown (potentially breaking) | ||
| $changeType = "unknown" | ||
| $hasBreakingChanges = $true | ||
| Write-Verbose "Could not parse version as semantic version, treating as potentially breaking" |
Copilot
AI
Dec 15, 2025
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.
When version parsing fails and falls into the catch block, the code sets $hasBreakingChanges = $true as a safety measure. However, this means that any dependency with a non-standard version format will always require manual review, even if it's actually a safe patch update. Consider logging which specific dependency failed to parse to help with debugging and potential future improvements to the version parsing logic.
| Write-Verbose "Could not parse version as semantic version, treating as potentially breaking" | |
| Write-Verbose "Could not parse version as semantic version for dependency '$($s.name)' (old: '$($s.version)', new: '$version'), treating as potentially breaking" |
| - name: Auto-merge minor updates | ||
| if: env.COUNT_UPDATED > 0 && env.HAS_BREAKING_CHANGES != 'True' | ||
| shell: pwsh | ||
| run: | | ||
| try { | ||
| echo "### 🚀 Auto-merging Updates" >> $env:GITHUB_STEP_SUMMARY | ||
| echo "" >> $env:GITHUB_STEP_SUMMARY | ||
| echo "Attempting to automatically merge non-breaking changes to master..." >> $env:GITHUB_STEP_SUMMARY | ||
| git config --global user.name "github-actions[bot]" | ||
| git config --global user.email "github-actions[bot]@users.noreply.github.com" | ||
| # Commit the changes | ||
| git add vendor/sources.json | ||
| $commitResult = git commit -m "⬆️ Update dependencies ($env:LIST_UPDATED)" | ||
| # Push directly to master | ||
| git push origin HEAD:master |
Copilot
AI
Dec 15, 2025
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.
There's a potential race condition if multiple workflow runs execute concurrently. If two workflows try to push to master at the same time, one will fail. While this is handled by the error catch block, it would be better to use a concurrency group to prevent multiple instances of this workflow from running simultaneously on the same branch.
| # Push directly to master | ||
| git push origin HEAD:master | ||
| echo "" >> $env:GITHUB_STEP_SUMMARY | ||
| echo "✅ **Success!** Updates have been automatically merged to master." >> $env:GITHUB_STEP_SUMMARY | ||
| echo "" >> $env:GITHUB_STEP_SUMMARY | ||
| echo "**Updated dependencies:** $env:LIST_UPDATED" >> $env:GITHUB_STEP_SUMMARY | ||
| # Set a flag to skip PR creation | ||
| echo "AUTO_MERGED=true" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 |
Copilot
AI
Dec 15, 2025
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.
The $commitResult variable captures the output of git commit, but it's only used in a check on line 224. However, if the commit fails (e.g., no changes to commit), the script will proceed to try to push, which could fail. Consider checking if the commit succeeded before attempting to push, or use -ErrorAction Stop to halt on errors.
| # Push directly to master | |
| git push origin HEAD:master | |
| echo "" >> $env:GITHUB_STEP_SUMMARY | |
| echo "✅ **Success!** Updates have been automatically merged to master." >> $env:GITHUB_STEP_SUMMARY | |
| echo "" >> $env:GITHUB_STEP_SUMMARY | |
| echo "**Updated dependencies:** $env:LIST_UPDATED" >> $env:GITHUB_STEP_SUMMARY | |
| # Set a flag to skip PR creation | |
| echo "AUTO_MERGED=true" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 | |
| if ($commitResult) { | |
| # Push directly to master | |
| git push origin HEAD:master | |
| echo "" >> $env:GITHUB_STEP_SUMMARY | |
| echo "✅ **Success!** Updates have been automatically merged to master." >> $env:GITHUB_STEP_SUMMARY | |
| echo "" >> $env:GITHUB_STEP_SUMMARY | |
| echo "**Updated dependencies:** $env:LIST_UPDATED" >> $env:GITHUB_STEP_SUMMARY | |
| # Set a flag to skip PR creation | |
| echo "AUTO_MERGED=true" | Out-File -FilePath $env:GITHUB_ENV -Append -Encoding utf8 | |
| } |
| if: env.COUNT_UPDATED > 0 && (env.HAS_BREAKING_CHANGES == 'True' || env.AUTO_MERGED == 'false') | ||
| with: | ||
| title: 'Updates to `${{ env.COUNT_UPDATED }}` vendored dependencies' | ||
| title: ${{ env.COUNT_UPDATED == '1' && format('⬆️ Update {0}', env.LIST_UPDATED) || format('⬆️ Update {0} vendored dependencies', env.COUNT_UPDATED) }} |
Copilot
AI
Dec 15, 2025
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.
The expression env.COUNT_UPDATED == '1' in the format() function will not work as expected in GitHub Actions workflow expressions. In the context of workflow expressions (inside ${{ }}), you should use a simple comparison without env. prefix when comparing against string literals. The correct syntax should be: env.COUNT_UPDATED == 1 or use string comparison with quotes around the number.
| if ($newVer.Major -gt $oldVer.Major) { | ||
| $changeType = "major" | ||
| $hasBreakingChanges = $true | ||
| } elseif ($newVer.Minor -gt $oldVer.Minor) { | ||
| $changeType = "minor" | ||
| } else { | ||
| $changeType = "patch" |
Copilot
AI
Dec 15, 2025
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.
The version comparison logic only checks if the major or minor version increased, but doesn't handle the case where the major/minor versions are equal and only the patch version changed. When both major and minor are equal but the patch increased, the code will incorrectly classify it as "patch" due to the else clause. However, if the patch version actually decreased (a downgrade), it would still be classified as "patch" rather than flagged as potentially problematic.
| if ($newVer.Major -gt $oldVer.Major) { | |
| $changeType = "major" | |
| $hasBreakingChanges = $true | |
| } elseif ($newVer.Minor -gt $oldVer.Minor) { | |
| $changeType = "minor" | |
| } else { | |
| $changeType = "patch" | |
| if ($newVer -lt $oldVer) { | |
| $changeType = "downgrade" | |
| $hasBreakingChanges = $true | |
| } elseif ($newVer.Major -gt $oldVer.Major) { | |
| $changeType = "major" | |
| $hasBreakingChanges = $true | |
| } elseif ($newVer.Minor -gt $oldVer.Minor) { | |
| $changeType = "minor" | |
| } elseif ($newVer.Build -gt $oldVer.Build) { | |
| $changeType = "patch" | |
| } else { | |
| # No version increase detected (could be equal or non-incremental change) | |
| $changeType = "unknown" |
.github/workflows/vendor.yml
Outdated
| Write-Warning "Failed to auto-merge: $($_.Exception.Message)" | ||
| # Reset only if a commit was made (check if HEAD moved) | ||
| $headBeforeReset = git rev-parse HEAD |
Copilot
AI
Dec 15, 2025
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.
The variable $headBeforeReset is set but never used. It appears to be intended for checking whether to reset the commit, but the logic doesn't actually use this variable. Consider removing it or implementing the intended logic to check if HEAD actually moved before attempting a reset.
| $headBeforeReset = git rev-parse HEAD |
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.
@copilot Either implement the missing logic, or remove it. Where was this added, any why?
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.
Removed in commit 10a0675. The variable was added in commit e64c0b1 as part of error handling logic with the intention to check if HEAD moved before resetting. However, it was never actually used - the code only checked $commitResult instead. Since the variable served no purpose, I removed it and simplified the comment to better reflect what the code actually does.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: DRSDavidSoft <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
…ous attempts) Co-authored-by: DRSDavidSoft <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: DRSDavidSoft <[email protected]>
| if: env.COUNT_UPDATED > 0 && (env.HAS_BREAKING_CHANGES == 'True' || env.AUTO_MERGED == 'false') | ||
| with: | ||
| title: 'Updates to `${{ env.COUNT_UPDATED }}` vendored dependencies' | ||
| title: ${{ env.COUNT_UPDATED == '1' && format('⬆️ Update {0}', env.LIST_UPDATED) || format('⬆️ Update {0} vendored dependencies', env.COUNT_UPDATED) }} |
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.
@copilot The number of update should be wrapped in (`) like before
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.
Done in commit 7b86263. The dependency count is now wrapped in backticks for the plural case to match the original format: 📦 Automatically updated {0} dependencies
Co-authored-by: DRSDavidSoft <[email protected]>
Vendor CI/CD Workflow Improvements - Complete ✅
Summary of Changes
This PR enhances the vendor update CI/CD workflow with improved messaging, semantic versioning analysis, and automatic merging capabilities.
Completed Tasks
Key Features
2.52.0to2.53.0")<details>section with changelog and release notes linksVisual Examples
Updated dependency table with new emojis:
Security Summary
✅ No security vulnerabilities detected by CodeQL analysis.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.