Conversation
…existing packages, add csproj editing for enabling packaging with comments
Contributor
There was a problem hiding this comment.
Pull request overview
Updates winapp init to be more non-destructive for existing .NET project package versions, and adds an opt-in flow for installing the WinApp “dotnet run packaged” integration with associated csproj tweaks.
Changes:
- Prompt user to optionally install
Microsoft.Windows.SDK.BuildTools.WinAppand gate packaged-app-related csproj edits behind that choice. - Preserve existing top-level package versions found in the project (except always updating the WinApp integration package).
- Add csproj helpers to ensure
EnableMsixTooling=true, removeWindowsPackageType=None, and annotate PackageReferences with explanatory comments.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/winapp-CLI/WinApp.Cli/Services/WorkspaceSetupService.cs | Adds WinApp package opt-in prompt; preserves existing package versions; conditionally updates csproj and annotates packages. |
| src/winapp-CLI/WinApp.Cli/Services/IDotNetService.cs | Extends interface with new csproj-editing helpers. |
| src/winapp-CLI/WinApp.Cli/Services/DotNetService.cs | Implements csproj regex-based edits for MSIX properties and PackageReference annotations. |
| src/winapp-CLI/WinApp.Cli.Tests/FakeDotNetService.cs | Updates test fake to delegate new csproj-editing helper methods to the real implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Build Metrics ReportBinary Sizes
Test Results✅ 702 passed out of 702 tests in 439.1s (+4 tests, +37.9s vs. baseline) Test Coverage❌ 20.9% line coverage, 36.7% branch coverage · ✅ no change vs. baseline CLI Startup Time41ms median (x64, Updated 2026-04-09 03:39:49 UTC · commit |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/bc7a49b1-9b1a-4b9d-93c7-5e8a9c8be379 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
#395, #399, #401: Init command improvements
Init: skip WinApp prompt when already installed, fix EnsureEnableMsixTooling no-op writes
Apr 7, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+488
to
+501
| var ridMatch = RuntimeIdentifierElementRegex().Match(content); | ||
| if (ridMatch.Success) | ||
| { | ||
| // Find end of the RuntimeIdentifier line | ||
| var lineEnd = content.IndexOf('\n', ridMatch.Index); | ||
| if (lineEnd >= 0) | ||
| { | ||
| var insertPos = lineEnd + 1; | ||
| content = content[..insertPos] | ||
| + " " + element + Environment.NewLine | ||
| + content[insertPos..]; | ||
| modified = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Insertion after an existing <RuntimeIdentifier> is based on finding the first newline after the <RuntimeIdentifier start tag (RuntimeIdentifierElementRegex only matches the start), then inserting after that line. If the element is formatted across multiple lines, this will insert inside the element and break the XML. Safer options are to match the full <RuntimeIdentifier>...</RuntimeIdentifier> element (singleline) and insert after its end, or insert relative to ridMatch.Index + ridMatch.Length instead of the first newline.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/3f70e57d-31a0-467f-8080-6ac6147c4902 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Copilot
AI
changed the title
Init: skip WinApp prompt when already installed, fix EnsureEnableMsixTooling no-op writes
init: fix WinApp package opt-in flow ordering and add test coverage
Apr 7, 2026
Contributor
Author
|
@copilot apply changes based on the comments in this thread |
…ng tag Agent-Logs-Url: https://github.com/microsoft/winappCli/sessions/4cbc7abe-b7d7-4d7f-9044-60a9a065e326 Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
Contributor
Applied in commit 5ad4595. |
Copilot
AI
changed the title
init: fix WinApp package opt-in flow ordering and add test coverage
DotNet init: fix RuntimeIdentifier regex, reorder MSIX edits, harden WinApp opt-in
Apr 8, 2026
zateutsch
changed the title
…ions on actual package add (#416) `winapp init` was returning exit code 1 whenever NuGet version resolution failed for `Microsoft.Windows.SDK.BuildTools.WinApp`, and MSIX csproj properties were being mutated regardless of whether the WinApp integration package was actually added. ## Behavior changes ### `WorkspaceSetupService.cs` - **WinApp package opt-in prompt** added before the live display (Spectre.Console constraint). Automatically accepts when `--use-defaults` or when the package is already referenced; otherwise prompts the user. - **Non-fatal NuGet lookup**: `Microsoft.Windows.SDK.BuildTools.WinApp` is now `required: false` in the package list — a transient failure warns and continues rather than aborting init. - **`winAppPackageApplied` tracking**: `EnsureEnableMsixToolingAsync` and `RemoveWindowsPackageTypeNoneAsync` are only called when the WinApp package was successfully added/updated, not merely when the user opted in. - **Version preservation**: existing pinned package versions are kept during re-init (except the WinApp package, which is always refreshed). ```csharp // Before: always required — any NuGet failure aborted init packages.Add((DotNetService.WINDOWS_SDK_BUILD_TOOLS_WINAPP_PACKAGE, true)); // After: non-fatal; track actual success before mutating csproj packages.Add((DotNetService.WINDOWS_SDK_BUILD_TOOLS_WINAPP_PACKAGE, false)); // ... if (winAppPackageApplied) { await dotNetService.EnsureEnableMsixToolingAsync(csprojFile, cancellationToken); await dotNetService.RemoveWindowsPackageTypeNoneAsync(csprojFile, cancellationToken); } ``` ### `IDotNetService` / `DotNetService` New methods: `EnsureEnableMsixToolingAsync`, `RemoveWindowsPackageTypeNoneAsync`, `AnnotatePackageReferencesAsync`. Uses a separate `RuntimeIdentifierSimpleElementRegex` to avoid breaking the existing `EnsureRuntimeIdentifierAsync` which needs to match `<RuntimeIdentifier Condition="...">` forms. ## Tests added (`WorkspaceSetupServiceTests.cs`) - `SetupWorkspace_DotNet_InitSucceeds_WhenNuGetThrowsForWinAppPackage` — exit code 0 when NuGet throws for the WinApp package - `SetupWorkspace_DotNet_DoesNotApplyMsixProperties_WhenWinAppNotOptedIn` — `EnableMsixTooling` not written when user declines - `SetupWorkspace_DotNet_PreservesExistingPackageVersions` — pinned versions are kept - `SetupWorkspace_DotNet_SkipsWinAppPrompt_WhenPackageAlreadyReferenced` — no prompt when already referenced `FakeNugetService` gains a `PackagesToThrow` set to simulate per-package NuGet failures. > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `https://api.github.com/repos/microsoft/msstore-cli/releases/latest` > - Triggering command: `/home/REDACTED/work/winappCli/winappCli/src/winapp-CLI/WinApp.Cli.Tests/bin/Debug/net10.0-windows10.0.19041.0/WinApp.Cli.Tests /home/REDACTED/work/winappCli/winappCli/src/winapp-CLI/WinApp.Cli.Tests/bin/Debug/net10.0-windows10.0.19041.0/WinApp.Cli.Tests --server dotnettestcli --dotnet-test-pipe /tmp/37e5f171e9414b228cb09a28c14f5b71` (http block) > - Triggering command: `/home/REDACTED/work/winappCli/winappCli/src/winapp-CLI/WinApp.Cli.Tests/bin/Debug/net10.0-windows10.0.19041.0/WinApp.Cli.Tests /home/REDACTED/work/winappCli/winappCli/src/winapp-CLI/WinApp.Cli.Tests/bin/Debug/net10.0-windows10.0.19041.0/WinApp.Cli.Tests --server dotnettestcli --dotnet-test-pipe /tmp/a9c118b0f7af4ef4ba6f988be7fce656` (http block) > - Triggering command: `/home/REDACTED/work/winappCli/winappCli/src/winapp-CLI/WinApp.Cli.Tests/bin/Debug/net10.0-windows10.0.19041.0/WinApp.Cli.Tests /home/REDACTED/work/winappCli/winappCli/src/winapp-CLI/WinApp.Cli.Tests/bin/Debug/net10.0-windows10.0.19041.0/WinApp.Cli.Tests --server dotnettestcli --dotnet-test-pipe /tmp/65ed7e97e0c043a2a6487a99a65f17ca` (http block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/microsoft/winappCli/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> Fix PR #407 CI failures in microsoft/winappCli (run 24152826595, job 70484215857). Failing tests indicate `winapp init` returns exit code 1 when NuGet version resolution fails for `Microsoft.Windows.SDK.BuildTools.WinApp` (aka `DotNetService.WINDOWS_SDK_BUILD_TOOLS_WINAPP_PACKAGE`). Logs show: `[ERROR] - Failed to get version for Microsoft.Windows.SDK.BuildTools.WinApp` and asserts expect init to succeed. Implement two behavior changes in the .NET init path: 1) Do not hard-fail init when NuGet version lookup fails for `Microsoft.Windows.SDK.BuildTools.WinApp`. It should be treated as non-fatal (warn and continue), while still keeping core build tools package required. 2) Do not mutate MSIX csproj properties (e.g., `EnableMsixTooling`, removal of `WindowsPackageType=None`) unless the WinApp integration package is actually present/added successfully. If the user declines the WinApp package, or if adding/updating it failed, do not apply MSIX property mutations. Target file: - `src/winapp-CLI/WinApp.Cli/Services/WorkspaceSetupService.cs` Update logic around the dynamic package list and the `Adding NuGet packages to project` subtask to: - Mark the WinApp integration package as non-required OR otherwise avoid returning failure solely due to inability to query its version. - Track whether the WinApp integration package was successfully added/updated, and gate MSIX property mutations on that boolean (not just user opt-in). Add/adjust tests as needed (likely in `src/winapp-CLI/WinApp.Cli.Tests/WorkspaceSetupServiceTests.cs`) to cover: - init succeeds (exit code 0) when NuGet service throws while fetching version for `Microsoft.Windows.SDK.BuildTools.WinApp`. - MSIX properties are not added when the user declines the WinApp integration package. Please open a PR with the changes. The following is the prior conversation context from the user's chat exploration (may be truncated): User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate. Assistant: The job is failing because `winapp init` returns exit code **1** during the .NET init flow when it tries to resolve NuGet versions, specifically: - **`[ERROR] - Failed to get version for Microsoft.Windows.SDK.BuildTools.WinApp`** - This bubbles up from `WorkspaceSetupService` where that package is treated as **required**; if version lookup fails, init fails. There’s also a separate test failure where MSIX tooling properties are being applied even when the WinApp integration package is declined. ## Fix 1 (primary): Don’t fail init when NuGet version lookup fails for the WinApp integration package In `WorkspaceSetupService.cs`, the .NET path builds a dynamic package list and marks the WinApp integration package as `Required` when `installWinAppPackage == true`: ```csharp if (installWinAppPackage) { packages.Add((DotNetService.WINDOWS_SDK_BUILD_TOOLS_WINAPP_PACKAGE, true)); } ``` Then, inside the “Adding NuGet packages to project” subtask, any failure to fetch the latest version for a *required* package returns `(1, ...)` and aborts init: ```csharp catch (Exception ex) { ... if (required) { return (1, $"Failed to get version for {packageName}"); } } ``` ### Recommended change Treat `Microsoft.Windows.SDK.BuildTools.WinApp` as **non-fatal** when version resolution fails, and fall back to: 1) an existing version if the project already references it, or 2) adding it without an explicit version (letting NuGet/MSBuild resolve), or 3) skipping it with a warning (but then you must not apply MSIX properties—see Fix 2). Concrete code suggestion (minimal behavioral change: keep trying, but don’t fail init just because NuGet version lookup failed for this package): **In** `src/winapp-CLI/WinApp.Cli/Services/WorkspaceSetupService.cs` (ref `66ee83b12c8470adc1c0fd34a13b1375e9552608`), change the “required” flag for the WinApp package to `false`: ```diff if (installWinAppPackage) { - packages.Add((DotNetService.WINDOWS_SDK_BUILD_TOOLS_WINAPP_PACKAGE, true)); + // Don't fail init if NuGet metadata is temporarily unavailable for this integration package. + packages.Add((DotNetService.WINDOWS_SDK_BUILD_TOOLS_WINAPP_PACKAGE, false)); } ``` And keep `BuildToolsService.BUILD_TOOLS_PACKAGE` as required. This directly addresses the failing tests/logs: init should succeed even if that package version can’t be fetched at that moment. ## Fix 2: Only apply MSIX properties if the WinApp integration package was actually added/updated One test is failing: - `SetupWorkspace_DotNet_DoesNotApplyMsixProperties_WhenWinAppNotOptedIn` - Assertion: `EnableMsixTooling` should **not** be added when user declines the WinApp package - But the csproj ends up containing `EnableMsixTooling` anyway. In `WorkspaceSetupService.cs`, MSIX properties are applied based on `installWinAppPackage`: ```cs... </details> <!-- START COPILOT CODING AGENT SUFFIX --> *This pull request was created from Copilot chat.* > --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: zateutsch <88554871+zateutsch@users.noreply.github.com>
…tifier ...> opening tags.
…timeIdentifier ...> opening tags." This reverts commit af9c604.
nmetulev
requested changes
Apr 8, 2026
Member
nmetulev
left a comment
nmetulev
left a comment
There was a problem hiding this comment.
After testing, this is what I've found:
- We don't actually need the buildtools package to be installed, we can skip that and remove the code adding the package or checking for it
- The true is not added if the winapp package is not available - it should be added regardless as long as the winappsdk package is in the csproj (whether we add it or not)
- The EnabledMsixTooling doesn't seem to work when the manifest is called appxmanifest.xml, but it works for Package.appxmanifest - we should name the generated manifest Package.appxmanifest to align with the conventions used in winui projects
- We should only add comments in the csproj for things we have changed - if we add the EnableMsixTooling propoerty, we should comment why it was added.
Co-authored-by: Nikola Metulev <nmetulev@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #395
Prevents --use-defaults flag from overwriting the existing dependency versions in a csproj.
fixes #399
Now prompts users to install nuget packages, unless --use-defaults is specified
fixes #401
If a user opts in to installing the nuget package, init will now make sure that the proper properties are added/removed in the csproj to enable running packaged.