-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Add merge conflict marker detection to linux-ci workflow and refactor existing actions to use reusable get-changed-files action #26350
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
Merged
TravisEz13
merged 26 commits into
master
from
copilot/add-github-workflow-merge-conflict-check
Oct 31, 2025
+1,001
−62
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
07c1caa
Initial plan
Copilot 2872767
Add merge conflict marker checker for pull requests
Copilot b40f9c3
Add merge_conflict_check to ready_to_merge and add test files
Copilot 1e25cfa
Update .github/actions/infrastructure/merge-conflict-checker/action.yml
TravisEz13 c91fb58
Add repository owner check to merge_conflict_check job
Copilot 3125a20
Fix pagination, regex pattern, add job summary with line numbers
Copilot 79ed2d1
Move PowerShell logic from YAML to ci.psm1 module
Copilot eb8ba81
Fix code review issues: use Function, rename matches variable, simpli…
Copilot 1f54fca
Extract get-changed-files into reusable composite action
Copilot de23d2e
Delete test/merge-conflict-markers/test-complete-conflict.txt
TravisEz13 db50c54
Delete test/merge-conflict-markers/test-end-marker.txt
TravisEz13 07e288b
Delete test/merge-conflict-markers/test-separator-marker.txt
TravisEz13 492b143
Delete test/merge-conflict-markers directory
TravisEz13 d72f2b8
Update tools/ci.psm1
TravisEz13 a884672
Refactor to use Select-String, add pagination, fix JSON security issue
Copilot 83d6b78
Fix $matches variable naming and add automatic variables instruction
Copilot f29b6c4
Securely retrieve changed files using environment variable in markdow…
TravisEz13 a49d97b
Apply suggestion from @TravisEz13
TravisEz13 cc95b6d
Add infrastructure tests for merge conflict detection
TravisEz13 9f1248f
Update .github/workflows/linux-ci.yml
TravisEz13 aa1ffc1
Update tools/ci.psm1
TravisEz13 9bdc802
Add tests for Test-MergeConflictMarker function to validate conflict …
TravisEz13 884f06a
Update Pester installation logic to check for existing version before…
TravisEz13 6896b80
Remove Nanoserver CI template as it is no longer needed
TravisEz13 3fc9357
Refactor Pester installation process to use Install-CIPester function…
TravisEz13 da14b6c
Merge branch 'master' into copilot/add-github-workflow-merge-conflict…
TravisEz13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
84 changes: 84 additions & 0 deletions
84
.github/actions/infrastructure/merge-conflict-checker/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # Merge Conflict Checker | ||
|
|
||
| This composite GitHub Action checks for Git merge conflict markers in files changed in pull requests. | ||
|
|
||
| ## Purpose | ||
|
|
||
| Automatically detects leftover merge conflict markers (`<<<<<<<`, `=======`, `>>>>>>>`) in pull request files to prevent them from being merged into the codebase. | ||
|
|
||
| ## Usage | ||
|
|
||
| ### In a Workflow | ||
|
|
||
| ```yaml | ||
| - name: Check for merge conflict markers | ||
| uses: "./.github/actions/infrastructure/merge-conflict-checker" | ||
| ``` | ||
|
|
||
| ### Complete Example | ||
|
|
||
| ```yaml | ||
| jobs: | ||
| merge_conflict_check: | ||
| name: Check for Merge Conflict Markers | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name == 'pull_request' | ||
| permissions: | ||
| pull-requests: read | ||
| contents: read | ||
| steps: | ||
| - name: checkout | ||
| uses: actions/checkout@v5 | ||
|
|
||
| - name: Check for merge conflict markers | ||
| uses: "./.github/actions/infrastructure/merge-conflict-checker" | ||
| ``` | ||
|
|
||
| ## How It Works | ||
|
|
||
| 1. **File Detection**: Uses GitHub's API to get the list of files changed in the pull request | ||
| 2. **Marker Scanning**: Reads each changed file and searches for the following markers: | ||
| - `<<<<<<<` (conflict start marker) | ||
| - `=======` (conflict separator) | ||
| - `>>>>>>>` (conflict end marker) | ||
| 3. **Result Reporting**: | ||
| - If markers are found, the action fails and lists all affected files | ||
| - If no markers are found, the action succeeds | ||
|
|
||
| ## Outputs | ||
|
|
||
| - `files-checked`: Number of files that were checked | ||
| - `conflicts-found`: Number of files containing merge conflict markers | ||
|
|
||
| ## Behavior | ||
|
|
||
| - **Event Support**: Only works with `pull_request` events | ||
| - **File Handling**: | ||
| - Checks only files that were added, modified, or renamed | ||
| - Skips deleted files | ||
| - Skips binary/unreadable files | ||
| - Skips directories | ||
|
|
||
| ## Example Output | ||
|
|
||
| When conflict markers are detected: | ||
|
|
||
| ``` | ||
| ❌ Merge conflict markers detected in the following files: | ||
| - src/example.cs | ||
| Markers found: <<<<<<<, =======, >>>>>>> | ||
| - README.md | ||
| Markers found: <<<<<<<, =======, >>>>>>> | ||
|
|
||
| Please resolve these conflicts before merging. | ||
| ``` | ||
|
|
||
| When no markers are found: | ||
|
|
||
| ``` | ||
| ✅ No merge conflict markers found | ||
| ``` | ||
|
|
||
| ## Integration | ||
|
|
||
| This action is integrated into the `linux-ci.yml` workflow and runs automatically on all pull requests to ensure code quality before merging. |
36 changes: 36 additions & 0 deletions
36
.github/actions/infrastructure/merge-conflict-checker/action.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| name: 'Check for Merge Conflict Markers' | ||
| description: 'Checks for Git merge conflict markers in changed files for pull requests' | ||
| author: 'PowerShell Team' | ||
|
|
||
| outputs: | ||
| files-checked: | ||
| description: 'Number of files checked for merge conflict markers' | ||
| value: ${{ steps.check.outputs.files-checked }} | ||
| conflicts-found: | ||
| description: 'Number of files with merge conflict markers' | ||
| value: ${{ steps.check.outputs.conflicts-found }} | ||
|
|
||
| runs: | ||
| using: 'composite' | ||
| steps: | ||
| - name: Get changed files | ||
| id: changed-files | ||
| uses: "./.github/actions/infrastructure/get-changed-files" | ||
|
|
||
| - name: Check for merge conflict markers | ||
| id: check | ||
| shell: pwsh | ||
| env: | ||
| CHANGED_FILES_JSON: ${{ steps.changed-files.outputs.files }} | ||
| run: | | ||
| # Get changed files from environment variable (secure against injection) | ||
| $changedFilesJson = $env:CHANGED_FILES_JSON | ||
| $changedFiles = $changedFilesJson | ConvertFrom-Json | ||
| # Import ci.psm1 and run the check | ||
| Import-Module "$env:GITHUB_WORKSPACE/tools/ci.psm1" -Force | ||
| Test-MergeConflictMarker -File $changedFiles -WorkspacePath $env:GITHUB_WORKSPACE | ||
| branding: | ||
| icon: 'alert-triangle' | ||
| color: 'red' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
159 changes: 159 additions & 0 deletions
159
.github/instructions/powershell-automatic-variables.instructions.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| --- | ||
| applyTo: | ||
| - "**/*.ps1" | ||
| - "**/*.psm1" | ||
| --- | ||
|
|
||
| # PowerShell Automatic Variables - Naming Guidelines | ||
|
|
||
| ## Purpose | ||
|
|
||
| This instruction provides guidelines for avoiding conflicts with PowerShell's automatic variables when writing PowerShell scripts and modules. | ||
|
|
||
| ## What Are Automatic Variables? | ||
|
|
||
| PowerShell has built-in automatic variables that are created and maintained by PowerShell itself. Assigning values to these variables can cause unexpected behavior and side effects. | ||
|
|
||
| ## Common Automatic Variables to Avoid | ||
|
|
||
| ### Critical Variables (Never Use) | ||
|
|
||
| - **`$matches`** - Contains the results of regular expression matches. Overwriting this can break regex operations. | ||
| - **`$_`** - Represents the current object in the pipeline. Only use within pipeline blocks. | ||
| - **`$PSItem`** - Alias for `$_`. Same rules apply. | ||
| - **`$args`** - Contains an array of undeclared parameters. Don't use as a regular variable. | ||
| - **`$input`** - Contains an enumerator of all input passed to a function. Don't reassign. | ||
| - **`$LastExitCode`** - Exit code of the last native command. Don't overwrite unless intentional. | ||
| - **`$?`** - Success status of the last command. Don't use as a variable name. | ||
| - **`$$`** - Last token in the last line received by the session. Don't use. | ||
| - **`$^`** - First token in the last line received by the session. Don't use. | ||
|
|
||
| ### Context Variables (Use with Caution) | ||
|
|
||
| - **`$Error`** - Array of error objects. Don't replace, but can modify (e.g., `$Error.Clear()`). | ||
| - **`$PSBoundParameters`** - Parameters passed to the current function. Read-only. | ||
| - **`$MyInvocation`** - Information about the current command. Read-only. | ||
| - **`$PSCmdlet`** - Cmdlet object for advanced functions. Read-only. | ||
|
|
||
| ### Other Common Automatic Variables | ||
|
|
||
| - `$true`, `$false`, `$null` - Boolean and null constants | ||
| - `$HOME`, `$PSHome`, `$PWD` - Path-related variables | ||
| - `$PID` - Process ID of the current PowerShell session | ||
| - `$Host` - Host application object | ||
| - `$PSVersionTable` - PowerShell version information | ||
|
|
||
| For a complete list, see: https://learn.microsoft.com/powershell/module/microsoft.powershell.core/about/about_automatic_variables | ||
|
|
||
| ## Best Practices | ||
|
|
||
| ### ❌ Bad - Using Automatic Variable Names | ||
|
|
||
| ```powershell | ||
| # Bad: $matches is an automatic variable used for regex capture groups | ||
| $matches = Select-String -Path $file -Pattern $pattern | ||
|
|
||
| # Bad: $args is an automatic variable for undeclared parameters | ||
| $args = Get-ChildItem | ||
|
|
||
| # Bad: $input is an automatic variable for pipeline input | ||
| $input = Read-Host "Enter value" | ||
| ``` | ||
|
|
||
| ### ✅ Good - Using Descriptive Alternative Names | ||
|
|
||
| ```powershell | ||
| # Good: Use descriptive names that avoid conflicts | ||
| $matchedLines = Select-String -Path $file -Pattern $pattern | ||
|
|
||
| # Good: Use specific names for arguments | ||
| $arguments = Get-ChildItem | ||
|
|
||
| # Good: Use specific names for user input | ||
| $userInput = Read-Host "Enter value" | ||
| ``` | ||
|
|
||
| ## Naming Alternatives | ||
|
|
||
| When you encounter a situation where you might use an automatic variable name, use these alternatives: | ||
|
|
||
| | Avoid | Use Instead | | ||
| |-------|-------------| | ||
| | `$matches` | `$matchedLines`, `$matchResults`, `$regexMatches` | | ||
| | `$args` | `$arguments`, `$parameters`, `$commandArgs` | | ||
| | `$input` | `$userInput`, `$inputValue`, `$inputData` | | ||
| | `$_` (outside pipeline) | Use a named parameter or explicit variable | | ||
| | `$Error` (reassignment) | Don't reassign; use `$Error.Clear()` if needed | | ||
|
|
||
| ## How to Check | ||
|
|
||
| ### PSScriptAnalyzer Rule | ||
|
|
||
| PSScriptAnalyzer has a built-in rule that detects assignments to automatic variables: | ||
|
|
||
| ```powershell | ||
| # This will trigger PSAvoidAssignmentToAutomaticVariable | ||
| $matches = Get-Something | ||
| ``` | ||
|
|
||
| **Rule ID**: PSAvoidAssignmentToAutomaticVariable | ||
|
|
||
| ### Manual Review | ||
|
|
||
| When writing PowerShell code, always: | ||
| 1. Avoid variable names that match PowerShell keywords or automatic variables | ||
| 2. Use descriptive, specific names that clearly indicate the variable's purpose | ||
| 3. Run PSScriptAnalyzer on your code before committing | ||
| 4. Review code for variable naming during PR reviews | ||
|
|
||
| ## Examples from the Codebase | ||
|
|
||
| ### Example 1: Regex Matching | ||
|
|
||
| ```powershell | ||
| # ❌ Bad - Overwrites automatic $matches variable | ||
| $matches = [regex]::Matches($content, $pattern) | ||
|
|
||
| # ✅ Good - Uses descriptive name | ||
| $regexMatches = [regex]::Matches($content, $pattern) | ||
| ``` | ||
|
|
||
| ### Example 2: Select-String Results | ||
|
|
||
| ```powershell | ||
| # ❌ Bad - Conflicts with automatic $matches | ||
| $matches = Select-String -Path $file -Pattern $pattern | ||
|
|
||
| # ✅ Good - Clear and specific | ||
| $matchedLines = Select-String -Path $file -Pattern $pattern | ||
| ``` | ||
|
|
||
| ### Example 3: Collecting Arguments | ||
|
|
||
| ```powershell | ||
| # ❌ Bad - Conflicts with automatic $args | ||
| function Process-Items { | ||
| $args = $MyItems | ||
| # ... process items | ||
| } | ||
|
|
||
| # ✅ Good - Descriptive parameter name | ||
| function Process-Items { | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(ValueFromRemainingArguments)] | ||
| [string[]]$Items | ||
| ) | ||
| # ... process items | ||
| } | ||
| ``` | ||
|
|
||
| ## References | ||
|
|
||
| - [PowerShell Automatic Variables Documentation](https://learn.microsoft.com/powershell/module/microsoft.powershell.core/about/about_automatic_variables) | ||
| - [PSScriptAnalyzer Rules](https://github.com/PowerShell/PSScriptAnalyzer/blob/master/docs/Rules/README.md) | ||
| - [PowerShell Best Practices](https://learn.microsoft.com/powershell/scripting/developer/cmdlet/strongly-encouraged-development-guidelines) | ||
|
|
||
| ## Summary | ||
|
|
||
| **Key Takeaway**: Always use descriptive, specific variable names that clearly indicate their purpose and avoid conflicts with PowerShell's automatic variables. When in doubt, choose a longer, more descriptive name over a short one that might conflict. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.