Add ApplicationArtifact support for Windows App SDK - #35973
Conversation
Enrich platform-produced ApplicationArtifact items with MAUI application metadata via GetApplicationArtifactsDependsOn instead of introducing MAUI-specific artifact item groups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35973Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35973" |
github-actions
Bot
added
the
area-tooling
Document that platform build/artifact-production steps populate ApplicationArtifact items before MAUI metadata enrichment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Multi-Model Review — PR #35973
Verdict: ❌ NEEDS_CHANGES
Confidence: High (unanimous across all 4 models)
All 4 models independently identified issues that should be addressed before merging.
Model agreement: gemini (NEEDS_CHANGES/1), gpt-5.5 (NEEDS_CHANGES/1), opus-4.6 (NEEDS_CHANGES/1), opus-4.8 (NEEDS_CHANGES/2)
Findings: 4 unique issues identified.
| <ApplicationIdGuid>11111111-2222-3333-4444-555555555555</ApplicationIdGuid> | ||
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
The relative path $(MSBuildThisFileDirectory)..\..\..\ assumes the test directory is located within the MAUI repository bin/ folder. However, on Azure Pipelines CI, TestDirectory is located under AGENT_TEMPDIRECTORY (e.g., .../test-dir/ApplicationArtifactsAreEnrichedWithMauiMetadata/). As a result, traversing up 3 directories points to AGENT_TEMPDIRECTORY/src/Workload/... instead of the repository, causing MSB4019 missing import errors and failing the integration tests.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
This import is rooted at the generated test project's MSBuildThisFileDirectory and walks up three directories to find src/Workload. That happens to work for local fallback TestDirectory under the repo's bin/test-dir, but in Azure Pipelines TestEnvironment.GetTestDirectoryRoot() uses AGENT_TEMPDIRECTORY/test-dir, so ......\src does not point at the MAUI checkout and the new Build integration test cannot import Microsoft.Maui.Sdk.After.targets. The current CI failures in the Build integration lanes are consistent with this PR-introduced test path issue.
(found by: 5, 4)
| <Target Name="SeedApplicationArtifacts"> | ||
| <ItemGroup> | ||
| <ApplicationArtifact Include="$(MSBuildProjectDirectory)/artifacts/platform/android/MyArtifactApp-Signed.apk"> | ||
| <PackageFormat>apk</PackageFormat> |
There was a problem hiding this comment.
The explicit <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\\..\\..\\src\\Workload\\...'))" /> assumes the project directory is 3 levels deep inside the repo root. Locally this resolves from <repo>/bin/test-dir/<testname>/ to <repo>/. In CI, TestDirectory is under $AGENT_TEMPDIRECTORY/test-dir/<testname>/, so 3 parent traversals land in the agent temp directory—not the repo. The targets file won't exist there, causing a build error. This is the likely cause of the CI 'Build integration tests' failures on both macOS and Windows.
| Lines="@(ApplicationArtifact->'%(Filename)%(Extension)|%(PackageFormat)|%(ApplicationTitle)|%(ApplicationName)|%(ApplicationId)|%(ApplicationIdGuid)|%(ApplicationDisplayVersion)|%(ApplicationVersion)|%(Signed)|%(PackageId)|%(PlatformName)|%(BundleIdentifier)')" | ||
| Overwrite="true" /> | ||
| </Target> | ||
| <Target Name="WritePublishApplicationArtifactsMetadata" DependsOnTargets="SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata"> |
There was a problem hiding this comment.
Both WriteGetApplicationArtifactsMetadata and WritePublishApplicationArtifactsMetadata depend directly on SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata, so they exercise the metadata target in isolation rather than through a real GetApplicationArtifacts/Publish invocation; the actual wiring is only checked indirectly by the string Assert.Contains on the depends-on file (line 178). The Publish variant therefore adds no coverage beyond the Get variant. This is understandable while the platform GetApplicationArtifacts target is unavailable in net11, but consider collapsing the duplicate target (or asserting the real wiring) once the platform producers land.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
[major] Build & MSBuild — This import is resolved relative to the generated test project. In CI TestDirectory is under
| </ApplicationArtifact> | ||
| </ItemGroup> | ||
| </Target> | ||
| <Target Name="WriteGetApplicationArtifactsMetadata" DependsOnTargets="SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata"> |
There was a problem hiding this comment.
[moderate] Regression Prevention — The test target depends on _AddMauiApplicationArtifactMetadata directly, so it validates the private target body rather than the GetApplicationArtifactsDependsOn extension path this PR adds. A regression where the hook is not actually invoked by GetApplicationArtifacts/Publish could still pass because lines 157 and 167 call the target manually. Seed the item, invoke the targets through $(GetApplicationArtifactsDependsOn)/the public artifacts target, and assert the metadata after that path runs.
MauiBot
added
s/agent-fix-win
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
❌ Error — This import is resolved relative to the generated temp test project via $(MSBuildThisFileDirectory), not relative to the MAUI repo. In CI, TestDirectory is under AGENT_TEMPDIRECTORY, so ..\..\..\src\Workload\... points outside /home/vsts/work/1/s and the test fails before exercising the PR fix. Please build the import path from TestEnvironment.GetMauiDirectory() or otherwise pass an absolute repo-rooted path into the generated project.
| </ApplicationArtifact> | ||
| </ItemGroup> | ||
| </Target> | ||
| <Target Name="WriteGetApplicationArtifactsMetadata" DependsOnTargets="SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata"> |
There was a problem hiding this comment.
_AddMauiApplicationArtifactMetadata target directly, so it can pass even if the public $(GetApplicationArtifactsDependsOn) hook is not honored by GetApplicationArtifacts/Publish or runs in the wrong order. Please exercise the public target path, or make this target depend on $(GetApplicationArtifactsDependsOn), so the regression test covers the actual integration contract added in Microsoft.Maui.Sdk.After.targets.
MauiBot
added
s/agent-fix-pr-picked
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
❌ Error — This import resolves from the generated temp project directory, not the MAUI repo root. In CI TestDirectory is under the agent temp/test-dir (and locally under bin/test-dir), so ..\\..\\..\\src\\Workload... points at a non-existent sibling src tree and the test will fail before exercising the target. Build the path from TestEnvironment.GetMauiDirectory() or pass the repo-root path as an MSBuild property instead.
| </ApplicationArtifact> | ||
| </ItemGroup> | ||
| </Target> | ||
| <Target Name="WriteGetApplicationArtifactsMetadata" DependsOnTargets="SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata"> |
There was a problem hiding this comment.
ApplicationArtifact items and directly invokes the private _AddMauiApplicationArtifactMetadata target, so it does not prove the public GetApplicationArtifacts/publish extension path actually returns enriched platform artifacts. A regression in target ordering or platform artifact production could still ship while this test passes; prefer invoking the real public targets or depending through $(GetApplicationArtifactsDependsOn) in the harness.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 5 findings
See inline comments for details.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
❌ [critical] Build & MSBuild — Import path is broken on CI and on Linux/macOS
$(MSBuildThisFileDirectory) evaluates to the generated project directory under the integration-test temp root, not the MAUI repo root. On CI that makes ..\..\..\src\Workload\... resolve outside the checkout, and on Linux/macOS the backslashes are literal path characters rather than directory separators. The test can hard-fail before exercising the metadata target.
Resolve the targets file from C# using TestEnvironment.GetMauiDirectory() and inject/pass an absolute path with platform-correct separators.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
The generated maui template project already imports Microsoft.Maui.Sdk.After.targets through the installed workload. The explicit source import can append _AddMauiApplicationArtifactMetadata a second time and makes the GetApplicationArtifactsDependsOn string assertion ambiguous. Prefer a minimal non-MAUI MSBuild project for this unit-style target test, or otherwise ensure the source targets are imported exactly once.
| </ApplicationArtifact> | ||
| </ItemGroup> | ||
| </Target> | ||
| <Target Name="WriteGetApplicationArtifactsMetadata" DependsOnTargets="SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata"> |
There was a problem hiding this comment.
WriteGetApplicationArtifactsMetadata and WritePublishApplicationArtifactsMetadata depend directly on _AddMauiApplicationArtifactMetadata, so the test would still pass if GetApplicationArtifactsDependsOn were removed, misspelled, or pointed at the wrong target. Invoke the public target/dependency chain and verify the enriched @(ApplicationArtifact) items are returned through that path.
| Assert.Contains("_AddMauiApplicationArtifactMetadata", File.ReadAllText(dependsOnFile), StringComparison.Ordinal); | ||
| AssertApplicationArtifactMetadata(File.ReadAllLines(getArtifactsFile)); | ||
|
|
||
| Assert.True(DotnetInternal.Build(projectFile, "Debug", target: "WritePublishApplicationArtifactsMetadata", framework: $"{DotNetCurrent}-android", properties: BuildProps, output: _output), |
There was a problem hiding this comment.
The publish assertion builds a custom target with the same direct dependency on _AddMauiApplicationArtifactMetadata, so it does not verify whether the real Publish path triggers artifact enrichment. Either document and test that Publish consumes the same GetApplicationArtifactsDependsOn extension path, or add the missing publish hook and verify it through the real publish target.
| </ItemGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <GetApplicationArtifactsDependsOn> |
There was a problem hiding this comment.
💡 [minor] Build & MSBuild — Consider guarding the dependency-chain extension
The property group appends _AddMauiApplicationArtifactMetadata for every project importing Microsoft.Maui.Sdk.After.targets, including configurations such as UseMauiEssentials=true that may never produce application artifacts. The target is a no-op when @(ApplicationArtifact) is empty, so this is likely harmless, but a UseMaui/artifact-intent condition or documentation would make the global extension explicit.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
[critical] Build & MSBuild — This import resolves from $(MSBuildThisFileDirectory), which is the generated temp test project directory, not the repo root. In CI the path walks up from the temp project and can miss src/Workload/Microsoft.Maui.Sdk/Sdk/Microsoft.Maui.Sdk.After.targets, causing MSB4019 before the regression test can run. Use the repo MAUI directory/test environment path instead of a project-relative path.
| </ApplicationArtifact> | ||
| </ItemGroup> | ||
| </Target> | ||
| <Target Name="WriteGetApplicationArtifactsMetadata" DependsOnTargets="SeedApplicationArtifacts;_AddMauiApplicationArtifactMetadata"> |
There was a problem hiding this comment.
[major] Regression Prevention/Test Coverage — The test directly depends on the private _AddMauiApplicationArtifactMetadata target here and again in WritePublishApplicationArtifactsMetadata, so it can pass even if the production GetApplicationArtifactsDependsOn hook is not actually used by GetApplicationArtifacts/Publish. Exercise the public depends-on path rather than invoking the private target directly.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| <ApplicationDisplayVersion>2.3</ApplicationDisplayVersion> | ||
| <ApplicationVersion>42</ApplicationVersion> | ||
| </PropertyGroup> | ||
| <Import Project="$([System.IO.Path]::GetFullPath('$(MSBuildThisFileDirectory)..\..\..\src\Workload\Microsoft.Maui.Sdk\Sdk\Microsoft.Maui.Sdk.After.targets'))" /> |
There was a problem hiding this comment.
[major] Build & MSBuild / integration test correctness — This import resolves relative to the temporary test project directory, not the MAUI repo root. In CI TestDirectory is under AGENT_TEMPDIRECTORY/test-dir, and locally it is under bin/test-dir, so $(MSBuildThisFileDirectory)..\..\..\src\Workload\... points at a non-existent src sibling and the test will fail before exercising the metadata target. It can also double-import Microsoft.Maui.Sdk.After.targets when the SDK already imports it. Use TestEnvironment.GetMauiDirectory() to build an absolute path if the test must import source targets, or rely on the SDK import from the built workload.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Align MAUI augmentation with the landed Android and Apple artifact contracts, including Android publish item recreation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b83c8e-9ff9-4712-87cf-f7a4c970f792
There was a problem hiding this comment.
Pull request overview
Adds .NET MAUI-provided metadata defaults onto the shared @(ApplicationArtifact) item type so that platform SDKs’ app artifact items (including items recreated during publish) automatically carry consistent MAUI application metadata for net11p6 scenarios.
Changes:
- Define
ApplicationArtifactitem-definition defaults inMicrosoft.Maui.Sdk.After.targetsfor MAUI app metadata (ID, title/name, versions), without overriding platform-supplied metadata. - Add an integration test that seeds
@(ApplicationArtifact)items and then simulates an Android-style “publish recreation” to verify the MAUI metadata persists via item-definition defaults. - Add a design doc describing ownership boundaries and the metadata enrichment model for
@(ApplicationArtifact).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Workload/Microsoft.Maui.Sdk/Sdk/Microsoft.Maui.Sdk.After.targets | Adds ItemDefinitionGroup defaults for MAUI ApplicationArtifact metadata driven by project properties. |
| src/TestUtils/src/Microsoft.Maui.IntegrationTests/SimpleTemplateTest.cs | Adds a test that validates MAUI metadata is present on both initial and recreated @(ApplicationArtifact) items. |
| docs/design/ApplicationArtifact.md | Documents how platform SDKs own artifact creation while MAUI supplies shared metadata defaults. |
| <ItemDefinitionGroup> | ||
| <ApplicationArtifact> | ||
| <ApplicationId Condition="'$(ApplicationId)' != ''">$(ApplicationId)</ApplicationId> | ||
| <ApplicationIdGuid Condition="'$(ApplicationIdGuid)' != ''">$(ApplicationIdGuid)</ApplicationIdGuid> | ||
| <ApplicationName Condition="'$(ApplicationTitle)' != ''">$(ApplicationTitle)</ApplicationName> | ||
| <ApplicationTitle Condition="'$(ApplicationTitle)' != ''">$(ApplicationTitle)</ApplicationTitle> | ||
| <ApplicationDisplayVersion Condition="'$(ApplicationDisplayVersion)' != ''">$(ApplicationDisplayVersion)</ApplicationDisplayVersion> | ||
| <ApplicationVersion Condition="'$(ApplicationVersion)' != ''">$(ApplicationVersion)</ApplicationVersion> | ||
| </ApplicationArtifact> | ||
| </ItemDefinitionGroup> |
There was a problem hiding this comment.
Should this actually go in dotnet/android and dotnet/macios?
What if you are an Uno or MonoGame project? ... then what about WindowsAppSDK?
There was a problem hiding this comment.
Yeah I think maybe the android/macios stuff can get pushed down into those repos - i'll make PR's for that. For windows I'm thinking maybe another package that could be added to any windowsappsdk app that achieves the same, though I'll check with someone on that team if they might consider just a PR to their repo directly to add this too.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e7b83c8e-9ff9-4712-87cf-f7a4c970f792
Redth
changed the title
| if (TestEnvironment.IsMacOS) | ||
| { | ||
| if (true) return; // Skip: "This test is designed for testing a windows build." | ||
| if (true) | ||
| return; // Skip: "This test is designed for testing a windows build." | ||
| } |
There was a problem hiding this comment.
I would maybe just discard the changes like this in this file as it's whitepace only for no reason.
| if (TestEnvironment.IsMacOS) | ||
| { | ||
| if (true) return; // Skip: "This test is designed for testing a windows build." | ||
| if (true) | ||
| return; // Skip: "This test is designed for testing a windows build." | ||
| } |
| if (!TestEnvironment.IsWindows) | ||
| { | ||
| if (true) return; // Skip: "Running Windows templates is only supported on Windows." | ||
| if (true) | ||
| return; // Skip: "Running Windows templates is only supported on Windows." | ||
| } |
| if (!TestEnvironment.IsWindows) | ||
| { | ||
| if (true) return; // Skip: "Running Windows templates is only supported on Windows." | ||
| if (true) | ||
| return; // Skip: "Running Windows templates is only supported on Windows." | ||
| } |
| if (!TestEnvironment.IsWindows) | ||
| { | ||
| if (true) | ||
| return; // Skip: "Running Windows templates is only supported on Windows." | ||
| } |
| if (!TestEnvironment.IsWindows) | ||
| { | ||
| if (true) | ||
| return; // Skip: "Running Windows templates is only supported on Windows." | ||
| } |
| if (!TestEnvironment.IsWindows) | ||
| { | ||
| if (true) | ||
| return; // Skip: "Running Windows templates is only supported on Windows." | ||
| } |
| if (!TestEnvironment.IsWindows) | ||
| { | ||
| if (true) | ||
| return; // Skip: "Packing the Windows Core target framework is only supported on Windows CI." | ||
| } |
## Why this change is necessary The platform SDK owns the authoritative final application artifact metadata for all Android consumers. Project properties are not sufficient because custom manifests can override them and `GenerateApplicationManifest=false` bypasses generated values entirely. This follows the `@(ApplicationArtifact)` contract introduced by [#11674](#11674) and provides the producer-owned metadata consumed by [dotnet/maui#35973](dotnet/maui#35973) without making the values MAUI-specific defaults. ## What changed - Extended the existing final merged-manifest read path to expose the resolved package, application `android:label`, `android:versionName`, and `android:versionCode`. - Stamped every APK/AAB `@(ApplicationArtifact)` with: - `ApplicationId` - `ApplicationTitle` - `ApplicationName` - `ApplicationDisplayVersion` - `ApplicationVersion` - Preserved `PackageId`, `PackageFormat`, `Signed`, and per-ABI `Abi` metadata. - Preserved all producer-owned common and Android metadata when `Publish` recreates artifact items under `$(PublishDir)`. - Kept `GetApplicationArtifactsDependsOn` overrides effective for both direct target results and Publish recreation. - Kept resource-backed labels such as `@string/app_name` unchanged instead of resolving a locale. - Documented final merged-manifest precedence and resource-reference behavior. ## Tests Focused host-side coverage validates generated manifests, custom manifest precedence, `GenerateApplicationManifest=false`, resource-backed labels, APK/AAB signed and unsigned outputs, per-ABI items, extension overrides, `GetApplicationArtifacts`, and Publish recreation. ```text Passed: 9 Failed: 0 Skipped: 0 Duration: 40 s ``` Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…6110) ## Summary Adds producer-owned common application metadata to every Apple `@(ApplicationArtifact)` item. - Extends `ReadAppManifest` with `CFBundleName` and `CFBundleShortVersionString`. - Reads metadata from the final compiled app bundle `Info.plist`, so custom manifest values win. - Preserves unresolved or localized plist references as written instead of selecting a locale. - Stamps `.app`, `.ipa`, `.pkg`, and `.xcarchive` artifacts with `ApplicationId`, `ApplicationTitle`, `ApplicationName`, `ApplicationDisplayVersion`, and `ApplicationVersion` while retaining existing Apple metadata. - Runs before downstream `GetApplicationArtifactsDependsOn` extensions, preserving their ability to override producer metadata. - Documents the common metadata contract and adds task/post-build coverage across Apple platforms, output formats, generated/custom manifests, `GenerateApplicationManifest=false`, `GetApplicationArtifacts`, and `Publish`. Follow-up to #25723. Coordinated with dotnet/maui#35973 and dotnet/android#12123. --------- Co-authored-by: redth <jondick@gmail.com>
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
MauiBot
added
the
s/agent-changes-requested
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@Redth — new AI review results are available based on this last commit:
860afcb.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ⚠️ SKIPPED
No tests were detected in this PR.
Recommendation: Add tests to verify the fix using the write-tests-agent.
📱 UI Tests — Button,Label,Layout
Detected UI test categories: Button,Label,Layout
✅ Deep UI tests — 339 passed, 0 failed across 3 categories on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Button |
65/66 ✓ | — |
Label |
91/94 ✓ | — |
Layout |
183/187 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
📋 Pre-Flight — Context & Validation
Issue: #35973 - Add ApplicationArtifact support for Windows App SDK
PR: #35973 - Add ApplicationArtifact support for Windows App SDK
Platforms Affected: Windows build/publish tooling; package/workload insertion surfaces
Files Changed: 15 implementation/documentation/build, 2 test
Key Findings
- PR adds a targets-only
Microsoft.Maui.ApplicationArtifacts.Windowspackage that definesGetApplicationArtifactsfor Windows App SDK projects and adds the package to workload manifests/solutions/package flows. - Current implementation collects Windows App SDK build/publish outputs from authoritative MSBuild properties/items and classifies primary packages, payload directories, bundles, store uploads, appinstaller manifests, symbols, certs, install scripts, dependencies, landing pages, and support files.
- Prior review comments on an earlier commit identified CI-invalid test imports and weak hook coverage; current head no longer contains that
SimpleTemplateTest.cspath-based import and now uses Windows-specific integration coverage. - New expert review risk: the target works around MSBuild single-execution behavior for unpackaged publish with a special refresh target, but packaged publish collection still reuses
_CollectMicrosoftMauiWindowsApplicationArtifacts, which may be stale ifGetApplicationArtifactsran beforePublishin the same project instance. - Gate was skipped externally because no tests were detected by the gate phase; per instructions, gate verification was not re-run.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 1 | Suggestions: 1
Key code review findings:
⚠️ src/Workload/Microsoft.Maui.ApplicationArtifacts.Windows/buildTransitive/Microsoft.Maui.ApplicationArtifacts.Windows.targets:38publish collection depends on the same_CollectMicrosoftMauiWindowsApplicationArtifactstarget used byGetApplicationArtifacts; MSBuild executes a target once, soGetApplicationArtifacts;Publishcan leave packaged publish-path artifacts stale forAfterTargets="Publish"consumers.- 💡
src/TestUtils/src/Microsoft.Maui.IntegrationTests/WindowsApplicationArtifactsPackageTest.cs:412has synthetic coverage for unpackagedWriteBuildApplicationArtifacts;Publish, but not an equivalent packaged same-invocation stale-target scenario.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35973 | Add a new Windows App SDK ApplicationArtifact producer package, reference it from Core/workload manifest, and cover synthetic + template Windows artifact scenarios |
src/Workload/Microsoft.Maui.ApplicationArtifacts.Windows/**, src/Core/src/Core.csproj, integration tests, workload/package files |
Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #35973
Independent Assessment
What this changes: Adds a targets-only Windows App SDK artifact producer package and wires it into MAUI package/workload flows. The target creates @(ApplicationArtifact) items for Windows build/publish outputs and enriches them with common identity/version metadata.
Inferred motivation: Complete the cross-platform ApplicationArtifact contract for Windows so downstream consumers can inspect final app artifacts without MAUI scanning output directories.
Reconciliation with PR Narrative
Author claims: Windows should produce authoritative ApplicationArtifact items from WinAppSDK output properties/items, support unpackaged/MSIX/sparse outputs, and expose metadata for MAUI and standalone Windows App SDK consumers.
Agreement/disagreement: The implementation broadly matches the narrative and current tests cover many roles. The remaining concern is target re-execution semantics when build artifact collection and publish happen in a single MSBuild project instance.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
| Earlier integration test imported MAUI targets using a generated-project-relative path invalid in CI | kubaflo / MauiBot prior reviews on commit 45c2182 | ✅ Obsolete | Current PR head no longer changes SimpleTemplateTest.cs; Windows artifact tests use TestEnvironment.GetMauiDirectory() or normal template/package imports. |
| Earlier test invoked a private metadata target directly instead of the public hook | kubaflo / MauiBot prior reviews on commit 45c2182 | ✅ Obsolete | Current tests exercise GetApplicationArtifacts, package references, and Windows template build/publish scenarios. |
Blast Radius Assessment
- Runs for all instances: Only for projects with Windows target platform, Windows App SDK package directory,
Exe/WinExe, non-design-time, and not.wapproj. - Startup impact: No runtime startup impact; build/publish target graph only.
- Static/shared state: No runtime static state; MSBuild item/property state can persist within a project instance and is the main failure mode.
CI Status
- Required-check result: undetermined locally because
ghis unauthenticated; gate result supplied by caller was skipped. - Classification: coverage gap / undetermined.
- Action taken: Capped confidence low and generated try-fix candidates instead of marking LGTM.
Findings
⚠️ Warning — Packaged publish collection can be stale after build artifact collection
src/Workload/Microsoft.Maui.ApplicationArtifacts.Windows/buildTransitive/Microsoft.Maui.ApplicationArtifacts.Windows.targets:38
GetApplicationArtifacts and the publish hook both depend on _CollectMicrosoftMauiWindowsApplicationArtifacts. MSBuild executes a target at most once per project instance. The PR handles the same-invocation problem for unpackaged publish with _RefreshMicrosoftMauiWindowsUnpackagedApplicationArtifactsForPublish, but packaged publish still relies on the shared collector target. If a consumer runs GetApplicationArtifacts;Publish, AfterTargets="Publish" consumers can observe build-time packaged artifacts rather than a freshly collected publish set.
💡 Suggestion — Add packaged same-invocation coverage
src/TestUtils/src/Microsoft.Maui.IntegrationTests/WindowsApplicationArtifactsPackageTest.cs:412
The synthetic unpackaged test covers WriteBuildApplicationArtifacts;Publish, but packaged synthetic coverage should also prove that publish-only packaged outputs are visible to AfterTargets="Publish" consumers when GetApplicationArtifacts ran earlier in the invocation.
Failure-Mode Probing
GetApplicationArtifacts;Publishfor unpackaged app: PR has a dedicated refresh target and synthetic coverage.GetApplicationArtifacts;Publishfor packaged app: shared collector may already be marked complete, leaving staleApplicationArtifactitems.- Design-time build: disabled by target conditions.
- Non-Windows or library project: disabled by target conditions.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: The product direction and most implementation details are sound, but packaged publish target ordering deserves either a target-graph fix or explicit regression coverage proving the current graph is safe.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Add packaged publish refresh target that rematerializes previously collected packaged items | ❌ Fail | 1 target file | Smoke test passed, but self-review found it preserves stale cached item identities. |
| 2 | try-fix-2 | Remove main collector from publish path and re-evaluate packaged Primary/PayloadDirectory from publish-time properties |
❌ Fail | 1 target file | Focused stale-primary synthetic test passed, but auxiliary packaged roles can remain stale/missing. |
| 3 | try-fix-3 | Use nested MSBuild project instance to bypass target-once semantics and return fresh publish artifacts | ❌ Fail | 1 target file | Refreshed property-backed roles, but lost item-backed outputs such as AppInstallerFilePath/DeploymentManifest. |
| PR | PR #35973 | Targets-only Windows App SDK artifact producer with synthetic and template integration coverage | 17 files | Current PR remains preferable to failed alternatives; consider adding packaged same-invocation regression coverage. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| gpt-5.5 / maui-expert-reviewer | 1 | Yes | Identified target-once risk for packaged GetApplicationArtifacts;Publish; proposed separate build/publish collectors. |
| claude-opus-4.6 / maui-expert-reviewer | 2 | Yes | Proposed publish hook self-sufficiency via current publish-time properties; became try-fix-2. |
| claude-opus-4.7 / maui-expert-reviewer | 3 | Yes | Proposed nested MSBuild fresh instance; became try-fix-3. |
| gpt-5.3-codex / maui-expert-reviewer | 4 | No runnable candidate | Suggested a non-cached same-instance recollector after packaging, but that requires broad collector-body extraction/duplication or a test-only change, both outside a meaningfully better surgical alternative. |
Exhausted: Yes — meaningful alternatives were explored across cached rematerialization, publish-time property refresh, and nested project-instance refresh. Remaining options are either broad refactors duplicating the collector body or test-only additions.
Selected Fix: PR #35973 — no alternative candidate passed all regression checks or was demonstrably better. Recommended follow-up is to add a focused packaged GetApplicationArtifacts;Publish same-invocation regression test if the team wants to lock down the suspected target-ordering edge case.
🏁 Report — Final Recommendation
Comparative Analysis — PR #35973
Candidates compared
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr |
Gate skipped; no candidate regression failure recorded | Best available implementation. It adds the Windows App SDK ApplicationArtifact producer, package/workload wiring, and broad synthetic plus Windows template coverage. Expert review found one packaged same-invocation publish risk, but no STEP 5a alternative resolved that risk without failing another scenario. |
| 2 | pr-plus-reviewer |
Not a complete production fix; reviewer feedback is test-only | Applies the expert reviewer's safely actionable feedback as a sandbox coverage candidate: add packaged GetApplicationArtifacts;Publish regression coverage. This is useful follow-up work, but without a safe production target-graph change it does not improve the shipped fix and may simply expose the unresolved edge. |
| 3 | try-fix-2 |
Failed | Best failed alternative. It refreshes packaged Primary and PayloadDirectory from publish-time properties and passed the focused stale-primary assertion, but it leaves auxiliary packaged roles such as Bundle, StoreUpload, DeploymentManifest, Symbols, Certificate, InstallScript, DependencyPackage, and Support stale or missing. |
| 4 | try-fix-3 |
Failed | Bypasses target-once semantics with a nested MSBuild project instance and refreshes property-backed roles, but loses target-created item-backed outputs such as AppInstallerFilePath/DeploymentManifest unless broad serialization plumbing is added. |
| 5 | try-fix-1 |
Failed | Adds a packaged publish refresh target, but it re-adds @(_MicrosoftMauiWindowsApplicationArtifactDistinct) from the previous collector run, preserving stale identities rather than forcing fresh publish collection. |
Winning candidate
Winner: pr
The raw PR is the single winning candidate because every STEP 5a try-fix candidate failed regression/self-review checks, and candidates that failed regression tests must rank below candidates without a recorded regression failure. The reviewer-enhanced variant is valuable as follow-up test coverage but is not a complete production fix; it does not safely address the packaged target-once issue that the reviewer identified.
Recommended follow-up
Before merge, the author should add packaged same-invocation coverage for GetApplicationArtifacts;Publish or otherwise prove that packaged publish ApplicationArtifact items are fresh for AfterTargets="Publish" consumers. If that test exposes the suspected stale-state failure, a broader packaged recollection design is needed rather than the simple STEP 5a alternatives.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| and '$(DesignTimeBuild)' != 'true' | ||
| and '$(BuildingProject)' == 'true'" | ||
| BeforeTargets="Publish" | ||
| DependsOnTargets="_PrepareMicrosoftMauiWindowsApplicationArtifactsForPublish;_CollectMicrosoftMauiWindowsApplicationArtifacts;_RefreshMicrosoftMauiWindowsUnpackagedApplicationArtifactsForPublish" /> |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Build & MSBuild — _CollectMicrosoftMauiWindowsApplicationArtifactsForPublish still depends on _CollectMicrosoftMauiWindowsApplicationArtifacts, the same target that GetApplicationArtifacts runs. MSBuild executes a target only once per project instance, so a packaged Windows consumer that invokes GetApplicationArtifacts;Publish will not recollect after GenerateMsixPackage has produced publish-time AppPackages properties/items; AfterTargets="Publish" consumers can keep stale build-path ApplicationArtifact items or miss publish-created roles. The PR already adds a distinct unpackaged refresh for this target-once issue, but the packaged path needs equivalent fresh collection or regression coverage proving this same-invocation path is safe.
MauiBot
removed
the
s/agent-review-in-progress
| "Microsoft.Maui.ApplicationArtifacts.Windows": { | ||
| "kind": "library", | ||
| "version": "@VERSION@" | ||
| }, |
There was a problem hiding this comment.
Why do we need a new package? If this is meant to be optional, could it just be in the Build.Tasks package instead? You'd opt into it with an MSBuild property.
| if (TestEnvironment.IsMacOS) | ||
| { | ||
| if (true) return; // Skip: "This test is designed for testing a windows build." | ||
| if (true) | ||
| return; // Skip: "This test is designed for testing a windows build." | ||
| } |
There was a problem hiding this comment.
I would maybe just discard the changes like this in this file as it's whitepace only for no reason.
| @@ -0,0 +1,21 @@ | |||
| <Project Sdk="Microsoft.Build.NoTargets"> | |||
There was a problem hiding this comment.
It feels like removing this project (and putting the changes in an existing one) would significantly simplify this PR: ProjectReference, sln files, etc.
| ## Windows App SDK | ||
|
|
||
| `Microsoft.Maui.ApplicationArtifacts.Windows` is a targets-only `buildTransitive` package with no MAUI runtime, Resizetizer, or MAUI build-task dependency. `Microsoft.Maui.Core` depends on it only for Windows target frameworks, and non-MAUI Windows App SDK applications can reference it directly. It activates for real, non-design-time Windows App SDK application builds with `Exe` or `WinExe` output. Set `EnableWindowsApplicationArtifacts` to `false` to disable it. Classic `.wapproj` packaging is not supported. |
There was a problem hiding this comment.
Still not understanding why a new package is needed at all.
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 of Change
Adds
ApplicationArtifactsupport for modern SDK-style Windows App SDK applications through a new targets-onlyMicrosoft.Maui.ApplicationArtifacts.Windowspackage.Artifact metadata is now owned by the platform that produces the final output instead of being inferred by MAUI from project inputs. Android and Apple expose authoritative common metadata in their platform follow-ups:
This PR supplies the missing Windows producer. It reads WinAppSDK's final output properties and item lists rather than searching output directories, de-duplicates normalized paths, excludes
objintermediates, and makes publish-path items available toAfterTargets="Publish"consumers.Package and consumption
Microsoft.Maui.ApplicationArtifacts.Windowsis a normal NuGet dependency package containing only a neutralbuildTransitivetarget. It has no MAUI runtime, Resizetizer, or MAUI build-task dependency.Microsoft.Maui.Corereferences it only for Windows target frameworks, and themaui-windowsworkload includes it. Non-MAUI WinUI, Uno, and other SDK-style Windows App SDK applications can also reference the package directly.The targets activate only for real, non-design-time Windows App SDK application builds with
ExeorWinExeoutput. SetEnableWindowsApplicationArtifacts=falseto opt out.Windows outputs
The producer supports unpackaged, MSIX, and sparse applications and classifies the final outputs WinAppSDK actually creates:
PrimaryPayloadDirectoryBundleStoreUploadDeploymentManifestSymbolsCertificateInstallScriptDependencyPackageLandingPageSupportEvery item carries common application identity/title/version metadata plus Windows metadata such as
PackageFormat,PackageType,PackageVersion,Architecture,RuntimeIdentifier,Signed,BundlePlatforms,EntryPoint,DeploymentDirectory,IsPrimary, andPrimaryArtifactwhen available.Packaged metadata comes from the final Appx manifest and preserves unresolved
ms-resource:references. Unpackaged metadata comes from the resolved application output.ApplicationIdGuidis emitted only when the effective Windows identity is a GUID.Classic
.wapprojpackaging is intentionally deferred because it uses a different packaging pipeline owned outside this repository.Testing
PackageReferencerestore ofMicrosoft.Maui.ApplicationArtifacts.Windowsis verified.GetApplicationArtifacts;Publishin one MSBuild invocation..appinstaller, then verifiesBundle,StoreUpload, andDeploymentManifestroles.Issues Fixed
N/A