Make MSIX install in Windows test cake script resilient

[akoeplinger]

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!

Make the MSIX install step in eng/devices/windows.cake more resilient:

• Add-AppxPackage's progress output floods cake's stdout pipe and makes the captured log unreadable in agent harnesses that don't render a console. Suppress it via $ProgressPreference='SilentlyContinue'.
• Check the install exit code: dependency failures now warn (previously silently ignored, since StartProcess returns the exit code rather than throwing), and app install failures now throw with the actual exit code (previously discarded entirely).

I noticed this while working on #29905 when Copilot got confused by the text-mode progress characters that got rendered when the output was redirected when Copilot ran the cake script.

[github-actions]

🚀 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 -- 35670

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35670"

[akoeplinger]

/azp run maui-pr-devicetests,maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[kubaflo]

/review -b feature/enhanced-reviewer -p windows

[MauiBot]

Expert Review — 1 findings

See inline comments for details.

[MauiBot]

[moderate] Build & MSBuild / Windows Platform - This embeds the MSIX path directly into a PowerShell -Command string using single quotes and -Path. A checkout/build path containing an apostrophe (for example C:\Users\O'Brien\...) will break the PowerShell string, and -Path still treats wildcard characters such as [/] specially. Since this helper is meant to make MSIX install robust, pass the path out-of-band (for example via an environment variable) and call Add-AppxPackage -LiteralPath $env:MAUI_MSIX_PATH, or at minimum escape single quotes and use -LiteralPath.

[MauiBot]

AI Review Summary

@akoeplinger — new AI review results are available based on this last commit: 31c136d.
Make MSIX install in Windows test cake script resilient To request a fresh review after new comments or commits, comment /review rerun.

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.

---

Pre-Flight — Context & Validation

Issue: #N/A - GitHub metadata unavailable (gh authentication missing)
PR: #35670 - Windows device test MSIX install robustness/log cleanup
Platforms Affected: Windows
Files Changed: 1 implementation, 0 test

Key Findings

• GitHub CLI authentication is unavailable in this environment, so issue body, PR body, comments, review comments, and CI details could not be fetched directly.
• Local squashed PR commit changes only eng/devices/windows.cake.
• The PR extracts MSIX installation into InstallAppxPackage, suppresses noisy Add-AppxPackage progress output, returns install exit codes, warns on dependency install failures, and throws when the primary app MSIX install fails.
• Gate result supplied by caller: SKIPPED - no tests detected in this PR. No gate verification was re-run.

Code Review Summary

Verdict: LGTM
Confidence: high
Errors: 0 | Warnings: 0 | Suggestions: 1

Key code review findings:

• Suggestion: eng/devices/windows.cake: PowerShell command embeds absPath in a single-quoted string; using -LiteralPath or argument-based invocation would be more robust for paths containing quotes, though this is not a practical regression for CI MSIX output paths.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35670	Local helper wraps Add-AppxPackage, suppresses progress, checks exit codes, warns for dependency failures, throws for app install failure	SKIPPED (Gate: no tests detected)	eng/devices/windows.cake	Original PR

---

Code Review — Deep Analysis

Code Review — PR #35670

Independent Assessment

What this changes:
The sole changed file is eng/devices/windows.cake. The PR:

1. Extracts a local C# function InstallAppxPackage(FilePath path) inside the testOnly Cake task, replacing two ad-hoc StartProcess("powershell", ...) call sites.
2. The new function invokes PowerShell with -NoProfile and suppresses progress output via $ProgressPreference='SilentlyContinue' before calling Add-AppxPackage.
3. Returns the StartProcess exit code to the caller rather than discarding it.
4. Dependency MSIX failures now log a Warning instead of being silently swallowed (the old try/catch was catching exceptions that StartProcess never throws, making it dead code).
5. App MSIX install failure now throws an exception with the exit code, rather than continuing silently.

Inferred motivation:
Two distinct problems motivate the change:

• Add-AppxPackage streams ANSI progress characters to stdout that corrupt log readability when output is captured by Cake or CI harnesses that don't render a console.
• StartProcess returns an int exit code; it does not throw. The pre-existing try/catch on the dependency loop was dead code, and the main MSIX install exit code was silently discarded — meaning install failures could go undetected until the test run itself fails with a confusing error.

---

Reconciliation with PR Narrative

Author claims:
(1) $ProgressPreference='SilentlyContinue' fixes log pollution in agent harnesses (specifically mentioned in context of PR #29905). (2) Exit-code checking properly surfaces dependency failures as warnings and app install failure as an exception.

Agreement/disagreement:
Code exactly matches the described intent. The root-cause analysis is accurate: StartProcess returns exit codes, not exceptions. The old catch for dependencies was indeed unreachable for that reason.

---

Findings

💡 Suggestion — Single-quote injection risk in PowerShell -Command string

eng/devices/windows.cake, inside InstallAppxPackage:

return StartProcess("powershell",
    "-NoProfile -Command \"$ProgressPreference='SilentlyContinue'; Add-AppxPackage -Path '" + absPath + "'; if (-not $?) { exit 1 }\"");

absPath is embedded directly inside a PowerShell single-quoted string. A path containing ' would break the command (single-quote strings cannot be escaped in PowerShell). The old code used double-quoted embedding (\") which has the same structural vulnerability but with " instead of '.

In practice Windows MSIX output paths never contain single quotes, so this is not a regression. But switching to single-quote wrapping does change the quoting semantics — the upside is that variable expansion in the path is suppressed, which is intentionally safer. Either quoting approach is fine given the path origin, but using [Management.Automation.WildcardPattern]::Escape() or -LiteralPath would be strictly correct.

Why this is a suggestion and not an error: The path is produced by Cake's MakeAbsolute(), which derives from the MSIX output directory. MSIX output paths on Windows CI agents don't contain single quotes in practice. This is the same class of issue as the original code.

---

Devil's Advocate

Challenging the approval:

• Is the $ProgressPreference scoping correct? Yes. The -Command argument runs in a fresh PowerShell process started by StartProcess. Setting the preference variable at the start of the command string scopes it correctly to that process.
• Could if (-not $?) { exit 1 } be bypassed? If Add-AppxPackage throws a terminating error, the if statement is skipped. However, in that case PowerShell's own exit code for an unhandled terminating error in -Command mode is also 1. So the exit-code signal is correct either way.
• Is the catch on dependencies still dead code? Almost. InstallAppxPackage calls StartProcess (no throw) and MakeAbsolute (could throw for null/invalid path). The catch defends against the latter. This is acceptable defensive programming.
• CI has a failing check (Build Analysis → PowerShell exited with code '1'): The failing test is MauiBlazorWebView.DeviceTests-unpackaged.WorkItemExecution / RequestsCanBeInterceptedAndCustomDataReturnedForDifferentHosts(uriBase: "https://echo.free.beeceptor.com/sample-request") — a network call to an external service that returned "Failed to fetch". This has a 12.18% historical failure rate and is an infrastructure flake, not caused by this PR's changes. The second test failure (RequestsCanBeInterceptedAndCustomDataReturnedForDifferentHosts) is the same flaky network test.

Challenging the one concern flagged:
The single-quote injection point is a pre-existing pattern (same vulnerability existed with double quotes). The PR author intentionally chose single quotes to prevent variable expansion in the path, which is the correct instinct. Not worth blocking on.

---

Verdict: LGTM

Confidence: high
Summary: The change is small, surgical, and correct. It fixes two real bugs (dead try/catch, discarded exit code) and one CI quality-of-life issue (progress output spam), with no functional regressions. The only flagged concern is a cosmetic injection-safety observation that applies equally to the pre-existing code. CI failures are pre-existing flakes unrelated to the PR.

---

Fix — Analysis & Comparison

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix	Pass MSIX path via child-process environment and use Add-AppxPackage -LiteralPath with PowerShell try/catch	BLOCKED (local quoting validation passed; no full regression test available)	1 file	Best alternative; safer path handling than PR fix and still readable
2	try-fix	Use UTF-16LE -EncodedCommand, escaped single-quoted path, and -LiteralPath	BLOCKED (local encoded-command validation passed; no full regression test available)	1 file	Robust but less debuggable due opaque Base64 command
3	try-fix	Invoke Add-AppxPackage in-process via System.Management.Automation typed parameters	BLOCKED (PowerShell SDK available in pwsh; Cake host + real install unverified)	1 file	Most distinct, but highest CI/build-host risk
PR	PR #35670	Local helper wraps Add-AppxPackage, suppresses progress, checks exit codes, warns for dependency failures, throws for app install failure	SKIPPED (Gate: no tests detected)	1 file	Original PR; code review LGTM

Cross-Pollination

Model	Round	New Ideas?	Details
maui-expert-reviewer	1	Yes	Generated env-var/-LiteralPath, -EncodedCommand, and in-process PowerShell SDK alternatives.
claude-opus-4.6	2	No	Covered safer subprocess, encoded script, and in-process typed invocation; additional variations would be minor quoting rewrites.
claude-opus-4.7	2	No	No meaningfully different root-cause approach beyond path-as-data, encoded script, or avoiding subprocess.
gpt-5.3-codex	2	No	No new candidate without adding broader test infrastructure or refactoring install orchestration.
gpt-5.5	2	No	Exhausted non-trivial alternatives for this one-file Cake install helper.

Exhausted: Yes - further candidates would be trivial variations of command quoting or error-message formatting.
Selected Fix: Candidate #1 if an alternative is required; otherwise PR's fix remains acceptable. Candidate #1 is safer for special-character paths and wildcard semantics, but it did not pass full regression tests because the gate found no tests and this environment lacks MSIX install artifacts.

Testing Notes

• Gate was not re-run and gate/content.md was not created or overwritten.
• Local validation only checked candidate command-construction mechanics for special characters. It does not prove real Add-AppxPackage success.
• Per try-fix rules, no candidate is marked Pass because no actual Windows packaged device-test run completed successfully.

---

Report — Final Recommendation

Comparative Report - PR #35670

Inputs

• Pre-flight summary: PR #35670 changes eng\devices\windows.cake for Windows packaged device-test MSIX install robustness and log cleanup.
• Gate result: SKIPPED - no tests detected in this PR. Gate verification was not rerun.
• Expert review result: raw PR needs one small robustness change in InstallAppxPackage.
• STEP 5a candidates reviewed: try-fix-1, try-fix-2, try-fix-3.

Candidate ranking

Rank	Candidate	Regression result	Assessment
1	pr-plus-reviewer	Not fully run / blocked by missing Windows MSIX regression artifact	Best candidate. Preserves the PR's behavior while applying the expert reviewer's path-handling fix: pass the MSIX path through an environment variable and use Add-AppxPackage -LiteralPath.
2	try-fix-1	BLOCKED	Technically equivalent to the reviewer-applied improvement and the best independent alternative. Ranked below pr-plus-reviewer only because pr-plus-reviewer is the PR fix with the same improvement integrated.
3	pr	SKIPPED	Directionally correct and improves install failure handling, but the new helper still interpolates paths into PowerShell command text and uses -Path, leaving avoidable quoting and wildcard edge cases.
4	try-fix-2	BLOCKED	Robust against command-line quoting via -EncodedCommand and -LiteralPath, but less debuggable and more opaque in CI logs than the environment-variable approach.
5	try-fix-3	BLOCKED	Avoids subprocess quoting by using System.Management.Automation, but has the highest Cake-host and CI assembly-resolution risk for this small helper change.

No candidate passed a full regression run. The ranking therefore prioritizes candidates that avoided known review findings and minimized implementation risk. If any candidate had failed regression tests, it would have been ranked below candidates that passed; no such pass/fail split exists in the available artifacts.

Winning candidate

Winner: pr-plus-reviewer

pr-plus-reviewer is the single best candidate because it keeps the raw PR's useful behavior changes while addressing the expert reviewer's only actionable concern. It is also lower risk than the broader alternatives because it stays with the existing powershell subprocess model and changes only how the path is passed to Add-AppxPackage.

Notes for PR author

The PR should add test coverage if feasible, because the supplied gate found no tests in the PR. At minimum, the install helper should be updated to pass the MSIX path as data and use -LiteralPath before merge.

---

Future Action — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.