Skip to content

[net11.0] Preserve source-only versions in release merges - #37019

Closed
kubaflo wants to merge 3 commits into
net11.0from
copilot/preserve-source-version-properties
Closed

[net11.0] Preserve source-only versions in release merges#37019
kubaflo wants to merge 3 commits into
net11.0from
copilot/preserve-source-version-properties

Conversation

@kubaflo

@kubaflo kubaflo commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description

The automated net11.0-to-release merge flow resets all of eng/Versions.props to the release branch. That preserves release pins, but it also removes properties introduced only on net11.0. This caused the maui-pr failures on #36986 / build 1536407.

This change:

  • adds a tested reconciler that preserves every release value and, using three-way (merge-base) provenance, adds only direct source properties that are genuinely new on net11.0 (absent from the merge base) and missing from the generated merge branch — a property deleted on the release branch is never resurrected;
  • fails closed for malformed XML, ambiguous properties, unsupported multiline placement, and mismatched property groups;
  • runs after Arcade creates the merge branch, verifies exact refs and ancestry, and pushes only a normal fast-forward commit;
  • the reconciler itself is idempotent, so re-running it on the same inputs produces no additional diff.

Validation

  • 13 Pester tests
  • PSScriptAnalyzer with no warnings or errors
  • Real net11.0 / Preview 7 reconciliation adds exactly the four missing properties and is idempotent
  • Workflow YAML and merge-flow JSONC parse successfully

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 37faae15-0e4c-46fd-9bef-f3006c8aaee8
Copilot AI review requested due to automatic review settings August 2, 2026 14:11
@kubaflo
kubaflo temporarily deployed to copilot-pat-pool August 2, 2026 14:11 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37019

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37019"

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@kubaflo
kubaflo temporarily deployed to copilot-pat-pool August 2, 2026 14:11 — with GitHub Actions Inactive
@kubaflo
kubaflo temporarily deployed to copilot-pat-pool August 2, 2026 14:12 — with GitHub Actions Inactive
@kubaflo
kubaflo temporarily deployed to copilot-pat-pool August 2, 2026 14:14 — with GitHub Actions Inactive
@kubaflo
kubaflo temporarily deployed to copilot-pat-pool August 2, 2026 14:15 — with GitHub Actions Inactive
@github-actions github-actions Bot added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Aug 2, 2026
@kubaflo
kubaflo temporarily deployed to copilot-pat-pool August 2, 2026 14:15 — with GitHub Actions Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the net11.0 → release automated merge flow to keep eng/Versions.props aligned with the release branch pins while preserving any version properties that exist only on net11.0, avoiding merge-branch CI breaks caused by losing source-only properties.

Changes:

  • Adds a new post-merge GitHub Actions job to reconcile eng/Versions.props on the generated merge branch.
  • Introduces a PowerShell reconciler that inserts only missing source-only properties (never overwriting release pins), with “fail closed” safety checks.
  • Adds Pester coverage for the reconciler, including idempotency and malformed/unsafe XML cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/merge-net11-to-release.yml Adds a reconciliation job that runs after the Arcade merge job and pushes a commit to the generated merge branch when needed.
.github/scripts/Merge-SourceOnlyVersionProperties.ps1 Implements the XML/text-layout-based reconciler that inserts missing source-only properties into eng/Versions.props.
.github/scripts/Merge-SourceOnlyVersionProperties.Tests.ps1 Adds Pester tests validating merge behavior, safety checks, and idempotency.

Comment thread .github/workflows/merge-net11-to-release.yml
Comment thread .github/workflows/merge-net11-to-release.yml Outdated
Comment thread .github/scripts/Merge-SourceOnlyVersionProperties.ps1
Address review feedback on the net11.0 -> release reconciliation flow:

- Validate MergeToBranch with `git check-ref-format --branch` (both resolve
  steps). The `^release/...$` regex still admits values Git rejects as refs
  (trailing '/', '..', '@{', ...), and the value is used to build refspecs.

- Use `git rev-list --count` instead of materializing the full SHA list just
  to test emptiness of the net11.0..release range.

- Fail closed in Merge-SourceOnlyVersionProperties.ps1 when the corresponding
  target PropertyGroup shares no property with the source group: the Nth-group
  correspondence can't be verified, so throw instead of inserting into a
  possibly-unrelated group (which would still be well-formed XML). Adds a
  Pester test for the new guard.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
Copilot AI review requested due to automatic review settings August 2, 2026 14:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs changes before merge.

The general capability belongs in dotnet/arcade, which owns ResetToTargetPaths, the immutable merge inputs, generated branch, push, and PR creation. A MAUI-local implementation is reasonable only as a short-term unblock; the durable solution should reconcile before Arcade pushes and creates the PR.

The Pester tests are automatically discovered, but coverage is missing for three-way deletion provenance, an absent generated merge branch, source advancement between jobs, and PR-visible failure recovery. The title is accurate; the description should update the test count and remove the current retry/idempotency overclaim.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist.

Comment thread .github/scripts/Merge-SourceOnlyVersionProperties.ps1
exit 0
}

git merge-base --is-ancestor origin/net11.0 "origin/$env:MERGE_BRANCH"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race Condition — This validates the mutable current origin/net11.0, not the source SHA Arcade merged. If net11.0 advances between the Arcade job and this job, reconciliation fails after the PR already exists; subsequent runs then see the open PR and skip both merge and reconciliation indefinitely. Consume immutable source/generated-branch SHAs from Arcade and allow reconciliation to resume an existing verified automation PR.
Flagged by: 3/3 reviewers after dispute

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this is a real race, and it's the part that genuinely belongs in Arcade (as you note in the summary). The reconciler currently keys off the mutable origin/net11.0; a correct fix needs Arcade to surface the immutable source/generated-branch SHAs it merged so reconciliation validates and operates on exactly those, and can resume an already-created automation PR. That's an Arcade-side capability I can't consume from here yet, so I'm leaving this thread open as the tracked durable follow-up rather than papering over it MAUI-side. The three-way-ancestor and missing-branch no-op changes reduce the blast radius in the meantime.

Comment thread .github/workflows/merge-net11-to-release.yml
configuration_file_branch: 'net11.0'
configuration_file_path: 'github-merge-flow-release-11.jsonc'

ReconcileVersionProperties:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Error Handling — Arcade creates the merge PR before this separate job runs, but reconciliation failures are attached only to the triggering net11.0 workflow run. A fail-closed error can therefore leave the broken generated PR open without a PR-visible signal or recovery path. Attach a status/comment/label to the generated PR, or perform reconciliation atomically before PR creation.
Flagged by: 3/3 reviewers after dispute

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid — because reconciliation runs as a separate job after Arcade creates the PR, a fail-closed error currently surfaces only on the triggering net11.0 run, not on the generated PR. The robust version (attach a status/label/comment to the generated PR, or reconcile atomically before PR creation) is coupled to the same Arcade-provides-the-generated-PR-identity work as the race-condition thread above, so I'm keeping this open as part of that durable follow-up rather than adding a fragile PR-number lookup here. Leaving the thread open.

PureWeen pushed a commit that referenced this pull request Aug 3, 2026
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

## Description

Flows the remaining CI fixes discovered while validating the automated
`net11.0` to Preview 7 merge in #36986 back to `net11.0`.

- Adds the missing iOS hosting namespace so the TabbedPage device tests
compile.
- Makes Avalonia template tests skip template post-action restore and
use a test-specific NuGet configuration. Existing approved feeds remain
mapped to all packages; NuGet.org is limited to the external `Avalonia*`
and `MicroCom.*` dependency families.
- Serializes `DispatcherTests` and `MainThreadBridgeTests`, which both
mutate the process-global `DispatcherProvider`, preventing the Helix
race observed in build 1536641.

The modal device-test override fix was removed from this PR after it
landed independently in #37030.

These remaining fixes produced a successful aggregate `maui-pr` run for
#36986 in [build
1536659](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1536659).

The source-only version-property merge hardening remains separately
tracked by #37019.

## Testing

- `Core.UnitTests`: 22 targeted `DispatcherTests` and
`MainThreadBridgeTests` passed on .NET 11 RC1 after rebasing.
- iOS device-test compilation was validated while fixing #36986.
- Avalonia integration scenarios restored through the restricted
package-source mapping in successful `maui-pr` build 1536659.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Vally Fixture <vally-fixture@example.invalid>
Copilot-Session: 37faae15-0e4c-46fd-9bef-f3006c8aaee8
Address PureWeen's review of the net11.0 -> release version reconciliation:

- Data loss: the name-only "missing from target" comparison could not tell a
  property newly added on net11.0 apart from one intentionally deleted on the
  release branch, so a servicing deletion could be silently resurrected. The
  reconciler now accepts an optional -AncestorPath (the merge-base copy of
  eng/Versions.props) and only restores a missing property when it is also
  absent from that ancestor. The workflow computes the merge base of net11.0
  and the release branch and passes it. Adds Pester coverage for both the
  deleted-on-target (not restored) and added-on-source (restored) cases.

- Error handling: Arcade legitimately exits without creating a merge branch
  when there is nothing to merge. Fetch net11.0/target first, run the no-work
  commit-count check, then fetch the generated branch separately and treat a
  missing ref as a clean no-op instead of failing the combined fetch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
Copilot AI review requested due to automatic review settings August 3, 2026 16:59
@kubaflo

kubaflo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

@PureWeen — pushed 1df1093 implementing two of the four: three-way provenance via -AncestorPath (prevents resurrecting a servicing-deleted pin, with Pester coverage) and treating a missing generated merge branch as a clean no-op. The race-condition and PR-visible-signal threads I've left open as the durable Arcade-side follow-up (they need Arcade to surface the immutable merged SHAs / generated-PR identity), as noted in your summary. Ready for re-review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

.github/scripts/Merge-SourceOnlyVersionProperties.Tests.ps1:3

  • PR description/validation notes say "10 Pester tests", but this test suite currently contains 13 It cases. Please update the PR description (or adjust the test suite) so the stated validation matches what’s actually in the PR.
#!/usr/bin/env pwsh
#Requires -Modules Pester

@kubaflo

kubaflo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Updated the PR description to match the current implementation: the validation now says 13 Pester tests (was 10 — the three-way-provenance work added two), the reconciler bullet notes the merge-base provenance (deleted-on-release pins are not resurrected), and the idempotency claim is scoped to the reconciler itself rather than the whole workflow retry path.

Vignesh-SF3580 pushed a commit to Vignesh-SF3580/maui that referenced this pull request Aug 10, 2026
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

## Description

Flows the remaining CI fixes discovered while validating the automated
`net11.0` to Preview 7 merge in dotnet#36986 back to `net11.0`.

- Adds the missing iOS hosting namespace so the TabbedPage device tests
compile.
- Makes Avalonia template tests skip template post-action restore and
use a test-specific NuGet configuration. Existing approved feeds remain
mapped to all packages; NuGet.org is limited to the external `Avalonia*`
and `MicroCom.*` dependency families.
- Serializes `DispatcherTests` and `MainThreadBridgeTests`, which both
mutate the process-global `DispatcherProvider`, preventing the Helix
race observed in build 1536641.

The modal device-test override fix was removed from this PR after it
landed independently in dotnet#37030.

These remaining fixes produced a successful aggregate `maui-pr` run for
dotnet#36986 in [build
1536659](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1536659).

The source-only version-property merge hardening remains separately
tracked by dotnet#37019.

## Testing

- `Core.UnitTests`: 22 targeted `DispatcherTests` and
`MainThreadBridgeTests` passed on .NET 11 RC1 after rebasing.
- iOS device-test compilation was validated while fixing dotnet#36986.
- Avalonia integration scenarios restored through the restricted
package-source mapping in successful `maui-pr` build 1536659.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Vally Fixture <vally-fixture@example.invalid>
Copilot-Session: 37faae15-0e4c-46fd-9bef-f3006c8aaee8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants