Repository navigation
Handle required architecture publish profiles in winapp run - #788
Conversation
Select a matching self-contained publish profile only when a trimmed framework-dependent project requires it, while preserving RID-only and guarded Platform behavior for existing project graphs. Add regression coverage and update generated CLI documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
…run-architecture # Conflicts: # src/winapp-npm/src/winapp-commands.ts
Align inferred profile resolution with the .NET SDK, escape MSBuild property separators, dispose test resources, and synchronize project-mode documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
Build Metrics ReportBinary Sizes
Test Results✅ 4815 passed, 5 skipped out of 4820 tests in 778.7s (+29 tests, -24.4s vs. baseline) Test Coverage✅ 88.9% line coverage, 82.3% branch coverage · ✅ no change vs. baseline CLI Startup Time61ms median (x64, Try This BuildInstalls the MSIX for your architecture, replacing any previously installed build. Needs the GitHub CLI — the command offers to install it and sign you in if it is missing. & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 788Switching between builds often?Put the tool on your PATH once: & ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPathThen this build is just: winapp-pr 788Run Updated 2026-09-09 04:43:56 UTC · commit |
There was a problem hiding this comment.
Pull request overview
Adds architecture-aware publish-profile inference to winapp run, keeping project-reference builds aligned across restore, build, and output evaluation.
Changes:
- Resolves safe, self-contained architecture profiles for trimmed builds.
- Propagates inferred profiles across build stages and solution restore handling.
- Adds regression tests and updates CLI documentation/generated surfaces.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/winapp-npm/src/winapp-commands.ts |
Updates generated npm API descriptions. |
src/winapp-CLI/WinApp.Cli/Services/ProjectRunService.PublishProfile.cs |
Implements profile discovery and validation. |
src/winapp-CLI/WinApp.Cli/Services/ProjectRunService.cs |
Integrates resolution and fallback behavior. |
src/winapp-CLI/WinApp.Cli/Services/ProjectRunService.Arguments.cs |
Forwards profiles across MSBuild passes. |
src/winapp-CLI/WinApp.Cli/Models/ProjectRunModels.cs |
Adds the resolved profile option. |
src/winapp-CLI/WinApp.Cli/Helpers/RunArchHelper.cs |
Updates architecture documentation. |
src/winapp-CLI/WinApp.Cli/Commands/RunCommand.cs |
Updates option help text. |
src/winapp-CLI/WinApp.Cli.Tests/ProjectRunServicePublishProfileTests.cs |
Adds profile-selection regression tests. |
plugins/winapp/skills/winapp-setup/SKILL.md |
Documents profile-aware project runs. |
plugins/winapp/com.github.copilot/agents/winapp.agent.md |
Updates agent command guidance. |
docs/usage.md |
Updates the canonical run reference. |
docs/npm-usage.md |
Updates npm API documentation. |
docs/guides/dotnet.md |
Explains multi-project profile handling. |
docs/cli-schema.json |
Regenerates CLI option descriptions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use MSBuild to resolve conditioned publish profiles and preserve existing PublishProfileName, PublishProfileFullPath, and WebPublishProfileFile selections. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
PR Review — nmetulev-investigate-run-architecture vs mainDecisionChanges required — inferred publish profiles can silently change the app’s target framework. The published CLI reproduced this with a normal stale Visual Studio profile. Must fixSelected publish profile can silently retarget the app
Non-blockingImported and conditional profile declarations bypass normal MSBuild evaluation
What was exercised
|
Zach Teutsch (zateutsch)
left a comment
There was a problem hiding this comment.
Review above, one blocking issue.
The non-blocking issue looks like it might be worth resolving as well to me, but not sure if thats a common case.
DecisionMerge. I built the CLI and reproduced the failure this fixes: a trimmed WinUI app with an Two non-blocking items below. Neither affects behavior. The .NET guide dropped the error users actually search forWhat is wrong: The rewritten "Multi-project apps" paragraph replaces the observable symptom with internal vocabulary — "RID-only", "when the effective configuration requires a self-contained profile". Show me:
The searchable error code is gone. Why it matters: A user hits Smallest fix: Keep the Location: Nothing automated guards the SDK behavior this depends onWhat is wrong: The 19 new tests drive the pipeline through Show me: I confirmed by hand that the contract holds today — a trimmed, framework-dependent WinUI build fails Why it matters: The trigger is entirely SDK-behavior-dependent, and this is exactly the boundary Smallest fix: Location: What was exercised
Not exercised, so treat these as reasoned rather than proven:
|
Reject inferred publish profiles that retarget the app, resolve imported profiles through MSBuild, and add a real winui-app SDK boundary test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
|
Addressed the latest feedback from Zach Teutsch (@zateutsch) and Alexandre Zollinger Chohfi (@azchohfi).
Validation: 208 project-run tests passed; the winui-app Pester sample passed 4/4; the merged repository build and package generation completed successfully. |
|
Addressed the latest feedback from Zach Teutsch (@zateutsch) and Alexandre Zollinger Chohfi (@azchohfi) in 8d1e5b8.
Validation is green: 208 focused project-run tests, the local winui-app Pester test, the repository build/package flow, and the complete PR CI matrix (including |
There was a problem hiding this comment.
🟡 Changes recommended
Profile propagation checks miss imported and custom-root references, while packaged-option preflight can reject an app before its required profile is resolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 3
- Review effort level: Balanced
Use MSBuild's root-project publish-profile scope, validate the final inferred profile, and align packaging preflight with the real build inputs. Add unit and SDK-level regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
DecisionChanges required — the publish-profile inference works and is well-tested, but (1) it silently switches a class of builds that would have succeeded RID-only, and (2) the new comma-as-separator handling isn't mirrored in secret redaction, so a user secret can leak into logs. Everything else (CLI UX, docs/schema/npm sync, structure, scope) came back clean. Must fixSecret in a comma-packed MSBuild property is printed unredacted
Inference switches valid framework-dependent trimmed builds onto a self-contained profile
Non-blockingNone. What was exercised
|
Zach Teutsch (zateutsch)
left a comment
There was a problem hiding this comment.
Few more findings attached above.
Limit automatic self-contained profile selection to trimmed MSIX-tooling builds, preserve trimming, and harden MSBuild property validation and secret redaction for comma-separated inputs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
|
Addressed in afeb3fd.
I kept inference before the build rather than deliberately failing and rebuilding, so restore/build/output evaluation remain one consistent pass. Added regression coverage for the valid framework-dependent case, profiles that disable trimming, packed explicit selectors/frameworks, and secret redaction. |
Keep generated command surfaces concise and leave separator escaping guidance in the detailed usage documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bd0c992-277e-447f-bedd-3268a4e74abf
Merge conflicts were mostly option-description edits where main (#788, publish profiles) and this PR each extended the same sentence; both intents are kept. Two were substantive. Main added ',' to the -p packing rejection, alongside ';'. This PR had extracted that validation into MsBuildPropertyValidator so unregister could share it, so taking either side wholesale would have lost one of them -- the ',' rejection is ported into the shared validator, which main's own ProjectMode_CommaPackedProperty test now covers. A publish-profile value like 'win-arm64,Extra=true' would otherwise split into two properties. Main's ForwardableProperties list gained the publish-profile properties while this PR added WinAppRunUseExecutionAlias; the list keeps both. Three review findings, all confirmed first: unregister used AcceptExistingOnly() on its input and --manifest, which System.CommandLine enforces during parsing -- before the handler runs, so a missing path printed the plain-text help page and bypassed --json entirely. That is exactly the case cleanup automation has to parse. Existence is now checked in the handler through FailWith, as RunCommand already did for the same reason. An inferred execution alias that could not be used fell back to AUMID at Debug level. Reaching that path means winapp inferred alias launch, which it only does for a console app, so the fallback launches it with no console and it prints nothing -- the silent success this feature exists to remove. The run still succeeds, but now says why the output is missing. The third finding, cross-publisher removal under --force, is pre-existing: FindDevPackages has always matched Identity/@name alone and --force has always bypassed the ownership check, both unchanged here. Filtering by package family is a behavior change to a service shared with run cleanup and --prune, so it belongs in its own PR. Since this PR is what points users at --force, its guidance now states the caveat. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 347dc68d-3b96-4846-93ec-81b1a1ffa407
Description
Fixes
winapp run --archfor project graphs where the app chooses a publish profile through$(Platform), the effective configuration enables trimming without self-containment, and forcing a global MSBuildPlatformwould be unsafe for anAnyCPUproject reference.The command now:
Platformpaths by defaultPublishTrimmed,PublishAot, andSelfContainedbefore building--no-buildoutput discovery alignedUsage Example
No additional property is required when the project already declares matching architecture publish profiles.
Related Issue
N/A
Type of Change
Checklist
Screenshots / Demo
N/A
Additional Notes