Skip to content

winapp: add 'migrate' command family (scaffold / validate) for UWP -> WinUI 3 - #695

Open
shisan (qiutongMS) wants to merge 9 commits into
mainfrom
user/qiutongshen/winapp-migrate-analyze
Open

winapp: add 'migrate' command family (scaffold / validate) for UWP -> WinUI 3#695
shisan (qiutongMS) wants to merge 9 commits into
mainfrom
user/qiutongshen/winapp-migrate-analyze

Conversation

@qiutongMS

@qiutongMS shisan (qiutongMS) commented Jul 29, 2026

Copy link
Copy Markdown

What

Adds the winapp migrate command family for UWP -> WinUI 3 migration, split into two source-only (no restore/build) commands:

Command Role
migrate scaffold --from-uwp Copy UWP source (C#/XAML/assets) into an existing WinUI 3 scaffold and apply the mechanical, deterministic transforms every migration needs: merge SDK-sample shared/ + SharedContent/, preserve the original csproj/appxmanifest under .uwp-source/, patch the RuntimeIdentifier for x86/x64/ARM64 F5, rewrite Windows.UI.Xaml -> Microsoft.UI.Xaml, neutralize content-filter-prone helpers, exclude build/packaging/signing files, and wire the MainWindow RootFrame + initial navigation.
migrate validate --from-uwp Static gate before declaring a migration done: namespace/csproj text-marker residue (comments and string literals sanitized out first), single-project layout, MainWindow shell wiring, and manifest packaging requirements. Emits [PASS]/[FAIL]/[WARN] and returns non-zero on any [FAIL].

Why

Gives the UWP->WinUI 3 migration skill a deterministic CLI backbone: scaffold (mechanical transforms) and validate (completion gate) so the agent spends tokens on real API fixes, not on boilerplate it can't reliably reproduce.

Note on migrate analyze (removed)

An earlier revision of this PR also added migrate analyze, a passthrough to an out-of-tree Roslyn analyzer driver, plus an analyzer-backed API-residue check in validate. After design review these were removed so the CLI stays self-contained and does not couple to a tool that physically lives in the win-dev-skills repo. Pre-build API analysis (the JSON migration plan) now lives entirely in the winui-uwp-migration skill, which invokes the winui-analyze driver directly. validate keeps only self-contained gates (layout / shell / text-marker / manifest).

Changes

  • Commands: MigrateScaffoldCommand, MigrateValidateCommand (+ MigrateCommand parent), registered in HostBuilderExtensions.ConfigureCommands.
  • --quiet: scaffold + validate honor the shared --quiet option via a QuietFilteringTextWriter that suppresses [PASS]/progress chatter.
  • Generated docs: regenerated docs/cli-schema.json, docs/npm-usage.md, and winapp-commands.ts so migrate appears -- satisfies the validate-llm-docs.ps1 -FailOnDrift CI gate.
  • Tests: unit tests across both handlers (temp-project fixtures), following the existing fake-injection pattern.

Testing

  • dotnet test --filter ~Migrate -> 17/17 passed.
  • scripts/validate-llm-docs.ps1 -FailOnDrift -> exit 0 (no doc drift).

Notes

  • migrate subcommands are intentionally not mapped into the skill command map (a non-fatal generator warning), consistent with the existing unregister precedent. CI passes either way.

Qiutong Shen (from Dev Box) and others added 3 commits July 24, 2026 15:51
New MigrateCommand parent + MigrateAnalyzeCommand leaf. The analyze verb
runs source-only (no restore/build) by shelling out to the bundled analyzer
driver (winui-analyze), resolved via WINAPP_MIGRATE_ANALYZER or the CLI
tools/ folder, and passes the JSON migration plan through on stdout. Wired
into command registration, the root command, and the categorized help
(new Migration category). Analyzer stays out-of-process, as required by the
AOT CLI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Complete the UWP->WinUI 3 migration command set alongside the existing
'migrate analyze':

- scaffold: copy UWP source into a WinUI 3 project and apply the
  deterministic transforms (verbatim copy, sibling shared/ + SharedContent/
  merge, Windows.UI.Xaml -> Microsoft.UI.Xaml rewrite, csproj
  RuntimeIdentifier patch for x86/x64/ARM64 F5, .uwp-source preservation,
  content-filter-prone class neutralization, MainWindow RootFrame + deferred
  initial Navigate wiring). Ports the generation half of the skill's
  Initialize-UwpMigration.ps1; triage/findings stay with analyze.
- validate: source-only static gate (analyzer-backed API residue, text-marker
  residue, single-project layout, MainWindow shell wiring, manifest packaging)
  with sanitized stdout + .validator-diagnostics.txt and non-zero exit on FAIL.
- Shared MigrateAnalyzerDriver service + AOT-safe JSON models
  (MigrateAnalyzeReport / MigrateJsonContext) consumed by analyze and validate.
- Refactor analyze to use the shared driver service; register scaffold/validate
  in MigrateCommand + HostBuilderExtensions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Regenerate the LLM/npm documentation artifacts so the migrate command
family (scaffold / analyze / validate) is reflected, and add unit tests
covering the three command handlers.

- docs/cli-schema.json, docs/npm-usage.md, winapp-commands.ts: regenerated
  from the CLI so 'migrate' subcommands appear (satisfies the CI
  validate-llm-docs -FailOnDrift gate).
- Add MigrateValidate/Analyze/Scaffold command tests + FakeMigrateAnalyzerDriver,
  mirroring the existing BaseCommandTests fake-injection pattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Copilot AI review requested due to automatic review settings July 29, 2026 05:21
Comment thread src/winapp-CLI/WinApp.Cli.Tests/MigrateCommandTestBase.cs Fixed
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs Fixed
Comment thread src/winapp-CLI/WinApp.Cli/Services/MigrateAnalyzerDriver.cs Fixed
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateAnalyzeCommand.cs Fixed
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Fixed
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Fixed

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

Adds a UWP-to-WinUI 3 migration command family with scaffolding, source analysis, validation, npm wrappers, documentation, and tests.

Changes:

  • Adds migrate scaffold, analyze, and validate.
  • Introduces an out-of-process analyzer contract and validation models.
  • Regenerates CLI/npm documentation and adds 10 tests.

Reviewed changes

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

Show a summary per file
File Description
src/winapp-npm/src/winapp-commands.ts Adds npm migration wrappers.
src/winapp-CLI/WinApp.Cli/Services/MigrateAnalyzerDriver.cs Launches the external analyzer.
src/winapp-CLI/WinApp.Cli/Services/IMigrateAnalyzerDriver.cs Defines analyzer abstraction.
src/winapp-CLI/WinApp.Cli/Models/MigrateAnalyzeReport.cs Models analyzer JSON.
src/winapp-CLI/WinApp.Cli/Helpers/MigrateJsonContext.cs Adds AOT-safe JSON context.
src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs Registers migration services and commands.
src/winapp-CLI/WinApp.Cli/Commands/WinAppRootCommand.cs Adds the migration command group.
src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Implements static validation gates.
src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs Implements source migration transforms.
src/winapp-CLI/WinApp.Cli/Commands/MigrateCommand.cs Defines the parent command.
src/winapp-CLI/WinApp.Cli/Commands/MigrateAnalyzeCommand.cs Exposes analyzer output.
src/winapp-CLI/WinApp.Cli.Tests/MigrateValidateCommandTests.cs Tests validation behavior.
src/winapp-CLI/WinApp.Cli.Tests/MigrateScaffoldCommandTests.cs Tests copying and rewriting.
src/winapp-CLI/WinApp.Cli.Tests/MigrateCommandTestBase.cs Provides migration test infrastructure.
src/winapp-CLI/WinApp.Cli.Tests/MigrateAnalyzeCommandTests.cs Tests analyzer handling.
src/winapp-CLI/WinApp.Cli.Tests/FakeMigrateAnalyzerDriver.cs Adds a fake analyzer driver.
docs/npm-usage.md Documents npm migration APIs.
docs/cli-schema.json Adds migration CLI schema.
Comments suppressed due to low confidence (8)

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:303

  • The fallback accepts any same-extension file whose name merely starts with the missing logo basename. For example, a missing Assets/Logo.png is considered resolvable when only Assets/LogoOld.png exists, so the manifest gate can pass a package with a missing asset. Match only valid resource qualifiers such as Logo.scale-*.png/Logo.targetsize-*.png.
                var dir = Path.GetDirectoryName(abs);
                var baseName = Path.GetFileNameWithoutExtension(abs);
                var ext = Path.GetExtension(abs);
                if (dir is not null && Directory.Exists(dir)
                    && Directory.EnumerateFiles(dir, $"{baseName}*{ext}").Any())
                {
                    continue; // scale-*/targetsize-* variant present

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:124

  • Returning zero here makes a missing analyzer a successful validation, and the handler later prints that all checks passed even though its analyzer-backed API-residue gate never ran. Treat this as a failed check; this is especially important for packaged installs where the driver currently is not bundled.
            if (!run.DriverFound)
            {
                Console.Out.WriteLine("[WARN] Residue (API) — analyzer driver 'winui-analyze' not found; skipped API-residue check. Text-marker residue still enforced.");
                return 0;

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:80

  • The command does not reject overlapping roots. With identical source and target, the first copy calls File.Copy on the same path and throws; with a target below source, recursive enumeration can consume files it just created, while a source below target is later rewritten in place. Validate that the two directory trees are disjoint before copying.
            var sourceRoot = source.FullName.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
            var targetRoot = target.FullName.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:330

  • These regular expressions treat XML serialization details as semantics. Valid manifests can reorder attributes, use single quotes, choose a different prefix for the restricted-capabilities namespace, or use non-self-closing elements; such manifests are incorrectly failed. Parse with the repository's AppxManifestDocument (Services/AppxManifestDocument.cs:45) and inspect namespaced elements/attributes instead.
            if (!TargetDeviceDesktop().IsMatch(text))
            {
                pkgFails.Add("<TargetDeviceFamily> is not Windows.Desktop (Windows.Universal is UWP-only; the registrar rejects it for a Win32 entrypoint).");
            }
            var hasRescapNs = RescapNamespace().IsMatch(text);
            var rescapIgnorable = RescapIgnorable().IsMatch(text);
            if (!hasRescapNs || !rescapIgnorable)
            {
                pkgFails.Add("missing the rescap namespace declaration (add xmlns:rescap=\"…/restrictedcapabilities\" on <Package> and append 'rescap' to IgnorableNamespaces).");

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:98

  • Every source collision is silently copied with overwrite: true. A normal UWP source includes App.xaml/App.xaml.cs, so this can destroy the existing WinUI scaffold's launch setup (and any user edits) before the shell wiring step, with no backup or warning. Detect collisions and preserve/fail on scaffold shell files instead of unconditionally overwriting them.
                var rel = Path.GetRelativePath(sourceRoot, file);
                CopyInto(file, Path.Combine(targetRoot, rel));
                copied.Add(rel);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:141

  • Malformed or empty analyzer output is treated as zero failures, so validation reports success without evaluating API residue. This is an analyzer failure, not a passing gate, and should contribute a failure result.
            if (report is null)
            {
                Console.Out.WriteLine("[WARN] Residue (API) — analyzer produced no parseable output; skipped API-residue check.");
                if (run.StdErr.Length > 0) { diag.AppendLine("[Residue API driver stderr]").AppendLine(run.StdErr.TrimEnd()).AppendLine(); }
                return 0;

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:130

  • The validator never checks run.ExitCode. A driver can report an analysis failure with a nonzero exit code while leaving parseable (possibly partial) JSON on stdout; if that JSON has no must-fix findings, this code declares the residue gate passed. Reject nonzero runs before trusting the report and preserve stderr in diagnostics.
            MigrateAnalyzeReport? report = null;
            try
            {
                report = JsonSerializer.Deserialize(run.StdOut, MigrateJsonContext.Default.MigrateAnalyzeReport);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:296

  • A manifest image path can be rooted or contain ..; Path.Combine then resolves outside the project, and an unrelated external file makes the asset check pass even though it cannot be packaged from the project. Canonicalize each path and reject it unless it remains beneath root before testing existence.
            foreach (var reff in imageRefs)
            {
                var abs = Path.Combine(root, reff.Replace('/', Path.DirectorySeparatorChar));
                if (File.Exists(abs)) { continue; }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/winapp-CLI/WinApp.Cli/Services/MigrateAnalyzerDriver.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread docs/npm-usage.md
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Validate (fail closed instead of falsely passing):
- CheckSingleProject: zero .csproj now FAILs (empty dir was reported PASS).
- CheckShellWiring: missing MainWindow.xaml now FAILs (blank-shell project passed).
- CheckAnalyzerResidueAsync: analyzer launch exception / unparseable output now
  FAIL (were WARN+pass); driver-not-found stays WARN pending driver packaging.
  Rethrow OperationCanceledException.

Scaffold:
- WireRootFrame: only inject 'RootFrame.Navigate(...)' when a RootFrame is
  actually present/inserted, else skip (avoids referencing a missing frame).
- Replace broken MIGRATION-PATTERNS.md reference with inline guidance.

Analyze:
- Suppress the first-run notice for 'migrate analyze' so its JSON stdout is
  parseable on a fresh install (it has no --json flag).

npm generator:
- generate-commands.mjs now honors required named options; regenerated
  winapp-commands.ts + npm-usage.md so 'scaffold --target' is typed required.

Tests: add empty-dir / missing-shell / driver-throws validate cases; mark
migrate test classes [DoNotParallelize] and dispose the capture StringWriter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Copilot AI review requested due to automatic review settings July 29, 2026 06:07
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Fixed
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Fixed

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 20 out of 20 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (12)

src/winapp-CLI/WinApp.Cli/Services/MigrateAnalyzerDriver.cs:27

  • No analyzer executable or analyzer project is added, and the CLI project/build only publishes winapp.exe; therefore neither candidate path can exist in shipped npm/NuGet/MSIX artifacts. migrate analyze will always return “not found” (and validate will skip its API gate) unless users supply an undocumented external binary via the environment override. The driver must be built and copied into each package's tools directory as part of this PR.
        var exeName = OperatingSystem.IsWindows() ? "winui-analyze.exe" : "winui-analyze";
        string[] candidates =
        [
            Path.Combine(AppContext.BaseDirectory, "tools", exeName),
            Path.Combine(AppContext.BaseDirectory, exeName),
        ];

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:96

  • Diagnostics are written only when this run produced text. After a failed validation creates .validator-diagnostics.txt, a later clean run leaves those stale failures on disk while reporting success. Always truncate/write the diagnostics file (including an empty result), or delete it on success.
            var diagPath = Path.Combine(root, ".validator-diagnostics.txt");
            if (diagnostics.Length > 0)
            {
                try { await File.WriteAllTextAsync(diagPath, diagnostics.ToString(), cancellationToken); } catch { /* best effort */ }
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:32

  • The advertised “assets” copy is restricted to images and RESW/RESJSON, so common UWP content such as fonts, media, JSON/XML data, shaders, and text files is silently omitted while the command reports completion. Copy non-build project content with an exclusion policy, or explicitly handle the full supported asset set and report skipped files.
    // Source files worth copying (source + assets). Build artifacts and projects are excluded.
    private static readonly string[] CopyExtensions =
    [
        ".xaml", ".cs", ".resw", ".resjson",
        ".png", ".jpg", ".jpeg", ".svg", ".ico", ".gif"
    ];

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:484

  • RootFrame insertion only supports the exact text <Grid Grid.Row="1" />. A valid existing WinUI scaffold using a root Grid, StackPanel, expanded Grid, different attribute order, or single quotes reaches the warning path, skips initial navigation, and still reports successful completion. Parse the XAML structurally and either wire supported layouts or fail the scaffold command when its promised shell transform cannot be applied.
                if (!body.Contains("x:Name=\"RootFrame\"") && !body.Contains(FrameMarker))
                {
                    if (EmptyGridRow1().IsMatch(body))
                    {
                        body = EmptyGridRow1().Replace(body,
                            $"{FrameMarker}\r\n        <Frame x:Name=\"RootFrame\" Grid.Row=\"1\" />", 1);
                        File.WriteAllText(mainWindowXaml, body);
                        Console.Out.WriteLine("    Replaced empty Grid with <Frame x:Name=\"RootFrame\"> in MainWindow.xaml");

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:84

  • This command exposes the standard --quiet option (“Suppress progress messages”), but all scaffold progress is written directly to Console.Out, bypassing the log-level suppression. Consequently migrate scaffold --quiet remains fully verbose, including through the generated npm wrapper. Route progress through the normal logging/console abstraction or condition it on the parsed quiet option.
            Console.Out.WriteLine("==> winapp migrate scaffold --from-uwp");
            Console.Out.WriteLine($"    Source : {sourceRoot}");
            Console.Out.WriteLine($"    Target : {targetRoot}");

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:219

  • The .Take(30) also encloses the diagnostic-file append, so marker failures in the 31st and later files are absent from the file advertised as containing full diagnostics. Limit only console output; append every group to diag.
            foreach (var g in hits.GroupBy(h => h.File).Take(30))
            {
                foreach (var h in g.Take(10)) { Console.Out.WriteLine($"       {g.Key}:{h.Line}"); }
                foreach (var h in g) { diag.AppendLine($"  {h.File}:{h.Line}  {h.Name}  | {h.Snippet}"); }
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:350

  • These packaging checks parse XML with formatting-sensitive regexes, so valid manifests can fail the gate—for example, XML permits Name after another TargetDeviceFamily attribute, single-quoted attributes, and an explicit <rescap:Capability ...></rescap:Capability> closing tag, none of which match the generated regexes. This repository already provides the namespace-aware AppxManifestDocument; use it/XDocument for target family, image, namespace, and capability checks.
            // 5b. WinUI 3 packaging requirements
            var pkgFails = new List<string>();
            if (!TargetDeviceDesktop().IsMatch(text))
            {
                pkgFails.Add("<TargetDeviceFamily> is not Windows.Desktop (Windows.Universal is UWP-only; the registrar rejects it for a Win32 entrypoint).");
            }
            var hasRescapNs = RescapNamespace().IsMatch(text);
            var rescapIgnorable = RescapIgnorable().IsMatch(text);
            if (!hasRescapNs || !rescapIgnorable)
            {
                pkgFails.Add("missing the rescap namespace declaration (add xmlns:rescap=\"…/restrictedcapabilities\" on <Package> and append 'rescap' to IgnorableNamespaces).");
            }
            if (!RunFullTrust().IsMatch(text))
            {

src/winapp-CLI/WinApp.Cli/Commands/WinAppRootCommand.cs:90

  • Registering a new public command without updating the hand-written user/agent documentation leaves docs/usage.md and the skill templates with no discoverable migration workflow; only generated schema/npm reference was added. Repository guidance requires public command workflows to update those sources (generated .github/plugin/skills must not be edited directly). Add a migrate usage section and a hand-written migration skill/template, then regenerate.
        Subcommands.Add(migrateCommand);

src/winapp-CLI/WinApp.Cli/Commands/MigrateAnalyzeCommand.cs:36

  • Unlike the command's directory argument and established file options such as ManifestOption/ExecutableOption, --project has no AcceptExistingOnly() validator. A typo or directory is therefore accepted as a “specific .csproj” and forwarded to the analyzer instead of producing a parser-level input error.
        ProjectOption = new Option<FileInfo?>("--project")
        {
            Description = "Target a specific .csproj (default: scan the whole directory)."
        };

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:40

  • Unlike the command's directory argument and established file options such as ManifestOption/ExecutableOption, --project has no AcceptExistingOnly() validator. A typo or directory is therefore accepted as a “specific .csproj” and forwarded to validation instead of producing a parser-level input error.
        ProjectOption = new Option<FileInfo?>("--project")
        {
            Description = "Target a specific .csproj (default: scan the whole directory)."
        };

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:180

  • The .Take(30) also encloses the diagnostic-file append, so findings in the 31st and later files are absent from the file advertised as containing full diagnostics. Limit only console output; append every group to diag.

This issue also appears on line 215 of the same file.

            foreach (var g in hits.GroupBy(h => h.File).Take(30))
            {
                foreach (var h in g.Take(10)) { Console.Out.WriteLine($"       {g.Key}:{h.Line}"); }
                if (g.Count() > 10) { Console.Out.WriteLine($"       {g.Key}: ({g.Count() - 10} more)"); }
                foreach (var h in g) { diag.AppendLine($"  {h.File}:{h.Line}  [{h.Id} {h.Severity}]  {h.Detected}"); }
            }

src/winapp-CLI/WinApp.Cli/Program.cs:76

  • Filtering only dash-prefixed tokens does not remove option values. With the recursive caller option in its normal two-token form—for example winapp --caller npm migrate analyze—the positional array starts with npm, this check is false, and the first-run banner/logs corrupt the promised JSON stdout. Determine the selected command from the parsed command result (or otherwise skip known option values) rather than heuristically filtering argv.
        var positionalArgs = args.Where(a => !a.StartsWith('-')).ToArray();
        bool isMigrateAnalyzeMode = positionalArgs.Length >= 2
            && positionalArgs[0] == "migrate" && positionalArgs[1] == "analyze";

Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs Outdated
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
…d paths

Round-2 review fixes for PR #695:

- scaffold: reject overlapping/nested source+target, validate UWP source + WinUI target prerequisites before mutating files, and never overwrite the WinUI scaffold's App.xaml/App.xaml.cs startup with the UWP originals (which launch via Window.Current).

- validate: fail closed when the analyzer exits nonzero (untrustworthy report) and delete stale .validator-diagnostics.txt on a clean pass so [PASS] is not contradicted by leftover failures.

- tests: rewrite scaffold fixtures for the new prerequisites; add coverage for startup-file preservation, path overlap, missing prerequisites, nonzero-exit fail-closed, and stale-diagnostics cleanup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Copilot AI review requested due to automatic review settings July 29, 2026 06:39
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs

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 20 out of 20 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (18)

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:137

  • Treating a missing analyzer as a warning makes the validation gate pass without checking the unsupported/startup-crash APIs it claims to gate. A damaged or incomplete installation can therefore print [PASS] Validation gate for a project with must-fix residue; fail closed here just as the exception, nonzero-exit, and invalid-JSON paths do.
            if (!run.DriverFound)
            {
                Console.Out.WriteLine("[WARN] Residue (API) — analyzer driver 'winui-analyze' not found; skipped API-residue check. Text-marker residue still enforced.");
                return 0;
            }

src/winapp-CLI/WinApp.Cli/Services/MigrateAnalyzerDriver.cs:27

  • No winui-analyze executable/project or publish item exists in the repository, so a normal CLI/npm publish cannot populate either of these candidate paths. Consequently migrate analyze always returns “driver not found” unless users manually set the undocumented deployment override, and the advertised analyzer-backed feature is unusable out of the box. Add the driver build and copy it into each published CLI architecture's tools directory.
        string[] candidates =
        [
            Path.Combine(AppContext.BaseDirectory, "tools", exeName),
            Path.Combine(AppContext.BaseDirectory, exeName),
        ];

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:264

  • A missing MainWindow.xaml.cs, or a code-behind file with no initial RootFrame.Navigate, currently adds no failure, so a project containing only a named Frame is reported as having valid shell wiring even though it can remain blank. Require the code-behind and verify the initial navigation that migrate scaffold promises to wire before emitting this PASS.
            if (mainWindowCs is not null)
            {
                var cs = SafeRead(mainWindowCs);
                if (DestructiveContent().IsMatch(cs))
                {
                    fails.Add("MainWindow.xaml.cs assigns Content directly — this overwrites XAML-defined layout and causes a blank window.");
                }
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:105

  • The target check also uses OR, so any directory with an arbitrary .csproj is accepted as a WinUI scaffold even when MainWindow.xaml is absent. The command then copies and rewrites files and still exits successfully; require both scaffold indicators before starting destructive work.
            var targetHasScaffold = FindFile(targetRoot, "MainWindow.xaml") is not null
                || EnumerateFiles(targetRoot).Any(f => f.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase));

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:97

  • The prerequisite accepts any directory containing either an arbitrary .csproj or a manifest as a UWP source. Thus a normal non-UWP .NET project passes and is copied/mutated despite the command claiming to require a UWP project. Require both the UWP project file and Package.appxmanifest (and ideally validate the project shape) before mutation.
            var sourceHasProject = EnumerateFiles(sourceRoot).Any(f =>
                f.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)
                || string.Equals(Path.GetFileName(f), "Package.appxmanifest", StringComparison.OrdinalIgnoreCase));

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:84

  • The shared --quiet option promises to suppress progress messages, but this handler writes all progress directly to Console.Out, bypassing the log-level suppression in Program. migrateScaffold({ quiet: true, ... }) therefore still captures the full banner and progress stream. Route progress through the configured logger/console abstraction or condition these writes on the parsed quiet option.
            Console.Out.WriteLine("==> winapp migrate scaffold --from-uwp");
            Console.Out.WriteLine($"    Source : {sourceRoot}");
            Console.Out.WriteLine($"    Target : {targetRoot}");

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:32

  • CopyExtensions is described as source plus assets but only includes XAML/C#, resources, and image formats. Common UWP content such as JSON/XML data, fonts, audio, video, and shader files is silently omitted, leaving migrated resource URIs broken while the command reports success. Copy project content/assets generically (excluding known build/project artifacts) rather than using this narrow allowlist.
    // Source files worth copying (source + assets). Build artifacts and projects are excluded.
    private static readonly string[] CopyExtensions =
    [
        ".xaml", ".cs", ".resw", ".resjson",
        ".png", ".jpg", ".jpeg", ".svg", ".ico", ".gif"
    ];

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:394

  • The RuntimeIdentifier transform only matches one exact textual serialization of the csproj element. Equivalent XML with reordered attributes, single quotes, additional whitespace, or a different line layout is silently skipped, yet scaffold still exits successfully claiming to apply the F5 fix. Parse and update the project with XDocument instead of regex/string splicing.
                var body = File.ReadAllText(cp);
                if (body.Contains(RidFixMarker)) { already++; continue; }
                var m = HostArchRuntimeIdentifier().Match(body);
                if (!m.Success)
                {
                    continue;
                }

                var indent = m.Groups["indent"].Value;
                var injection =
                    $"{indent}{RidFixMarker}\r\n" +
                    $"{indent}<RuntimeIdentifier Condition=\"'$(RuntimeIdentifier)' == '' AND '$(Platform)' == 'x86'\">win-x86</RuntimeIdentifier>\r\n" +
                    $"{indent}<RuntimeIdentifier Condition=\"'$(RuntimeIdentifier)' == '' AND '$(Platform)' == 'x64'\">win-x64</RuntimeIdentifier>\r\n" +
                    $"{indent}<RuntimeIdentifier Condition=\"'$(RuntimeIdentifier)' == '' AND '$(Platform)' == 'ARM64'\">win-arm64</RuntimeIdentifier>\r\n" +
                    m.Value;
                body = body[..m.Index] + injection + body[(m.Index + m.Length)..];
                File.WriteAllText(cp, body);

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:524

  • RootFrame detection and insertion are formatting-sensitive string/regex operations on XAML. For example, a valid existing x:Name='RootFrame' is not recognized; if an exact empty grid is also present, this can inject a second RootFrame name and make XAML compilation fail. Parse the XAML and inspect/add elements and expanded attributes structurally.
                var body = File.ReadAllText(mainWindowXaml);
                if (!body.Contains("x:Name=\"RootFrame\"") && !body.Contains(FrameMarker))
                {
                    if (EmptyGridRow1().IsMatch(body))
                    {
                        body = EmptyGridRow1().Replace(body,
                            $"{FrameMarker}\r\n        <Frame x:Name=\"RootFrame\" Grid.Row=\"1\" />", 1);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:216

  • The residue regexes run against raw lines, so comments and string literals containing examples such as using Windows.UI.Xaml or an old PackageReference are counted as must-fix residue. This produces a failing validation gate even though no UWP API/project setting remains active. Parse the relevant C#/XAML/project syntax, or at minimum exclude comments, rather than scanning raw text.
                foreach (var (rx, name) in ResidueMarkers)
                {
                    for (int i = 0; i < lines.Length; i++)
                    {
                        if (rx.IsMatch(lines[i]))
                        {
                            hits.Add((rel, i + 1, name, lines[i].Trim()));
                            break; // one hit per marker per file is enough

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:129

  • The command promises sanitized status lines on stdout, but this line interpolates the raw exception message, which commonly contains full executable/project paths or other driver details. Keep stdout generic and leave the raw message only in .validator-diagnostics.txt, as the other analyzer-error paths do.
                Console.Out.WriteLine($"[FAIL] Residue (API) — analyzer driver failed to run: {ex.Message}. Cannot verify API residue.");
                diag.AppendLine("[Residue — API]").AppendLine($"  analyzer driver failed to run: {ex.Message}").AppendLine();

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:96

  • Diagnostics writes are silently ignored, but the final failure line unconditionally tells users that full diagnostics were written. On a read-only project root or I/O failure, the command returns a failure with no actionable details and points to a nonexistent/stale file. Track write success and report the write error or emit diagnostics to stderr instead of claiming the file exists.
            if (diagnostics.Length > 0)
            {
                try { await File.WriteAllTextAsync(diagPath, diagnostics.ToString(), cancellationToken); } catch { /* best effort */ }
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:364

  • These packaging checks parse structured manifest XML with formatting-sensitive regexes. Valid XML using single quotes, a different restricted-capability prefix, reordered attributes, or an explicit <rescap:Capability>...</rescap:Capability> form will be falsely rejected. Parse with the repository's AppxManifestDocument/XDocument and inspect expanded names and attributes instead (Services/AppxManifestDocument.cs:10-21).
            if (!TargetDeviceDesktop().IsMatch(text))
            {
                pkgFails.Add("<TargetDeviceFamily> is not Windows.Desktop (Windows.Universal is UWP-only; the registrar rejects it for a Win32 entrypoint).");
            }
            var hasRescapNs = RescapNamespace().IsMatch(text);
            var rescapIgnorable = RescapIgnorable().IsMatch(text);
            if (!hasRescapNs || !rescapIgnorable)
            {
                pkgFails.Add("missing the rescap namespace declaration (add xmlns:rescap=\"…/restrictedcapabilities\" on <Package> and append 'rescap' to IgnorableNamespaces).");
            }
            if (!RunFullTrust().IsMatch(text))

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:89

  • --project scopes only the analyzer call; every subsequent gate still scans target.FullName. In a solution directory, selecting one project can still fail on another project's residue/duplicate csproj, inspect an unrelated MainWindow.xaml, and only check a root manifest, contrary to “Target a specific .csproj.” Derive the validation root/scope from the selected project and apply it consistently to all gates.

This issue also appears in the following locations of the same file:

  • line 128
  • line 133
  • line 257
            failures += await CheckAnalyzerResidueAsync(target, project, deferred, diagnostics, cancellationToken);

            // ─── 1b. Residue: text markers (UWP namespaces / csproj shape) ───
            failures += CheckTextResidue(root, deferred, diagnostics);

            // ─── 2. Shell wiring integrity ───
            failures += CheckShellWiring(root, diagnostics);

            // ─── 3. Single-project layout ───
            failures += CheckSingleProject(root, diagnostics);

            // ─── 4. Package.appxmanifest packaging requirements ───
            failures += CheckManifest(root, diagnostics);

src/winapp-CLI/WinApp.Cli/Commands/WinAppRootCommand.cs:90

  • Registering this public command without updating the hand-written command documentation leaves docs/usage.md, .github/plugin/agents/winapp.agent.md, and the source templates under docs/fragments/skills/winapp-cli/ unaware of the migration workflow. These files are the repository's documented public/agent surfaces and are not regenerated from cli-schema.json; add the migrate workflow there before regenerating plugin skills.
        Subcommands.Add(migrateCommand);

src/winapp-CLI/WinApp.Cli/Commands/MigrateAnalyzeCommand.cs:63

  • This catch also consumes OperationCanceledException from RunAsync, converting Ctrl+C/cancellation into a logged analyzer failure instead of allowing command cancellation to propagate. Match the validate handler's explicit cancellation catch before handling launch failures.
            catch (Exception ex)
            {
                logger.LogError("Failed to launch analyzer driver: {Message}", ex.Message);
                return 1;
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:324

  • App.xaml is structured XML, but these regex replacements only recognize exact element spelling/shape and silently return when attributes, namespace prefixes, or otherwise valid formatting differ. That leaves the copied Styles.xaml unregistered and its resources unavailable at runtime. Use XDocument to locate/create Application.Resources, ResourceDictionary, and MergedDictionaries as required by the repository XML convention.

This issue also appears on line 378 of the same file.

            if (MergedDictionariesOpen().IsMatch(body))
            {
                updated = MergedDictionariesOpen().Replace(body,
                    m => m.Value + "\r\n                <ResourceDictionary Source=\"Styles.xaml\"/>", 1);
            }
            else if (AppResourcesOpen().IsMatch(body))
            {
                updated = AppResourcesOpen().Replace(body,
                    m => m.Value + "\r\n            <ResourceDictionary.MergedDictionaries>\r\n                <ResourceDictionary Source=\"Styles.xaml\"/>\r\n            </ResourceDictionary.MergedDictionaries>", 1);
            }
            else
            {
                return;

src/winapp-CLI/WinApp.Cli/Program.cs:76

  • Filtering out option tokens does not remove their values. A valid invocation such as winapp --caller nodejs-package migrate analyze yields positional args nodejs-package, migrate, analyze, so first-run output is not suppressed and corrupts the promised JSON stdout. Determine the selected command from ParseResult (before displaying the notice) rather than heuristically scanning raw arguments.
        var positionalArgs = args.Where(a => !a.StartsWith('-')).ToArray();
        bool isMigrateAnalyzeMode = positionalArgs.Length >= 2
            && positionalArgs[0] == "migrate" && positionalArgs[1] == "analyze";

Qiutong Shen (from Dev Box) and others added 2 commits July 30, 2026 14:17
…marker sanitizer

Addresses PR review feedback [2][3][4]:
- [2] scaffold: switch asset copy to a DoNotCopyExtensions denylist (was an
  allowlist that dropped legitimate assets); add ShouldCopy / IsBuildOrProjectFile.
- [3] add a --quiet flag to scaffold + validate via a QuietFilteringTextWriter
  wrapper that suppresses [PASS]/informational chatter.
- [4] validate: add a text-marker residue sanitizer (SanitizeForMarkers /
  SanitizeCSharpLine / SanitizeXmlLine) that blanks comments and string literals
  before scanning, eliminating false positives from markers in comments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
'migrate analyze' was a pure passthrough to an external Roslyn analyzer driver
that physically lives in the win-dev-skills repo, and validate's API-residue
check shelled out to the same driver. This coupled the CLI to an out-of-tree
tool. Remove both so the CLI ships only self-contained gates:

- Delete MigrateAnalyzeCommand, MigrateAnalyzeReport, MigrateJsonContext,
  IMigrateAnalyzerDriver, MigrateAnalyzerDriver and their tests/fakes.
- Drop analyze from MigrateCommand, DI + command-handler wiring
  (HostBuilderExtensions), and the Program.cs schema mode.
- validate: remove ProjectOption + CheckAnalyzerResidueAsync; keep the
  self-contained layout/shell/text-marker gates.
- Regenerate docs/cli-schema.json, npm winapp-commands.ts, docs/npm-usage.md.

Analysis moves entirely to the winui-uwp-migration skill; the SKILL.md rewrite
to stop invoking 'winapp migrate analyze' is a separate coordinated change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Copilot AI review requested due to automatic review settings July 30, 2026 06:49
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs

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 13 out of 13 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (11)

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:120

  • This prerequisite uses OR, so a folder containing only an arbitrary .csproj (or only a manifest) is accepted as UWP source. The handler then performs destructive UWP-specific rewrites and reports success even though the documented source contract requires both project metadata and Package.appxmanifest.
            var sourceHasProject = EnumerateFiles(sourceRoot).Any(f =>
                f.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)
                || string.Equals(Path.GetFileName(f), "Package.appxmanifest", StringComparison.OrdinalIgnoreCase));

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:128

  • The target check has the same OR problem: any directory with any .csproj is treated as a WinUI 3 scaffold even when MainWindow.xaml is absent (and a directory with only MainWindow is also accepted). The command then copies/rewrites files, skips shell wiring, and still exits successfully instead of rejecting the unsupported target before mutation.
            var targetHasScaffold = FindFile(targetRoot, "MainWindow.xaml") is not null
                || EnumerateFiles(targetRoot).Any(f => f.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase));

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:368

  • String state is reset for every line, so valid multiline verbatim/raw strings are scanned as code after their opening line. For example, a C# constant containing @"...\nusing Windows.UI.Xaml\n..." produces a residue failure even though the marker is only data; the sanitizer needs to carry string kind/state across lines (including raw-string delimiters).
        private static string SanitizeCSharpLine(string line, ref bool inBlockComment)
        {
            var sb = new StringBuilder(line.Length);
            bool inLineComment = false, inString = false, inChar = false, inVerbatim = false;
            for (int j = 0; j < line.Length; j++)

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:547

  • Root-frame wiring only occurs when the target contains the exact text <Grid Grid.Row="1" />. The earlier scaffold check accepts any project/MainWindow layout, so common valid forms such as <Grid />, a non-self-closing Grid, or a StackPanel are accepted; this branch then only warns and the command still returns success without the advertised RootFrame/navigation wiring.
                if (!body.Contains("x:Name=\"RootFrame\"") && !body.Contains(FrameMarker))
                {
                    if (EmptyGridRow1().IsMatch(body))
                    {
                        body = EmptyGridRow1().Replace(body,
                            $"{FrameMarker}\r\n        <Frame x:Name=\"RootFrame\" Grid.Row=\"1\" />", 1);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:425

  • This pattern only detects a using Windows.UI.Xaml... directive, so fully qualified residue such as Windows.UI.Xaml.Controls.Button is missed. I verified that a project containing that reference is reported as [PASS] Residue and exits 0, even though the migrated source still depends on the UWP XAML API.
    [GeneratedRegex(@"using\s+Windows\.UI\.Xaml")] private static partial Regex UsingWindowsUiXaml();

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:181

  • The shell gate never verifies that RootFrame is navigated or otherwise populated; it only rejects a destructive Content assignment when a code-behind file happens to exist. Consequently the current “clean” fixture (which has a RootFrame but no MainWindow.xaml.cs) passes, even though the scaffolded window renders an empty frame and the advertised initial-navigation wiring is absent.
            if (mainWindowCs is not null)
            {
                var cs = SafeRead(mainWindowCs);
                if (DestructiveContent().IsMatch(cs))
                {
                    fails.Add("MainWindow.xaml.cs assigns Content directly — this overwrites XAML-defined layout and causes a blank window.");
                }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:275

  • This validates XML serialization rather than the manifest value: the regex requires Name="Windows.Desktop" to be the first attribute and to use double quotes. A semantically valid element such as <TargetDeviceFamily MinVersion="10.0.19041.0" Name="Windows.Desktop" /> is therefore reported as a packaging failure. Parse the manifest with XDocument and inspect attributes by local name so attribute order and quoting do not change the result.
            if (!TargetDeviceDesktop().IsMatch(text))
            {
                pkgFails.Add("<TargetDeviceFamily> is not Windows.Desktop (Windows.Universal is UWP-only; the registrar rejects it for a Win32 entrypoint).");
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateCommand.cs:16

  • This public CLI family has no corresponding section in the hand-written docs/usage.md; regenerating the schema and npm API reference does not document the native winapp migrate workflow, arguments, outputs, or examples for CLI users. Add the command family to the native CLI usage documentation.
        : base("migrate", "Migrate apps to WinUI 3 / Windows App SDK. Use 'migrate scaffold --from-uwp' to copy UWP source into a WinUI 3 project and apply mechanical transforms, and 'migrate validate --from-uwp' to gate a completed migration on residue / single-project / manifest checks.")
    {
        Subcommands.Add(migrateScaffoldCommand);
        Subcommands.Add(migrateValidateCommand);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:120

  • Unreadable source files are silently skipped, allowing the residue gate—and potentially the overall validation—to pass without examining all non-deferred code. A static completion gate should treat a read failure as a failed/indeterminate check and report the path, rather than certifying a project whose source could not be scanned.
                string[] lines;
                try { lines = File.ReadAllLines(file); } catch { continue; }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:327

  • Directory.EnumerateFiles is lazy, so this try only creates the enumerable; access/path errors encountered while the foreach advances it occur outside the catch and abort validation. The traversal should surface an unreadable subtree as a validation failure rather than crashing or silently certifying an incomplete scan.
            IEnumerable<string> files;
            try { files = Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories); }
            catch { yield break; }
            foreach (var f in files)

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:656

  • Recursive enumeration can throw when any nested directory is inaccessible. Because this iterator is consumed throughout the mutation sequence without error handling, scaffold can terminate after copying some files and leave a partially migrated target; enumerate/validate the inputs before mutation or convert traversal failures into an explicit command failure.
        private static IEnumerable<string> EnumerateFiles(string root)
        {
            foreach (var f in Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories))
            {

Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateCommand.cs
Comment thread src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs
The asset denylist copied everything not on it, but private signing keys
(.pfx / .snk / .p12 / .pvk / .cer / .key) should never be treated as source
content — copying them into the migrated project risks committing or sharing
private keys. Add them to DoNotCopyExtensions. Addresses PR review feedback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 401212b2-aeb4-40ef-8c4b-a429ee700740
Copilot AI review requested due to automatic review settings July 30, 2026 07:11
@qiutongMS shisan (qiutongMS) changed the title winapp: add 'migrate' command family (scaffold / analyze / validate) for UWP → WinUI 3 winapp: add 'migrate' command family (scaffold / validate) for UWP -> WinUI 3 Jul 30, 2026

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 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (22)

src/winapp-CLI/WinApp.Cli/Commands/MigrateCommand.cs:16

  • The PR description and test summary still advertise a third migrate analyze command and analyzer-backed validation, but the registered command family now contains only scaffold and validate. Since the analyzer removal is intentional per the resolved thread, update the PR description to match the shipped API rather than documenting a command users cannot invoke.
    public MigrateCommand(MigrateScaffoldCommand migrateScaffoldCommand, MigrateValidateCommand migrateValidateCommand)
        : base("migrate", "Migrate apps to WinUI 3 / Windows App SDK. Use 'migrate scaffold --from-uwp' to copy UWP source into a WinUI 3 project and apply mechanical transforms, and 'migrate validate --from-uwp' to gate a completed migration on residue / single-project / manifest checks.")
    {
        Subcommands.Add(migrateScaffoldCommand);
        Subcommands.Add(migrateValidateCommand);

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:136

  • Using || here accepts any directory with any .csproj as a WinUI 3 scaffold, even if it has no MainWindow.xaml or WinUI settings. The command can then copy over that unrelated project and still finish successfully after merely warning that shell wiring was skipped. Require the advertised WinUI scaffold shape before any files are copied.
            var targetHasScaffold = FindFile(targetRoot, "MainWindow.xaml") is not null
                || EnumerateFiles(targetRoot).Any(f => f.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase));
            if (!targetHasScaffold)
            {
                Console.Out.WriteLine("[ERROR] Target is not a WinUI 3 scaffold — no .csproj or MainWindow.xaml found. Run 'dotnet new winui' first.");
                return Task.FromResult(1);

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:569

  • An unrecognized but valid WinUI layout only produces this warning; execution continues to SCAFFOLD COMPLETE and returns 0 even though the promised RootFrame/navigation transform was not performed. The repository's own samples/winui-app/MainWindow.xaml:14-85 uses a populated Grid that does not match the sole <Grid Grid.Row="1" /> pattern. Insert the frame using structured XAML handling, or fail instead of reporting successful scaffolding.
            if (!rootFrameReady)
            {
                Console.Out.WriteLine("    WARNING: MainWindow.xaml has no <Frame x:Name=\"RootFrame\"> (unrecognized layout) — Navigate injection skipped to avoid a broken build.");
                return;

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:172

  • RootFrame() runs against raw XAML, so a commented-out <Frame x:Name="RootFrame"> satisfies the gate and can produce an overall PASS with no real frame. Sanitize XML comments here just as the residue check already does before applying text markers.
                var xaml = SafeRead(mainWindowXaml);
                if (!RootFrame().IsMatch(xaml))
                {
                    fails.Add("MainWindow.xaml is missing <Frame x:Name=\"RootFrame\"> — app content will not render.");

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:180

  • DestructiveContent() is applied to raw C# text, so a comment or string such as // Content = new Grid incorrectly fails validation. Reuse the existing C# sanitizer before matching so only executable code is considered.
                var cs = SafeRead(mainWindowCs);
                if (DestructiveContent().IsMatch(cs))
                {
                    fails.Add("MainWindow.xaml.cs assigns Content directly — this overwrites XAML-defined layout and causes a blank window.");

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:88

  • The diagnostics write failure is silently swallowed, but the later FAIL summary unconditionally tells users that full diagnostics are in .validator-diagnostics.txt. On a read-only directory, access failure, or cancellation, that file is absent/stale and the actionable details are lost. Track write success and report the write error or adjust the summary accordingly.
            var diagPath = Path.Combine(root, ".validator-diagnostics.txt");
            if (diagnostics.Length > 0)
            {
                try { await File.WriteAllTextAsync(diagPath, diagnostics.ToString(), cancellationToken); } catch { /* best effort */ }
            }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:437

  • These regexes reject semantically valid manifests based on XML formatting—for example, TargetDeviceFamily with Name after another attribute, or a non-self-closing rescap:Capability, fails despite declaring the required values. Repository guidance requires structured XML handling, and Services/AppxManifestDocument.cs already provides namespace-aware XDocument access. Parse elements/attributes instead of matching serialized text.
    [GeneratedRegex("<TargetDeviceFamily\\s+Name=\"Windows\\.Desktop\"")] private static partial Regex TargetDeviceDesktop();
    [GeneratedRegex("xmlns:rescap\\s*=\\s*\"http://schemas\\.microsoft\\.com/appx/manifest/foundation/windows10/restrictedcapabilities\"")] private static partial Regex RescapNamespace();
    [GeneratedRegex("IgnorableNamespaces\\s*=\\s*\"[^\"]*\\brescap\\b[^\"]*\"")] private static partial Regex RescapIgnorable();
    [GeneratedRegex("<rescap:Capability\\s+Name=\"runFullTrust\"\\s*/>")] private static partial Regex RunFullTrust();

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:43

  • The private-key denylist omits common private key formats such as PEM and PuTTY PPK. Those files are copied into the migrated target and can then be committed despite the stated guarantee that private signing material is never copied. Exclude these common private-key extensions as well.
        // Private signing material — must never be copied into (and risk being committed with)
        // the migrated project.
        ".pfx", ".snk", ".p12", ".pvk", ".cer", ".key"

src/winapp-CLI/WinApp.Cli/Commands/MigrateCommand.cs:16

  • This introduces a public CLI workflow without any matching hand-written native CLI or migration-skill documentation. AGENTS.md:36-46 and AGENTS.md:68-73 explicitly require public command/workflow changes to update docs/usage.md and docs/fragments/skills/; neither tree currently mentions migrate, so the generated schema/npm reference alone does not make the workflow available to CLI users or the migration agent.
        : base("migrate", "Migrate apps to WinUI 3 / Windows App SDK. Use 'migrate scaffold --from-uwp' to copy UWP source into a WinUI 3 project and apply mechanical transforms, and 'migrate validate --from-uwp' to gate a completed migration on residue / single-project / manifest checks.")
    {
        Subcommands.Add(migrateScaffoldCommand);
        Subcommands.Add(migrateValidateCommand);

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:128

  • This predicate treats any directory containing either an arbitrary .csproj or an appxmanifest as a UWP project. For example, the tests use a plain SDK UseWinUI=true project as the “UWP” source, and it passes this check, after which UWP-specific copying and rewrites run against it. Validate the expected UWP project/manifest shape (and the documented project root) before mutating the target.
            var sourceHasProject = EnumerateFiles(sourceRoot).Any(f =>
                f.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)
                || string.Equals(Path.GetFileName(f), "Package.appxmanifest", StringComparison.OrdinalIgnoreCase));
            if (!sourceHasProject)
            {
                Console.Out.WriteLine("[ERROR] Source is not a UWP project — no .csproj or Package.appxmanifest found. Nothing to migrate.");
                return Task.FromResult(1);

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:43

  • The denylist says project/system files are not copied, but it omits MSBuild .props and .targets (and packaging .wapproj) files. A source Directory.Build.props is therefore copied with overwrite enabled into the target and is automatically imported, potentially reapplying UWP settings and breaking the WinUI project. Treat these as project metadata rather than ordinary runtime content.
    private static readonly string[] DoNotCopyExtensions =
    [
        ".csproj", ".vcxproj", ".vbproj", ".shproj", ".projitems", ".sln", ".slnf",
        ".user", ".suo", ".cache", ".vsidx", ".pdb", ".ilk", ".exp", ".idb", ".tlog",
        ".exe", ".dll", ".lib", ".obj", ".appxmanifest",
        ".appx", ".msix", ".appxbundle", ".appxupload", ".nupkg", ".snupkg",
        // Private signing material — must never be copied into (and risk being committed with)
        // the migrated project.
        ".pfx", ".snk", ".p12", ".pvk", ".cer", ".key"

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:173

  • The shell gate checks only for a named Frame, not whether anything navigates into it. If scaffolding finds a RootFrame but cannot resolve MainPage (and therefore skips RootFrame.Navigate at lines 577-581), this method still emits PASS and the overall validator can approve a blank window. Check for an initial navigation/source target or another concrete content path before passing.
                var xaml = SafeRead(mainWindowXaml);
                if (!RootFrame().IsMatch(xaml))
                {
                    fails.Add("MainWindow.xaml is missing <Frame x:Name=\"RootFrame\"> — app content will not render.");
                }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:253

  • The wildcard accepts any same-prefix image as a scale/targetsize variant. For example, a missing Assets/Logo.png is considered present if only Assets/LogoOld.png exists, so the packaging gate can pass a manifest with an unresolved resource. Match valid package resource qualifier forms rather than ${baseName}*${ext}.
                var dir = Path.GetDirectoryName(abs);
                var baseName = Path.GetFileNameWithoutExtension(abs);
                var ext = Path.GetExtension(abs);
                if (dir is not null && Directory.Exists(dir)
                    && Directory.EnumerateFiles(dir, $"{baseName}*{ext}").Any())
                {
                    continue; // scale-*/targetsize-* variant present

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:368

  • String state is recreated for every line, so multiline verbatim strings and raw string literals are not actually blanked. A valid value such as var sample = @"\nusing Windows.UI.Xaml\n"; causes the second line to be scanned as code and fails the residue gate. Carry lexical state across lines and handle raw-string delimiters, or use a C# syntax-aware scanner.
        private static string SanitizeCSharpLine(string line, ref bool inBlockComment)
        {
            var sb = new StringBuilder(line.Length);
            bool inLineComment = false, inString = false, inChar = false, inVerbatim = false;
            for (int j = 0; j < line.Length; j++)

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:523

  • The class boundary scanner counts braces inside comments, character literals, and strings as syntax. A helper containing something as ordinary as var json = "}"; makes depth reach zero early, so the rewrite splices the remainder outside the class and corrupts the C# file. Use a syntax-aware/lexically aware class-body transform rather than raw brace counting.
            int bodyStart = m.Index + m.Length;
            int depth = 1, i = bodyStart;
            while (i < text.Length && depth > 0)
            {
                if (text[i] == '{')
                {
                    depth++;
                }
                else if (text[i] == '}')
                {
                    depth--;
                }

                i++;
            }
            if (depth != 0)
            {
                return false;

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:351

  • When Styles.xaml exists but App.xaml uses any valid resource shape outside these two exact regexes (for example a ResourceDictionary with attributes), this branch silently returns and the required dictionary is never registered, while scaffolding still reports success. Parse the XAML with XDocument and add the merged dictionary structurally, as required by the repository XML-handling guidance.
            if (MergedDictionariesOpen().IsMatch(body))
            {
                updated = MergedDictionariesOpen().Replace(body,
                    m => m.Value + "\r\n                <ResourceDictionary Source=\"Styles.xaml\"/>", 1);
            }
            else if (AppResourcesOpen().IsMatch(body))
            {
                updated = AppResourcesOpen().Replace(body,
                    m => m.Value + "\r\n            <ResourceDictionary.MergedDictionaries>\r\n                <ResourceDictionary Source=\"Styles.xaml\"/>\r\n            </ResourceDictionary.MergedDictionaries>", 1);
            }
            else
            {
                return;

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:327

  • Directory.EnumerateFiles is lazy, so the try only assigns an enumerable and does not catch failures from actually traversing it. An inaccessible child directory therefore throws from the subsequent foreach, aborting validation rather than producing a gate result. Enumerate with an explicit inaccessible-path policy or catch traversal exceptions during iteration.
        private static IEnumerable<string> EnumerateSource(string root, params string[] extensions)
        {
            IEnumerable<string> files;
            try { files = Directory.EnumerateFiles(root, "*", SearchOption.AllDirectories); }
            catch { yield break; }
            foreach (var f in files)

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:246

  • Manifest references are combined without checking that the resolved path remains under root. A reference such as ..\outside.png passes whenever that external file exists, even though it is not a package resource and packaging cannot resolve it. Canonicalize the path and reject rooted or parent-traversing references before checking existence.
            foreach (var reff in imageRefs)
            {
                var abs = Path.Combine(root, reff.Replace('/', Path.DirectorySeparatorChar));
                if (File.Exists(abs)) { continue; }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:241

  • Image references are extracted from serialized XML with formatting-specific regexes, so valid manifests using single-quoted attributes or multiline <Logo> text are not checked. This can leave imageRefs empty or incomplete and let missing package assets pass. Use AppxManifestDocument/XElement to enumerate the relevant elements and attributes namespace-independently.
            // 5a. image references resolvable under the project
            var imageRefs = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
            foreach (Match m in LogoElement().Matches(text)) { imageRefs.Add(m.Groups[1].Value.Trim()); }
            foreach (Match m in ImageAttr().Matches(text)) { imageRefs.Add(m.Groups[1].Value.Trim()); }

src/winapp-CLI/WinApp.Cli/Helpers/QuietFilteringTextWriter.cs:59

  • The common --quiet behavior keeps warnings enabled (Program.cs:41-44 sets the minimum log level to Warning), but this filter drops every [WARN] line. That hides important successful-exit warnings such as skipped RootFrame/navigation wiring or skipped manifest checks, leaving users with exit 0 and no indication that migration is incomplete. Forward [WARN] alongside errors/failures and update the wrapper summary accordingly.
        if (trimmed.StartsWith("[ERROR]", StringComparison.Ordinal)
            || trimmed.StartsWith("[FAIL]", StringComparison.Ordinal))

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:81

  • The new validator tests never exercise CheckManifest, even though it contains the image-resolution and packaging gates and accounts for a substantial part of this command's contract. Add fixtures for valid and invalid manifests, including qualified image assets and namespace/attribute formatting, so this gate cannot silently drift or reject equivalent XML.
            // ─── 4. Package.appxmanifest packaging requirements ───
            failures += CheckManifest(root, diagnostics);

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:175

  • Most advertised scaffold transforms in this sequence—shared/SharedContent merging, Styles registration, UWP reference preservation, RuntimeIdentifier patching, helper neutralization, and RootFrame navigation—have no tests. The current suite mainly covers raw copy/rewrite and argument failures, so regressions in the core generated project shape are not detected. Add focused fixture tests for these transforms and their idempotent/error paths.
            // ── 1b. Merge sibling shared/ (cross-language SDK-sample layout) ─────
            MergeSiblingShared(sourceRoot, targetRoot, copied);

            // ── 1c. Merge top-level SharedContent/ (repo-wide sample assets) ─────
            MergeSharedContent(sourceRoot, targetRoot, copied);

Copilot AI review requested due to automatic review settings July 30, 2026 07:22

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 13 out of 13 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (5)

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:517

  • The class-body scan counts every brace without recognizing comments, character literals, or strings. A valid RootFrameNavigationHelper containing text such as var token = "}"; makes the scan stop at that brace and splice the stub into the middle of the string, producing corrupted C# while scaffold still reports success. Use a C#-aware tokenizer/parser, or at minimum skip braces inside comments and literals before replacing the body.
            while (i < text.Length && depth > 0)
            {
                if (text[i] == '{')
                {
                    depth++;
                }
                else if (text[i] == '}')
                {
                    depth--;
                }

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:235

  • Parsing the manifest as raw text makes the packaging gate reject semantically valid XML. The regexes below require literal double quotes, a fixed rescap prefix, fixed attribute order, and a self-closing capability tag; equivalent forms such as reordered TargetDeviceFamily attributes or another prefix bound to the restricted-capabilities namespace fail validation. Parse with the existing AppxManifestDocument/XDocument and inspect expanded element/attribute names instead.
            var text = SafeRead(manifest);

src/winapp-CLI/WinApp.Cli/Commands/MigrateValidateCommand.cs:253

  • This wildcard treats any similarly prefixed file as a valid scale/target-size variant. If Logo.png is missing but LogoOld.png exists, Logo*.png matches and validation passes even though packaging cannot resolve the referenced logo. Restrict fallback matches to actual qualifier filenames (for example Logo.scale-* or Logo.targetsize-*).
                if (dir is not null && Directory.Exists(dir)
                    && Directory.EnumerateFiles(dir, $"{baseName}*{ext}").Any())
                {
                    continue; // scale-*/targetsize-* variant present

src/winapp-CLI/WinApp.Cli/Commands/MigrateScaffoldCommand.cs:721

  • This startup-file guard preserves only root App.xaml files, even though the copy loop relies on it to protect the scaffold startup surface. A valid UWP source with a root MainWindow.xaml or code-behind therefore overwrites the WinUI scaffold's MainWindow before shell wiring runs, leaving UWP window code in the target and breaking the migrated app. Preserve the root MainWindow files as well.
        private static bool IsScaffoldStartupFile(string rel)
        {
            var name = Path.GetFileName(rel);
            return string.Equals(name, "App.xaml", StringComparison.OrdinalIgnoreCase)
                || string.Equals(name, "App.xaml.cs", StringComparison.OrdinalIgnoreCase);

src/winapp-CLI/WinApp.Cli/Helpers/QuietFilteringTextWriter.cs:62

  • --quiet is defined repository-wide as warning-level output, but this filter drops every [WARN] line (and scaffold's WARNING: lines). For example, migrate validate --quiet can return success with no output when the manifest check was skipped, hiding the only indication that validation was incomplete. Preserve warnings while filtering progress and [PASS] output.
        if (trimmed.StartsWith("[ERROR]", StringComparison.Ordinal)
            || trimmed.StartsWith("[FAIL]", StringComparison.Ordinal))
        {

Comment on lines +175 to +181
if (mainWindowCs is not null)
{
var cs = SafeRead(mainWindowCs);
if (DestructiveContent().IsMatch(cs))
{
fails.Add("MainWindow.xaml.cs assigns Content directly — this overwrites XAML-defined layout and causes a blank window.");
}
Comment on lines +407 to +410
var m = HostArchRuntimeIdentifier().Match(body);
if (!m.Success)
{
continue;
@github-actions

Copy link
Copy Markdown
Contributor

Build Metrics Report

Binary Sizes

Artifact Baseline Current Delta
CLI (ARM64) 36.63 MB 36.80 MB 📈 +179.0 KB (+0.48%)
CLI (x64) 36.82 MB 36.98 MB 📈 +168.5 KB (+0.45%)
MSIX (ARM64) 15.26 MB 15.31 MB 📈 +53.0 KB (+0.34%)
MSIX (x64) 16.20 MB 16.28 MB 📈 +80.9 KB (+0.49%)
NPM Package 31.83 MB 31.97 MB 📈 +137.7 KB (+0.42%)
NuGet Package 31.86 MB 32.00 MB 📈 +141.8 KB (+0.43%)

Test Results

3736 passed, 4 skipped out of 3740 tests in 681.4s (+17 tests, +71.6s vs. baseline)

Test Coverage

93.4% line coverage, 86.8% branch coverage · ⚠️ -1.1% vs. baseline

CLI Startup Time

46ms median (x64, winapp --version) · ✅ no change vs. baseline

Try This Build

Installs 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))) 695
Switching between builds often?

Put the tool on your PATH once:

& ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPath

Then this build is just:

winapp-pr 695

Run winapp-pr with no arguments to pick from a list of open PRs.


Updated 2026-07-30 07:46:31 UTC · commit 2a586e8 · workflow run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants