Fix inter-branch-merge to create proper merge commit when using ResetToTargetPaths by PureWeen · Pull Request #16673 · dotnet/arcade

PureWeen · 2026-04-02T20:27:31Z

Fix inter-branch-merge to create proper merge commit when using ResetToTargetPaths

Problem

When ResetToTargetPaths is configured, the inter-branch-merge script creates a branch from source HEAD and later resets specific files to the target branch. This means GitHub shows the diff relative to source, not accounting for target branch content — producing misleading "changed files" lists.

Approach: Three-Path Merge Strategy

When ResetToTargetPaths is set, the script now attempts a proper merge commit (checkout from target, merge source into it):

Clean merge → proper 2-parent merge commit. ResetFilesToTargetBranch then overwrites pattern files with target content.
Conflicts only in ResetToTargetPaths patterns → abort, retry with -X ours (favors target content as a safety net). Safe because those files get overwritten by ResetFilesToTargetBranch anyway.
Conflicts outside patterns → fall back to source-only branch (original behavior) so GitHub's merge button surfaces real conflicts to the reviewer.

Without ResetToTargetPaths, behavior is unchanged.

Safety Measures

Never auto-resolves conflicts outside ResetToTargetPaths patterns — falls back to original behavior
-X ours (favor target) instead of -X theirs — if ResetFilesToTargetBranch misses a file due to pathspec differences, the merge commit already has the target version rather than silently carrying source content
Human-commit protection — before any force-push, checks git rev-list for remote-only commits to avoid overwriting manual conflict resolutions
Fresh fetch before guard — refreshes origin/$mergeBranchName immediately before the safety check
Safe merge --abort — uses plain call (not Invoke-Block) to handle non-conflict git failures gracefully
Empty conflict guard — if merge fails with no conflicts detected, falls back to source-only branch
CI diagnostics — merge output is always logged (was previously captured but never displayed)

Known Limitations / Open Discussion Points

⚠️ Design question for @mmitche: This PR still performs client-side merges for the clean-merge and pattern-only-conflict cases. mmitche's feedback is that any client-side merge is an unacceptable tradeoff because it changes the GitHub review UX. This is a design disagreement — the code is correct but the approach may not be desired. Need owner decision.

Pattern matching mismatch (pre-existing): PowerShell -like (used for conflict classification) treats * as matching across /, while git pathspec (used by ResetFilesToTargetBranch) does not. Mitigated by using -X ours (target wins on mismatch), but not fully resolved.
No PR notification of auto-resolution: When conflicts are auto-resolved, the PR comment says only "updated" with no indication that auto-resolution occurred. Could enhance the update comment to list resolved files.
Force-push on every merge-commit run: Merge commits create non-fast-forwardable history, requiring --force on each update. This may reset PR review approvals in some GitHub configurations.

Testing

Validated with 24 integration tests across 4 scenarios:

Clean merge (no conflicts)
Conflicts only in ResetToTargetPaths patterns
Conflicts outside ResetToTargetPaths patterns
No ResetToTargetPaths configured (original behavior)

Fixes #10212

ViktorHofer · 2026-04-23T18:39:23Z

@mmitche PTAL - I noticed myself that ResetToTargetPaths doesn't fully work yet.

mmitche · 2026-04-24T14:44:28Z

+        # Merge source branch. Use -X theirs to auto-resolve conflicts in favor of
+        # the source branch, since ResetToTargetPaths will overwrite target-wins files
+        # in the next step anyway.
+        Invoke-Block { & git merge --no-ff "origin/$MergeFromBranch" -X theirs -m "Merge branch '$MergeFromBranch' into $MergeToBranch" }


I don't think this is correct. ResetToTargetPaths doesn't overwrite all possible files from the target, only those that are specified in the pattern. This merges in the source branch and auto-resolves conflicts, which may not be the correct behavior as some of those files may not be in the ResetToTargetPaths list.

mmitche · 2026-04-24T17:41:13Z

+            }
+
+            if ($outsidePatternConflicts.Count -gt 0) {
+                Write-Host -f Yellow "WARNING: The following conflicting files are NOT in ResetToTargetPaths and will be auto-resolved in favor of '$MergeFromBranch':"


I don't think this is good enough. Teams aren't going to review this close enough to catch issues.

mmitche

I think this is the same problem where the changed files is listed incorrectly, right? When I did research into this the other week, basically the conclusion I got is that this a GitHub UX issue primarily, and if you attempt to resolve on the client side via merge prior to push (since you can't partially resolve), then you're just inviting the possibility of hiding conflicts.

I don't think that's an acceptable tradeoff.

…ToTargetPaths When ResetToTargetPaths is configured, the script creates the merge branch from the source branch HEAD and only resets specified files to the target branch. This means the merge branch is missing all target-only content (APIs, platform fixes, version updates, etc.) — it's a linear commit off source, not a real merge. This causes: - Build failures from missing target branch fixes (nullable annotations, platform version mismatches, etc.) - Merge conflicts when the PR is completed because the target branch's changes were never incorporated The fix creates a proper merge commit when ResetToTargetPaths is used: 1. Start the merge branch from the target branch (not source) 2. Merge source into it with --no-ff to create a merge commit 3. Use -X theirs to auto-resolve conflicts (ResetToTargetPaths will overwrite target-wins files in the next step anyway) 4. Then apply ResetToTargetPaths as before Without ResetToTargetPaths, the original behavior is preserved — the branch is created from source and GitHub's merge button does the merge. Discovered in dotnet/maui PR #34789 where the main→net11.0 merge was missing all net11.0-specific content (Directory.Build.props TFMs, nullable fixes, PublicAPI entries) because the script never merged net11.0 into the branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

When merge conflicts exist outside ResetToTargetPaths patterns, fall back to the original source-only branch behavior so GitHub's merge button surfaces the real conflicts to the reviewer. Only auto-resolve when ALL conflicts are within pattern files (since those get overwritten by target anyway). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

When merge conflicts exist outside ResetToTargetPaths patterns, fall back to the original source-only branch behavior so GitHub's merge button surfaces the real conflicts to the reviewer. Only auto-resolve when ALL conflicts are within pattern files (since those get overwritten by target anyway). Also fixes: fallback path now explicitly checks out from source HEAD (origin/MergeFromBranch) instead of staying at target after merge --abort. Also fixes: PR update push uses --force when a merge commit was created, since merge commits create non-fast-forwardable history on each run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Changes: - Use -X ours instead of -X theirs: resolves in favor of target (safety net if ResetFilesToTargetBranch misses a file due to pathspec differences) - Log merge output for CI diagnostics (was captured but never displayed) - Safe merge --abort: use plain call instead of Invoke-Block to handle non-conflict git failures gracefully (exit 128 = no merge to abort) - Guard empty conflict list: if merge fails but no conflicts detected, fall back to source-only branch instead of misdiagnosing as safe - Force-push protection: check for human-pushed commits on remote before force-pushing to avoid silently overwriting manual conflict resolutions - History divergence fix: non-force push path retries with --force on failure (handles previous merge-commit run leaving diverged history) - Remove duplicate git config from ResetFilesToTargetBranch (caller sets it) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Non-merge path now checks git rev-list for remote-only commits before retrying with --force (same guard as merge-commit path) - Refresh remote tracking ref for merge branch just before the safety check to close the stale-fetch window Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Adversarial review found that the previous design had two release-blocking issues: 1. The human-commit guard misclassified the bot's OWN prior merge commit as an unmerged human commit. Each run builds a fresh merge commit whose ancestors are current origin/target + origin/source, which does not include the previous run's merge commit. `git rev-list local..origin` therefore reported the prior bot merge as 'extra', triggering the guard and failing every PR update after the first. 2. The `-X ours` auto-resolution path could silently hide conflicts in files matched only by PowerShell's `-like` operator and not by git pathspec (e.g. `eng/common/foo/bar.yml` against pattern `eng/common/*`). This is exactly the 'hiding conflicts' scenario the reviewer objected to. Changes: - Remove the `-X ours` path entirely. ANY merge conflict (in or out of patterns) now falls back to a source-only branch so GitHub's merge button surfaces the conflict to the reviewer. This also removes the PowerShell `-like` vs git pathspec mismatch as a vector. - Add Get-NonBotExtraCommits helper that filters `rev-list` output by author email. Only commits whose author is not 'github-actions[bot]' count as a reason to skip force-push. This lets the bot safely overwrite its own prior merge commits while still protecting manual changes pushed by contributors. - Replace silent `git fetch ... 2>$null` with an ls-remote existence check followed by a fail-closed fetch. If the remote branch doesn't exist we skip the guard entirely (vacuous); if fetch fails for any other reason we throw so we don't push with a stale view of remote state. - Use `-c core.quotePath=false` when listing conflict files so paths with special characters aren't quoted in the output. - The PR update comment now distinguishes between a clean merge commit and a source-only fallback (with the list of conflicting files), so reviewers know whether GitHub's diff reflects a real merge or whether they need to resolve conflicts via the merge button. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR rewrites the inter-branch-merge.ps1 automation so that when ResetToTargetPaths is configured, the bot attempts to produce a real two-parent merge commit (target ← source) instead of a source-only branch with files later overwritten. It also adds safety guards around force-pushes so the bot does not overwrite manual pushes to the PR branch, and surfaces the chosen merge path in the PR update comment.

Changes:

Adds a clean-merge attempt rooted on the target branch (with source merged in) when ResetToTargetPaths is set, falling back to the original source-only branch on any conflict.
Introduces Get-NonBotExtraCommits plus a refreshed-fetch / ls-remote check before force-pushing to avoid clobbering human commits, and updates the PR comment to describe which merge path was taken.
Moves the git bot identity configuration out of ResetFilesToTargetBranch to the caller, and trims/filters the ResetToTargetPaths pattern list.

Show a summary per file

File	Description
`.github/workflows/scripts/inter-branch-merge.ps1`	Implements the three-path (PR description) / two-path (actual code) merge strategy, adds force-push safety guards via `Get-NonBotExtraCommits`, and rewrites the PR update comment to indicate which path was taken.

Copilot's findings

Files reviewed: 1/1 changed files
Comments generated: 4

+        # Try a clean merge. We do NOT pass -X ours / -X theirs anywhere in this
+        # script — any conflict must surface to the reviewer via the source-only
+        # fallback below.
+        $mergeOutput = & git merge --no-ff "origin/$MergeFromBranch" -m "Merge branch '$MergeFromBranch' into $MergeToBranch" 2>&1
+        $mergeExitCode = $LASTEXITCODE
+
+        # Always log merge output for CI diagnostics
+        if ($mergeOutput) {
+            $mergeOutput | Write-Host
+        }
+
+        if ($mergeExitCode -eq 0) {
+            $createdMergeCommit = $true
+        } else {
+            # Capture conflict file list before aborting so we can surface it.
+            [string[]] $conflictFiles = & git -c core.quotePath=false diff --name-only --diff-filter=U
+
+            # Abort the conflicted merge before proceeding.
+            # Use plain call (not Invoke-Block) because git merge --abort exits 128
+            # if there is no merge-in-progress (e.g. a non-conflict git failure).
+            & git merge --abort 2>&1 | Write-Host
+
+            if (-not $conflictFiles -or $conflictFiles.Count -eq 0) {
+                Write-Host -f Yellow "Merge failed with exit code $mergeExitCode but no conflicts were detected."
+                Write-Host -f Yellow "Falling back to source-only branch."
+            } else {
+                Write-Host -f Yellow "Merge produced conflicts in the following files:"
+                $conflictFiles | % { Write-Host -f Yellow "  - $_" }
+                Write-Host -f Yellow "Falling back to source-only branch so GitHub surfaces these conflicts in the PR."
+            }
+
+            Invoke-Block { & git checkout -B $mergeBranchName "origin/$MergeFromBranch" }
+        }


+    $botEmail = '41898282+github-actions[bot]@users.noreply.github.com'
+    $nonBot = @()
+    foreach ($sha in $extraShas) {
+        $authorEmail = & git show -s --format='%ae' $sha 2>$null
+        if ($LASTEXITCODE -ne 0 -or -not $authorEmail) {
+            # Couldn't read commit metadata — be conservative and treat as non-bot.
+            $nonBot += $sha
+            continue
+        }
+        if ($authorEmail.Trim() -ne $botEmail) {
+            $nonBot += $sha
+        }
+    }
+    return $nonBot


+            Invoke-Block { & git checkout -B $mergeBranchName "origin/$MergeFromBranch" }
+        }
+
        ResetFilesToTargetBranch $patterns $MergeToBranch


+                } else {
+                    # Try non-force push first. If it fails (e.g. remote diverged from
+                    # a previous merge-commit run), retry with --force after checking
+                    # for human-pushed commits (same guard as the merge-commit path).
+                    & git push $remoteName "${mergeBranchName}:${mergeBranchName}" 2>&1 | Write-Host
+                    if ($LASTEXITCODE -ne 0) {
+                        if ($remoteBranchExists) {
+                            [string[]] $extraCommits = Get-NonBotExtraCommits $mergeBranchName "origin/$mergeBranchName"
+                            if ($extraCommits -and $extraCommits.Count -gt 0) {
+                                Write-Warning "Remote branch '$mergeBranchName' has $($extraCommits.Count) non-bot commit(s) not in the local branch. Skipping force push to avoid overwriting manual changes."
+                                $extraCommits | % { Write-Warning "  $_" }
+                                throw "Remote branch has unmerged human commits"
+                            }
+                        }
+                        Write-Host "Non-force push failed (likely diverged history). Retrying with --force..."
+                        Invoke-Block { & git push --force $remoteName "${mergeBranchName}:${mergeBranchName}" }
+                    }
+                }


…ation Follow-up to the Option B redesign addressing 5 issues found by adversarial code review against the prior commit on this branch: 1. `Get-NonBotExtraCommits` failed open on `git rev-list` errors. The function used `2>$null` which suppressed git error output and let a non-zero exit code fall through to "return @()" — meaning a stale ref or transient git failure would silently bypass the human-commit guard. It now captures stderr, checks $LASTEXITCODE, and throws on failure. 2. The PR-update force-push paths used plain `--force`, leaving a TOCTOU window between the fetch (which reads the current remote SHA) and the push (which overwrites it). A concurrent push during that window would be silently overwritten. Both force-push call sites in the PR-update path now use `--force-with-lease=<refname>:<sha>` where <sha> is captured at fetch time; git rejects the push atomically if the remote moved. 3. The non-merge-commit fallback push path used `& git push ... 2>&1 | Write-Host` (no Invoke-Block) and then retried with `--force` on ANY non-zero exit code. That meant a token-expired / network / permission error would be misclassified as "diverged history" and trigger an automatic force-retry. The code now captures push stderr and inspects it for known non-fast-forward signatures (`non-fast-forward`, `failed to push some refs`, `tip of your branch is behind`, `Updates were rejected`). Only those patterns trigger a force-retry; any other failure throws so the actual cause surfaces in the catch block. 4. The "no existing PR" branch (else of the matching-PR check) used unguarded `git push --force`. If the merge branch already exists remotely (residual from a closed PR or a manual contributor push) that force overwrites human commits without warning. The path now uses the same `Update-RemoteBranchInfo` + `Assert-NoHumanCommitsOnRemote` + `--force-with-lease` sequence as the PR-update path. 5. The comment at the top of the merge block overclaimed that the script "deliberately does NOT auto-resolve conflicts client-side, even within ResetToTargetPaths patterns." That contradicts both the documented purpose of ResetToTargetPaths (see .PARAMETER block: "resolving potential merge conflicts for these files") and the actual behavior on line 304 (ResetFilesToTargetBranch runs in both clean and fallback paths). The comment now accurately describes the contract: non-pattern conflicts surface to the reviewer; pattern files are always reset to target per the opt-in. Refactoring: - Extracted `Update-RemoteBranchInfo` helper that returns `@{ Exists; Sha }`, combining ls-remote / fetch / rev-parse in one fail-closed step. The Sha is the value passed to --force-with-lease. - Extracted `Assert-NoHumanCommitsOnRemote` helper that wraps the Get-NonBotExtraCommits + throw pattern, since it's now called from three places. Tests (all in /tmp during development, not committed): - Get-NonBotExtraCommits: throws on bad ref; returns @() for no divergence; correctly filters bot commits while preserving human commits. - Update-RemoteBranchInfo: returns Exists=false for missing branch; returns Exists=true + valid SHA for existing branch. - Assert-NoHumanCommitsOnRemote: throws when remote has human commits; no-op when remote has only bot commits. - --force-with-lease: rejects push with stale lease SHA (TOCTOU sim); accepts push with matching lease SHA. - Regex classification: matches all 3 sample non-fast-forward outputs from real git push failures; does NOT match auth/network/permission errors (4 samples). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

PureWeen force-pushed the fix/inter-branch-merge-missing-merge branch 3 times, most recently from 6d20d47 to a0f3105 Compare April 2, 2026 20:43

ViktorHofer requested review from akoeplinger and mmitche April 3, 2026 09:55

mmitche reviewed Apr 24, 2026

View reviewed changes

mmitche requested changes Apr 24, 2026

View reviewed changes

PureWeen force-pushed the fix/inter-branch-merge-missing-merge branch from 0b75f0b to 3e898de Compare April 24, 2026 16:57

mmitche reviewed Apr 24, 2026

View reviewed changes

mmitche requested changes Apr 24, 2026

View reviewed changes

PureWeen force-pushed the fix/inter-branch-merge-missing-merge branch from 3e898de to 3e39955 Compare April 24, 2026 19:24

PureWeen marked this pull request as draft April 24, 2026 21:15

PureWeen and others added 6 commits May 18, 2026 13:17

Copilot AI review requested due to automatic review settings May 18, 2026 18:22

PureWeen force-pushed the fix/inter-branch-merge-missing-merge branch from 2a54873 to 8293b4b Compare May 18, 2026 18:22

Copilot started reviewing on behalf of PureWeen May 18, 2026 18:22 View session

Copilot AI reviewed May 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix inter-branch-merge to create proper merge commit when using ResetToTargetPaths#16673

Fix inter-branch-merge to create proper merge commit when using ResetToTargetPaths#16673
PureWeen wants to merge 7 commits into
mainfrom
fix/inter-branch-merge-missing-merge

PureWeen commented Apr 2, 2026 •

edited

Loading

Uh oh!

ViktorHofer commented Apr 23, 2026

Uh oh!

mmitche Apr 24, 2026

Uh oh!

mmitche Apr 24, 2026

Uh oh!

mmitche left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

PureWeen commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!