You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Adopt ApplicationData.GetForUnpackaged(); bump Windows App SDK to 2.2.0 - #1259
Bumps Windows App SDK 2.1.3 to 2.2.0 and adopts the new ApplicationData.GetForUnpackaged() API for unpackaged window-placement persistence, closing the row deferred in spec 059 section 5. Along the way it fixes a data-loss bug in the default persistence store, and 22 stale SDK version pins that the previous bump structurally could not see (18 of which ship inside the NuGet agent kit).
tests/Reactor.Tests/UnpackagedAppDataStoreTests.cs (12 tests). These assert storage location, not round-trip behavior: both stores write the same JSON shape, so a round-trip would pass whichever one ran and would prove nothing about the adoption.
tests/Reactor.Tests/JsonFileStoreConcurrencyTests.cs — the oracle is a count of surviving ids, because a round-trip passes whenever the last writer wins, which is exactly the broken behaviour.
ETW store-label coverage for write, read and both rejection arms.
WinAppSDKReferenceGuardTests.No_literal_SDK_pin_sits_below_the_central_pinned_version — sweeps three pin shapes, with positive controls that must match in production files so scanner decay reddens rather than passing.
Mutation-verified. Removing the write guard drops the concurrency suite to 1 of 8 surviving. Injecting a stale pin (wildcard, multiline, reversed attribute order, VersionOverride, sub-package, script $ver) reddens the guard with the offending file:line. Breaking the element regex reddens the positive control.
dotnet restore Reactor.slnx then dotnet build Reactor.slnx --no-restore -c Release: 0 errors attributable to this change, matching a baseline measured with the changes stashed.
dotnet test tests/Reactor.Tests: 14066 total, 0 failed.
Ran the repo pr-review skill, then 13 rounds of Copilot review with all findings addressed.
Risk / breaking changes
Behaviour change in the default store — read this even if you skip the rest. The concurrency fix lands in JsonFileStore, which is the auto-detected default for every unpackaged app, so it is not opt-in. Writes now take a cross-process lock file (a .lock sidecar appears beside reactor-windows.json), wait up to 250 ms for it, and abandon the save if it cannot be acquired. That last point is deliberate: proceeding would merge a stale document and drop another window's entry, which is the bug being fixed, whereas skipping costs at most one window's placement — something the store's best-effort contract already permits. The commit path also moves from File.Move(overwrite: true) to File.Replace.
The new API is opt-in.UnpackagedAppDataStore is only used if an author assigns it; auto-detection still selects JsonFileStore. The two stores key data differently (publisher/product vs entry process name), so switching does not migrate existing layouts — an app that opts in starts from an empty layout at the new root, and the old file stays on disk, unread.
Consumer floor rises.Microsoft.UI.Reactor is framework-dependent and flows only the lean WinUI sub-package, so the floor consumers inherit is Microsoft.WindowsAppSDK.WinUI >= 2.2.1; self-contained/MSIX projects that reference the metapackage themselves move to 2.2.0. This is in-place servicing within the same side-by-side runtime family (Microsoft.WindowsAppRuntime.2), so no new runtime generation is required — the same reasoning spec 059 D5 accepted for 2.1.3.
New public API, deliberately minimal: a constructor plus the two IWindowPersistenceStore methods. Path is internal (spec 063 D7) so the public surface stays identical under every outcome of the LocalPath-vs-LocalSettings revisit that section 5 leaves open.
Scope note: this touches window placement only. Dock layouts use a different mechanism (WindowPersistedScope, an IPersistedStateScope) and are untouched.
Notes for reviewers
The diff is lopsided. About 30 files are mechanical one-line version-pin edits plus a regenerated API index and guide, skimmable in one pass. The parts worth real attention are UnpackagedAppDataStore.cs, CrossProcessWriteGuard.cs, JsonFileStore.cs, their tests, and spec 063.
Why LocalPath and not LocalSettings. On 2.2.0, GetForUnpackaged().LocalSettings writes to a roaming registry hive instead of the machine-local key it is contracted to use. That was measured, not taken from the release note, with a three-arm probe so that "found in neither hive" would have been reported as a broken probe rather than as absence of the defect. Window placement is monitor-topology and DPI dependent, so roaming it restores windows onto monitors that do not exist on the current machine.
Worth knowing: the published release notes attribute that fix to 2.5.1, and that attribution is wrong — a two-arm experiment places it at metapackage 2.3.1 / Foundation 2.3.5. The follow-up trigger is therefore keyed to an observable condition enforced by a live test, not to a version number. Section 3.2 has the detail, including why behaviour follows the loaded runtime rather than the SDK pin.
Why the pin fixes are in scope. Spec 059 told future bumps to grep Microsoft.WindowsAppSDK" Version=. That shape cannot match a #:package Microsoft.WindowsAppSDK@<version> header, so all 22 occurrences across 20 files were missed and sat a full major-minor below the floor that PR itself raised. Three of them live in a generator system prompt that was teaching an LLM to emit the stale pin into new apps. Fixing the instances without fixing the class would have guaranteed a third occurrence at the next bump, so the prose instruction is replaced by a guard test whose positive controls must be satisfied from production files.
Closes the ApplicationData.GetForUnpackaged() row deferred in spec 059 §5.
Full design, evidence and decisions: docs/specs/063-unpackaged-app-data.md.
SDK bump
--------
WindowsAppSDKVersion 2.1.3 -> 2.2.0 and WindowsAppSDKWinUIVersion 2.1.0 ->
2.2.1. The WinUI sub-package version is neither equal to nor a fixed offset
from the metapackage version, and is not "the newest WinUI on the feed"
(2.3.6/2.3.9 belong to later trains) - it is read from the 2.2.0
metapackage's own nuspec dependency. The stale comment claiming WinUI
"trails" the aggregate version is corrected accordingly.
Literal pins: fixing the class, not the instance
------------------------------------------------
Spec 059 §3 told future bumps to grep `Microsoft.WindowsAppSDK" Version=`.
That shape is structurally blind to the file-based-app `#:package
Microsoft.WindowsAppSDK@<version>` header, so PR #723 missed all 21
instances of it. They sat at 2.0.1 - a full major-minor below the floor that
PR itself raised - for an entire release cycle. 18 of them are recipe files
packed into the shipped NuGet agent kit, so this was a user-facing NU1605
downgrade for anyone who ran a recipe, not internal-only staleness. Three
more live in the demo-script generator's system prompt, which teaches an LLM
which pin to emit, so the stale value propagated into generated apps.
All 21 are now 2.2.0, and the prose instruction is replaced by a gate:
WinAppSDKReferenceGuardTests.No_literal_SDK_pin_sits_below_the_central_pinned_version
sweeps the tree for both pin shapes and fails on any literal below
$(WindowsAppSDKVersion). Forward pins are allowed - docs legitimately state
per-feature minimums and a forward pin is not a downgrade. The guard asserts
both shapes are observed somewhere, so a decayed regex reddens instead of
reporting zero offenders and passing exactly as green as a clean tree.
Adoption
--------
New public UnpackagedAppDataStore persists to
<GetForUnpackaged(publisher, product).LocalPath>/reactor-windows.json,
keyed on a stable publisher/product pair instead of the entry process name
(which changes on executable rename and collides across apps).
It uses LocalPath and deliberately NOT LocalSettings. Measured on 2.2.0:
GetForUnpackaged().LocalSettings writes to HKCU\SOFTWARE\<pub>\<prod> - a
roaming hive - while the machine-local
HKCU\SOFTWARE\Classes\Local Settings\Software\... key is never created
(WindowsAppSDK#6559, fixed in 2.5.1). Window placement is monitor-topology
and DPI dependent, so roaming it restores windows onto monitors that do not
exist on the current machine. LocalPath resolves under %LOCALAPPDATA%, which
does not roam, and is correct on 2.2.0 today. The probe distinguished
"found in the roaming hive" from "found in neither" so a wrong probe could
not read as absence of the defect.
The store is opt-in; auto-detection still picks JsonFileStore when
unpackaged. The two stores key their data differently, so flipping the
default would strand existing apps' saved layouts with no migration path.
Implementation composes JsonFileStore over the SDK root rather than forking
its hardened I/O (atomic rename, 1 MB cap, AOT-safe JSON). JsonFileStore
gains an internal ctor overload taking a trace label so the two stay
distinguishable on the spec 044 Persistence trace - asserted, so the
refactor is not paying for an unverified claim.
Not simplified
--------------
PackagedSettingsStore's IsAvailable() probe and its three catch arms are
deliberately kept: they guard Windows.Storage.ApplicationData.Current, a
different API that this bump does not affect, and PackagedSettingsStoreTests
demonstrates the arms are live. Not dead code.
Verification
------------
WinUiNavigationMotionParityTests' drift guard re-verified rather than merely
re-pinned: the four WinUI sources backing every copied motion constant are
byte-identical between microsoft-ui-xaml tags winui3/release/2.1.3 and
winui3/release/2.2.0 (blob SHAs recorded in the test). The 22-file tag diff
is the positive control proving the comparison can detect change.
The discharge METHOD is now recorded in the guard's own failure message, so
the next person inherits it instead of re-deriving it. Three details decide
whether the guard is dischargeable at all: tags are keyed to the SDK
metapackage version (no winui3/release/<winui-subpackage> tag exists); the
tree is rooted under src/ at a release tag but NOT on winui3/main, so
querying the main-branch path against a tag 404s on every file and reads as
"this tree isn't published at tags"; and a README-resolves positive control
cannot detect that, because README.md exists under both layouts. Measured:
all four files 404 at dxaml/phone/lib/... and resolve at
src/dxaml/phone/lib/... at 2.1.3, 2.2.0 and 2.3.1 alike, with identical blob
SHAs throughout.
Store tests assert storage location, not round-trip behaviour - both stores
write the same document shape, so a round-trip would pass whichever one ran.
Four mutations (wrong root, touching LocalSettings, flipping the default,
reintroducing a stale pin) were each caught by exactly the intended test.
The roaming guard carries its own positive control and doubles as the
trigger to revisit this decision when a future bump picks up the 2.5.1 fix.
Reactor.Tests: 14010 total, 0 failed. Release solution build matches the
pre-change baseline exactly (3 pre-existing NU1301s from blocked nuget.org
in src/vs-reactor, reproduced with the changes stashed).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Doc/spec-only follow-up to the 2.2.0 adoption. No behaviour change; the
store, the default, and every assertion are unchanged. Reactor.Tests
14010/0; Release build matches the pre-change baseline.
Three corrections, all pointing the same way: the release note this spec
originally trusted is wrong, and the version it named was never the right
thing to key a roadmap to anyway.
1. The #6559 fix does NOT land in 2.5.1 as its release notes claim. A
companion two-arm experiment, run on one machine from this branch's own
2.2.0 base with registry and %LOCALAPPDATA% wiped between arms, places it
at metapackage 2.3.1: arm A (2.2.0 / Foundation 2.1.0) reports the roaming
hive, arm B (2.3.1 / Foundation 2.3.5) the machine-local one, both loading
a bundled runtime so the machine's 2.5.1 SbS package is excluded in both
directions. Arm A doubles as arm B's negative control and independently
reproduces §3.1 from a separate worktree. So the follow-up is reachable a
release earlier than written.
2. The boundary is a Foundation version, not a metapackage version, and the
pairing is neither stable nor monotonic - verified against the nuspecs:
2.2.0->2.1.0 (Foundation below), 2.3.1->2.3.5 (above), 2.5.1->2.3.12. A
trigger naming a metapackage version is two indirections from the binaries
that decide the behaviour. The trigger is now the observable condition -
the roaming key stops appearing - enforced by the guard's positive control,
which cannot rot the way an inferred version number can.
3. New §3.2 records why a framework-dependent probe cannot attribute this
behaviour to a pinned SDK version at all: GetForUnpackaged's registry
behaviour lives in the in-place-serviced 2.x runtime. Reactor.Tests is
self-contained and was measured loading Microsoft.WindowsAppRuntime.dll
from its own bin\ rather than WindowsApps\, so §3.1 is attributable - and
the confound only runs toward the newer claim, since loading a fixed
runtime cannot manufacture the roaming result §3.1 observed.
That last point sharpens the D1/D2 rationale rather than weakening it: the
same source at the same pin splits two ways - self-contained loads the
bundled Foundation and roams, framework-dependent loads the machine's
runtime and may not. A default whose correctness depends on packaging mode
and on what happens to be installed is a bad default regardless of which
release fixed the bug, and unlike the defect itself that split persists for
as long as any unfixed 2.x runtime is in the wild. LocalPath has no such
variance in either mode.
The guard's failure message now also tells the next reader to identify the
runtime by hashing the loaded binaries back to a Foundation package, not
just by checking the module path: a bin\ path proves the runtime is bundled,
but not which Foundation produced the behaviour.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keeps the new store's public surface to the constructor plus the two
IWindowPersistenceStore methods, so it is identical under every outcome of
the LocalPath-vs-LocalSettings revisit that §5 leaves open.
JsonFileStore.Path stays public because that type is definitionally
file-backed - a path is part of what it is. This store is "the SDK app-data
store"; its file is an artifact of the §6 D1 choice to build on LocalPath
rather than LocalSettings. Exposing a path promises a file-backed store we
may not want to keep promising, and public -> internal later would be a
breaking change, while internal -> public is additive and can wait for an
actual request.
Barely reachable in practice either way: ReactorApp.WindowPersistenceStore
is typed IWindowPersistenceStore?, which has no Path member, so reading it
required a caller's own concrete-typed reference.
Tests reach it through InternalsVisibleTo("Reactor.Tests"), unchanged.
Regenerated both reactor.api.txt copies (Path drops from the index; the two
stay byte-identical). New §6 D7 records the decision.
Reactor.Tests 14010/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Artifact sizes for 8ccd196 vs the base branch (40dff87).
Packages (compressed .nupkg)
Artifact
base
PR
Δ
Microsoft.UI.Reactor.nupkg
1.70 MB
1.71 MB
+6.07 KB (+0.35%)
⚠️
Microsoft.UI.Reactor.Advanced.nupkg
487.6 KB
487.6 KB
-10 B (0.00%)
≈
Microsoft.UI.Reactor.Devtools.nupkg
283.2 KB
283.2 KB
-8 B (0.00%)
≈
Assemblies in Microsoft.UI.Reactor
Artifact
base
PR
Δ
Reactor.Analyzers.dll
373.0 KB
373.0 KB
+0 B (0.00%)
≈
Reactor.dll
2.44 MB
2.44 MB
+3.50 KB (+0.14%)
⚠️
Reactor.Localization.Generator.dll
16.0 KB
16.0 KB
+0 B (0.00%)
≈
Reactor.Wrappers.Abstractions.dll
10.5 KB
10.5 KB
+0 B (0.00%)
≈
Reactor.Wrappers.Generator.dll
99.5 KB
99.5 KB
+0 B (0.00%)
≈
Assemblies in Microsoft.UI.Reactor.Advanced
Artifact
base
PR
Δ
Reactor.Advanced.dll
1021.5 KB
1021.5 KB
+0 B (0.00%)
≈
Assemblies in Microsoft.UI.Reactor.Devtools
Artifact
base
PR
Δ
Microsoft.UI.Reactor.Devtools.dll
784.0 KB
784.0 KB
+0 B (0.00%)
≈
✅ smaller / ⚠️ larger / ≈ within noise. Sizes come from a Release dotnet pack on the CI runner: packages are the compressed .nupkg download size, assemblies the uncompressed DLL inside it. workflow run.
Coverage for 8ccd196 vs the base branch (feafd90) — unit + selftest merged.
Metric
base
PR
Δ
Line
85.77%
85.79%
+0.02 pp
≈
Branch
77.55% (957/1234)
77.58% (962/1240)
+0.03 pp
≈
No coverage change beyond the noise floor. ✅
✅ higher / ⚠️ lower / ≈ within noise. Δ is in percentage points; coverage is unit + selftest merged (Debug x64) on the CI runner. Cobertura reports attached to the workflow run as artifacts.
§3.2 claimed the self-contained vs framework-dependent LocalSettings split
"does not go away when the fix propagates... persists for as long as any
unfixed 2.x runtime is in the wild". That was reasoning, not measurement,
and it is wrong.
The split is a property of pinning BELOW the fix, not a permanent property
of in-place servicing. The SDK's generated MddBootstrapAutoInitializer
passes the build-time Runtime.Version as minVersion and hard-exits when no
compatible runtime resolves (measured by varying only minVersion: 2.3.1.0 ->
TryInitialize=True; impossible minimum -> False / 0x80670016). So at a
>= 2.3.1 pin a framework-dependent app cannot load an unfixed runtime and a
self-contained one bundles a fixed Foundation; the modes converge. Reactor's
pin is a transitive NuGet floor, so consumers are covered too.
Consequence: the deployment-variance argument is real at the 2.2.0 pin this
spec ships, but it EXPIRES at 2.3.1 - so it is not the durable reason for
D1. D1 now leads with the retirement test instead, which is the repo's
actual "don't reimplement platform machinery" criterion (spec 033, spec
017): adopting LocalSettings retires nothing, because JsonFileStore stays as
the default holding all existing data and PackagedSettingsStore stays, and
it would GROW this store from a ~3-line composition into a near-duplicate of
PackagedSettingsStore. The shipped design already satisfies the principle
where it applies, delegating both root resolution and publisher/product
validation to the SDK.
Also records a known limitation the adoption makes newly reachable:
JsonFileStore does read-merge-write under a per-INSTANCE lock through a
shared .tmp, so separate processes can drop each other's entries even when
writing different ids. Its class doc claimed a FileShare.None protection
that is not in the code (both read sites use FileShare.Read) and scoped
cross-process out because the store "is keyed off the entry process's name"
- true for JsonFileStore, where two differently-named executables get
different files, but NOT for UnpackagedAppDataStore, where a publisher/
product root is shared across executables. Doc corrected to describe actual
behaviour; the defect itself is pre-existing and belongs in the shared
JsonFileStore machinery, where fixing it also benefits the default path.
Findings from the companion LocalSettings/2.3.1 investigation, which
independently recommends keeping this design. Docs only; no product
behaviour change, no public API change (index unaffected).
Reactor.Tests 14010/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ten findings across three classes, all on code this PR adds.
Path.Combine -> Path.Join (4 sites). Combine silently discards every
earlier argument when a later one is rooted. Every current call site passes
a non-rooted literal, so behaviour is unchanged today; Join keeps that true
if a segment ever becomes caller-supplied. Matches the existing convention
in WinUiNavigationMotionParityTests.
Bare `catch { }` -> narrowed catches with a diagnostic (3 sites, flagged
twice each as "generic catch clause" and "empty catch block"). Test cleanup
legitimately tolerates a transient lock or an ACL denial, so those are
caught by type; anything else is a real defect and now surfaces instead of
being swallowed. Each writes a Debug.WriteLine so a leaking test run is
diagnosable rather than silent.
Left alone: the pre-existing bare catch and Path.Combine in
PersistenceEtwBridgeTests' own ctor/Dispose (lines 47/56), which predate
this PR and were not flagged.
Reactor.Tests 14010/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
critical (1 vote): wildcard pins are exempted. critical (2 votes): multiline references are missed. moderate (1 vote):.dt files are excluded. nit (1 vote): scope counts conflate 22 occurrences, 20 files, and 18 recipe files. critical (1 vote): the positive control can match its own XML-doc placeholder.
nit (3 votes): the inventory reports 21 pins, but the breakdown is 22 occurrences across 20 files.
docs/specs/059-titlebar-drag-regions.md
nit (3 votes): the row still attributes the fix to 2.5.1 instead of 2.3.1/Foundation 2.3.5.
Directory.Build.props
Raises the central SDK version; no final comment.
CHANGELOG.md
nit (2 votes): reports 21 files instead of 22 literal occurrences across 20 files.
bootstrap.ps1
Updates runtime-version messaging; no final comment.
Review details
Suppressed comments (4)
skills/reactor.api.txt:6048
The generated API index exposes UnpackagedAppDataStore, but the windowing authoring docs remain unchanged: skills/windowing.md, plugins/reactor/skills/reactor-windowing/SKILL.md, and the windows/persistence guide template only show .WithPersistence/SavePlacement and never explain assigning ReactorApp.WindowPersistenceStore with a publisher/product pair. Add a short opt-in example to the source docs and regenerate the guide so users of the shipped agent kit can discover the new public API.
The custom _storeKind is used for read, rejection, and write events, but this new test asserts only PersistenceWrite. A regression that emitted json-file for TryRead or PersistenceRejected would still pass; add at least a custom-store read and rejection assertion, or make the existing ETW cases exercise the new store kind too. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:500
The guard claims to sweep the tree, but PinScanExtensions excludes .dt, while the docs pipeline's source files are *.md.dt. A future #:package Microsoft.WindowsAppSDK@... pin in a guide template would therefore bypass both the positive-control scan and the stale-pin check. Include the .dt extension (or otherwise enumerate the tracked template sources) before relying on this as the replacement for the manual grep. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:511
This comment says there are 21 shipped agent-kit recipes, but src/Reactor/Reactor.csproj packs 8 skills/recipes files plus 10 plugin recipe reference files, i.e. 18 recipe files; the three SystemPrompt.txt occurrences are not in the agent kit. Please separate the 22 total occurrences, 20 files, and 18 shipped recipe files so the documented scope is accurate.
coverage, correct pin counts, document the concurrency limit
Guard test (4 findings, all real holes I introduced):
- Multiline PackageReference was invisible: the scan ran per line, so a
reference splitting Include and Version across lines could hide a stale
pin while one-line examples kept the positive control green. Now matches
whole file text with Singleline.
- Wildcard pins were silently exempt: ParsePin returned null for anything
containing '*', so a literal 2.1.* below the 2.2.0 floor passed. New
ComparePin compares the wildcard's literal prefix component-wise. This
immediately caught a real 1.7.* pin in docs/research, which is dated
investigation material and is now excluded alongside CHANGELOG and
docs/specs, with the rationale recorded.
- The positive control could satisfy itself from a prose placeholder in
this file's own XML docs. It now requires a parseable NUMERIC version, so
a doc comment cannot stand in for a real pin.
- .dt excluded from the scan, though docs-pipeline sources are *.md.dt and
a pin there reaches users via the compiled guide. Added.
Both new arms are mutation-verified: injecting a wildcard 2.1.* and a
multiline 2.0.1 each reddens the guard with the offending file:line.
ETW: the custom store label feeds write, read and rejection events, but
only write was asserted, so a regression emitting json-file on either of
the others would have passed. Read and oversize-rejection now covered.
Counts: the inventory said 21 files. It is 22 literal occurrences across 20
files, of which 18 are shipped agent-kit recipes (SystemPrompt.txt holds 3
occurrences in one file and is not packed). Corrected in spec 063, spec 059
and CHANGELOG; measured rather than re-counted by hand.
Spec 059's deferred-work row still attributed the #6559 fix to 2.5.1 after
spec 063 §3.2 corrected it to metapackage 2.3.1 / Foundation 2.3.5. Fixed.
Concurrency: UnpackagedAppDataStore's public doc now states the
single-process limitation outright, including why this identity makes the
collision MORE reachable than for JsonFileStore -- that store's process-name
key means two differently-named executables never share a file, whereas two
executables sharing one publisher/product do.
Discoverability: the windowing guide template and the shipped windowing
skill now show the opt-in, explain the process-name rename hazard it
solves, and say one instance per publisher/product. Guide regenerated via
mur docs compile (21 insertions, no snippet churn).
Reactor.Tests 14011/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This updates only the plugin-format windowing skill, but the shipped loose skills/windowing.md still ends its persistence section without documenting UnpackagedAppDataStore. src/Reactor/Reactor.csproj packs both the loose skills and this plugin skill, so consumers using the loose agent-kit entry will miss the new public API; mirror the opt-in guidance there as well.
Opt-in rather than default: the two stores key data differently, so switching
does not migrate existing layouts. One instance per publisher/product — the
backing file is shared and concurrent writers can drop each other's entries.
This creates a new JsonFileStore with a private _ioLock for every UnpackagedAppDataStore. Instances in separate processes (or two instances in one process) still write the same reactor-windows.json and .tmp, so concurrent read-merge-write operations can silently discard each other's persistence IDs. That violates the cross-process sharing contract documented by IWindowPersistenceStore and is now especially reachable because this API gives different executables a shared publisher/product root; add path/process-level synchronization before exposing this as a shared store. tests/Reactor.Tests/UnpackagedAppDataStoreTests.cs:55
The positive-control write is intentionally expected to move to HKCU\SOFTWARE\Classes\Local Settings\Software\<publisher> once issue #6559 is fixed, but cleanup only deletes the roaming HKCU\SOFTWARE\<publisher> root. The follow-up test will therefore leave a registry key behind when it correctly trips on a fixed runtime; delete both possible roots (or the specific control product) in cleanup. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:688
ParsePin deliberately discards every suffix after the numeric core, so a literal such as 2.2.0-preview.1 compares equal to the stable central 2.2.0. NuGet orders that prerelease below the stable floor, meaning the new guard would still allow a real downgrade; compare prerelease semantics or reject such pins as below/ambiguous instead of stripping the suffix. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:532
The new sweep only recognizes double-quoted Version= attributes. A legal central-package override such as <PackageReference Include='Microsoft.WindowsAppSDK' VersionOverride='2.0.1' /> is invisible, even though the repository's CPM contract explicitly permits VersionOverride (Directory.Packages.props:8-11). The guard can therefore pass with a stale literal SDK pin; parse project files with the existing XDocument path or cover both quote styles and VersionOverride. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:535
This optional suffix makes the sweep treat Microsoft.WindowsAppSDK.WinUI and .Runtime pins as if they were metapackage pins, but the repository deliberately keeps WindowsAppSDKWinUIVersion independent from WindowsAppSDKVersion (Directory.Build.props:30-36; Directory.Packages.props:28-30). A valid literal WinUI pin can therefore be reported stale or a pin below the WinUI floor can pass; restrict this pattern to the metapackage or compare each package name with its corresponding central property.
…ad of
guessing attribute order
Concurrency (raised twice by review; now fixed rather than documented).
JsonFileStore's read-merge-write was guarded only by a per-instance lock, so
two writers could each read the pre-merge document and the second rename
discarded the first's entry -- losing data even when the two wrote DIFFERENT
ids. Measured: 8 concurrent writers, 1 id survived.
New internal CrossProcessWriteGuard takes a Local\-scoped named mutex keyed
on a hash of the store path, so exclusion spans instances and processes. The
staging temp file is now per process+thread, so a writer that dies mid-write
cannot leave a shared .tmp that another process renames over the real file.
Acquisition is bounded (5s) and degrades to writing unguarded rather than
hanging the UI thread on close, which is when placement is saved; an
abandoned mutex is acquired, not failed, because the rename is atomic so the
protected file is never torn. Reads need no guard for the same reason.
JsonFileStoreConcurrencyTests asserts a COUNT of surviving ids, not a round
trip -- a round trip passes whenever the last writer wins, which is exactly
the broken behaviour. Mutation-verified: removing the guard yields 1/8
survivors and reddens the test. This fixes the DEFAULT store, so it also
benefits every existing unpackaged app, not just the new one.
Pin guard: replaced the attribute-order-dependent regex with element
matching plus separate attribute extraction, which fixes four holes at once
-- reversed attribute order (Version before Include), single-quoted
attributes, VersionOverride (which CPM permits), and sub-package pins being
compared against the wrong floor. Each package now compares against its own
central property, so Microsoft.WindowsAppSDK.WinUI is measured against
WindowsAppSDKWinUIVersion (2.2.1) rather than WindowsAppSDKVersion (2.2.0);
the old code could both report a current WinUI pin stale and pass a stale
one. Prerelease ordering added: 2.2.0-preview.1 now sorts below stable
2.2.0 as NuGet orders it, instead of comparing equal after suffix
stripping. All five cases mutation-verified.
Docs: the opt-in samples lacked their namespace, so copying them verbatim
did not compile. Added the using to the guide template, the plugin skill,
and the loose skills/windowing.md, which the previous round missed entirely
-- Reactor.csproj packs both skill formats, so agent-kit consumers reading
the loose entry would not have found the API. Guide regenerated.
Test cleanup now removes both the roaming and machine-local registry roots:
the roaming guard's positive control is designed to trip onto the
machine-local key once the runtime carries the #6559 fix, and would
otherwise leak a key on every run of the test that detects it.
Reactor.Tests 14013/0; Release build matches the pre-change baseline; API
index unchanged (the guard is internal).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved critical and moderate findings remain in persistence I/O, SDK pin validation, concurrency tests, and related documentation.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
CHANGELOG.md:92
The PR also changes the existing default JsonFileStore semantics to prevent cross-instance/process read-merge-write data loss, but the Unreleased changelog only describes the new opt-in store and SDK/pin changes. Add a Fixed or Changed entry for this persistence reliability fix (spec 063 §5), since it affects existing apps as well as the new shared publisher/product store.
- **Stale `Microsoft.WindowsAppSDK` version pins in shipped agent-kit recipes and docs
(spec 063 §3.0).** **22 literal occurrences across 20 files** used a
`#:package Microsoft.WindowsAppSDK@2.0.1` file-based-app header, which the 2.1.3 bump
(spec 059) missed entirely because that shape does not match the
`Microsoft.WindowsAppSDK" Version=` grep that spec prescribed. They had been advertising a
version *below* the framework's own floor — an `NU1605` downgrade for anyone running them — and
the **18 recipe files** under `skills/recipes/` and
`plugins/reactor/skills/reactor-recipes/references/` are packed into the shipped NuGet agent
kit, so this was user-facing rather than internal-only. All now track the pinned version, and a
new guard (`WinAppSDKReferenceGuardTests.No_literal_SDK_pin_sits_below_the_central_pinned_version`)
sweeps the tree for both pin shapes so the class cannot recur.
The new guard contains bespoke ordering branches for wildcard and prerelease pins, but the repository has no scanned #:package/PackageReference cases that exercise 2.1.* or 2.2.0-preview.1; the current-tree fact only covers stable and variable pins. A regression in these branches would therefore pass while the test still claims to enforce the pin policy. Add direct theory cases (or synthetic scan fixtures) for wildcard-prefix and prerelease-vs-stable comparisons. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:728
The prerelease arm only distinguishes stable versus prerelease; it does not compare prerelease identifiers. If the central floor is 2.2.0-preview.2, a literal 2.2.0-preview.1 is parsed with the same numeric core and returns Cmp == 0, so this guard misses a real NuGet downgrade. Compare the full SemVer prerelease sequence (or explicitly reject prerelease central floors) rather than treating all prereleases as equal. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:546
PackageElement stops at the opening > and PackageVersionAttr only reads attributes, so a legal MSBuild form such as <PackageReference Include="Microsoft.WindowsAppSDK"><Version>2.0.1</Version></PackageReference> is ignored. The guard's claim to cover PackageReference/PackageVersion pins is therefore incomplete; parse child Version/VersionOverride metadata (or add an equivalent whole-element matcher) and a positive regression case.
…in guard
Round-3 review. Three classes: a real I/O defect my own concurrency fix
missed, remaining gaps in the pin guard, and docs left contradicting the
fixed behaviour.
Reader/writer collision. Review pointed out that TryRead opened with
FileShare.Read, which does not grant delete sharing, so a reader could block
the commit and the write would fail silently. I added FileShare.Delete and
then MEASURED it: File.Move(overwrite:true) still fails --
UnauthorizedAccessException -- even when the reader grants delete sharing.
The share flag alone does not fix it. File.Replace does, and only when the
reader grants delete sharing (control: a Read-only reader makes Replace fail
with IOException). Both halves are now in place, with the measurement
recorded where the claim is made. New regression test holds a reader open
across a write; it failed against the FileShare-only fix, which is how the
insufficiency surfaced.
Pin guard: three remaining holes, each now covered by direct theory cases
because the current tree exercises only stable and variable pins, leaving
the wildcard and prerelease branches unreachable from the sweep.
- Child-element metadata (<Version>2.0.1</Version>) was ignored; MSBuild
accepts metadata as attribute or child element.
- A prerelease float (2.2.1-*) resolves only prereleases, which NuGet orders
below the stable floor, but matched on numeric prefix and passed.
- Prerelease labels compared only stable-vs-prerelease, so against a
prerelease floor 2.2.0-preview.1 tied with 2.2.0-preview.2. Now compares
identifier sequences per SemVer 11 (numeric below alphanumeric, shorter
prefix below its extension).
Guard-name test was vacuous: Acquire returns a non-null guard even when the
wait times out, so two paths colliding on one mutex would still have passed
after a 5s block. Added an internal IsHeld and an elapsed-time bound, so the
test now proves acquisition rather than inferring it.
Docs: the template, both windowing skills and spec 063 still described the
old broken concurrency and told callers to use one instance per
publisher/product -- guidance this PR makes obsolete. Updated and guide
regenerated. Added a CHANGELOG Fixed entry, since the persistence repair
lands in the DEFAULT store and therefore reaches every existing unpackaged
app, not only users of the new one.
Note for the record: an earlier run of this suite reported the concurrency
test failing 1/8. That was a stale binary from the preceding mutation run
(dotnet test --no-build against an unrebuilt tree), and my check for it was
itself broken -- the mutation marker was a // comment, which never reaches
the DLL, so the scan could not have detected it either way. Rebuilt; 14031/0.
Reactor.Tests 14031/0; Release build matches the pre-change baseline; API
index unchanged.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (7)
CHANGELOG.md:70
Reactor is a framework-dependent library and Directory.Build.targets injects Microsoft.WindowsAppSDK.WinUI; the bumped central value is WindowsAppSDKWinUIVersion = 2.2.1. Calling this a Microsoft.WindowsAppSDK >= 2.2.0 floor can make consumers infer the wrong transitive package; name the actual WinUI sub-package here and reserve the full metapackage wording for self-contained/MSIX projects.
transitive floor for `Microsoft.UI.Reactor` consumers rises to `Microsoft.WindowsAppSDK >= 2.2.0`.
docs/specs/059-titlebar-drag-regions.md:171
The earlier §1 survey row at line 47 still records ApplicationData.GetForUnpackaged() as Defer (§5), while this updated §5 row records it as adopted by spec 063. Leaving both statuses in the same spec makes the adoption state contradictory; update the original survey row as part of this amendment.
| `ApplicationData.GetForUnpackaged()` (2.2.0) | Could replace bespoke file-path persistence for dock-layout / window-placement in unpackaged apps with a first-class `ApplicationData`. Worth a focused look, but orthogonal to this spec and gated on a 2.2.0 bump. <br><br>✅ **Adopted by [spec 063](063-unpackaged-app-data.md)** (2026-09-16), which bumps the SDK to 2.2.0 and adds `UnpackagedAppDataStore`. Note it adopts the `LocalPath` surface, *not* `LocalSettings` — the latter writes to a roaming registry hive on 2.2.0 (measured; WindowsAppSDK#6559). The fix lands at metapackage **2.3.1** / `Foundation` 2.3.5, **not** 2.5.1 as that issue's release notes claim; see [spec 063 §3.2](063-unpackaged-app-data.md) for the two-arm measurement and for why the boundary attaches to a `Foundation` version rather than a metapackage one. |
When the 5-second wait expires, this returns an unheld guard and JsonFileStore.Write still performs its read-merge-write. If the peer is merely slow or suspended, both writers can read the same old document and the later rename drops the earlier id—the exact lost-update bug this guard is intended to prevent. Please retry or abort the write when the guard is not held instead of proceeding concurrently, or narrow the cross-process-safety guarantee. tests/Reactor.Tests/JsonFileStoreConcurrencyTests.cs:51
This exercises eight threads in one process, so a per-path static SemaphoreSlim (or other process-local lock) could pass while separate processes still race and lose entries. The production change and IWindowPersistenceStore contract specifically promise cross-process safety; add a two-process writer test that shares the same file and verifies all ids survive, rather than relying only on this same-process proxy. tests/Reactor.Tests/JsonFileStoreConcurrencyTests.cs:126
This test opens the reader with FileShare.Read | FileShare.Delete directly, so it never exercises JsonFileStore.TryRead or ReadDocumentOrEmpty's new share mode. Reverting the production lines to FileShare.Read would still pass, leaving the change that prevents a real second-process reader from blocking File.Replace unguarded. Hold a stream opened by the production read path (or add a test seam) and perform the write while it remains open. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:717
The wildcard comparison stops once the specified prefix matches, so it treats 2.2.* as safe against a future central floor such as 2.2.1. NuGet can resolve that range to 2.2.0, which is below the floor, and the guard would silently allow the downgrade it is meant to catch. Compare the lowest version permitted by the wildcard with the full central version (or reject underspecified floating pins), and add a patch-level regression case. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:718
A valid NuGet floating prerelease pin such as 2.2.0-preview.* reaches this branch with a prefix like 2.2.0-preview; int.TryParse(pinParts[i]) then fails and returns 0, so the guard treats it as acceptable even though it can only resolve below a stable 2.2.0 floor. Parse and compare the numeric core and prerelease label for this wildcard form, and add a regression case.
…e it
Round-4 review. The concurrency fix from round 3 had two holes of its own,
both correctly identified.
Session scoping. A mutex name carries a scope. Local\ is per logon session,
so the same user in two sessions -- concurrent RDP, or a service alongside a
desktop logon -- took DIFFERENT mutexes for the same %LOCALAPPDATA% file and
raced anyway. Global\ spans sessions but needs SeCreateGlobalPrivilege,
which a standard-user process may not hold. Replaced with a lock file beside
the store (FileShare.None + DeleteOnClose): it inherits the scope of the
thing it protects, so any principal that can write the store can take it,
across sessions, with no privilege, and a killed holder releases it when the
OS closes the handle.
Timeout semantics. On timeout the old code proceeded to write unguarded,
which reintroduced exactly the lost update the guard exists to prevent: the
blocked writer merges a stale document and its commit drops the peer's
entry. It now skips the write and emits PersistenceRejected. Best-effort
already permits losing this window's placement; it does not permit
corrupting another window's.
Both concurrency tests were weaker than they looked, also as flagged:
- The same-process test would pass for a process-local lock, so it could not
establish the cross-process claim. Added a test that takes the guard's
underlying kernel object from OUTSIDE the guard, exactly as a second
process would, and asserts both that the guard reports no ownership and
that the store declines to write.
- The reader/writer test opened its own FileStream with hand-picked flags,
so reverting production to FileShare.Read would still have passed. Both
read sites now route through an internal OpenForRead seam and the test
holds a stream opened by the production path.
Pin guard, two more wildcard cases: a floating range resolves to the LOWEST
version it permits, so 2.2.* is a downgrade against a 2.2.1 floor even
though the prefix matches; and a floating prerelease (2.2.0-preview.*) hit
int.TryParse on "preview" and returned "acceptable". Both now compare the
zero-filled lowest permitted version plus the prerelease label, with
regression cases.
Docs: CHANGELOG named the wrong transitive package -- Reactor is
framework-dependent and flows Microsoft.WindowsAppSDK.WinUI 2.2.1, not the
2.2.0 metapackage, which only self-contained/MSIX consumers reference.
Spec 059's original survey row still read "Defer", contradicting the
adoption note this PR added to the same spec's section 5.
Reactor.Tests 14035/0; Release build matches the pre-change baseline; API
index and generated guide both clean.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
SavePlacement is UI-thread-only (src/Reactor/Hosting/ReactorWindow.cs:2634-2643) and calls store.Write synchronously (:3534-3543). When another process holds this lock, Acquire sleeps for up to 5 seconds before returning, so a stalled peer can freeze window close/manual saves for a user-visible interval and then silently discard this placement. Please avoid a multi-second blocking wait on the UI thread (for example, use a much shorter/non-blocking acquisition or move the persistence write off-thread while preserving the merge guard). tests/Reactor.Tests/Diagnostics/PersistenceEtwBridgeTests.cs:177
The listener receives process-global events, and these new assertions discriminate only on the shared storeKind value. A concurrent UnpackagedAppDataStoreTests writer/reader can therefore satisfy them even if this operation emits nothing, so the mutation is not guaranteed to redden. Isolate all persistence ETW tests from same-kind writers or add a non-PII per-test correlation before asserting these events. tests/Reactor.Tests/Diagnostics/PersistenceEtwBridgeTests.cs:209
This rejection assertion has the same process-global false-positive: it matches any unpackaged-appdata rejection, not the oversize rejection caused by this test. A concurrent test could emit the same store kind and let a mutation of this write path pass; isolate the listener or assert a deterministic per-test discriminator. tests/Reactor.Tests/JsonFileStoreConcurrencyTests.cs:105
This assertion is load-sensitive rather than a lock oracle: a busy CI runner can take over one second to acquire an unrelated lock even when g2.IsHeld proves the paths do not collide. Remove the elapsed-time requirement and rely on the ownership result (or use a deterministic blocked-acquisition probe) so this test does not introduce timing flakes. tests/Reactor.Tests/JsonFileStoreConcurrencyTests.cs:65
This ignores the Join result and immediately reads the file. If a writer stalls or fails to reach the barrier, the test can inspect an incomplete document, report a misleading survivor count, and leave foreground threads running into later tests. Assert that every writer joined (and report thread failures) before constructing the reader; the existing DataGridCommitThreadingTests pattern treats the join as the happens-before edge. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:503
This guard claims to cover literal SDK pins across the tree, but its only file filter omits tracked MSBuild project formats present in this repository, including DocApps.proj, .wapproj, and .vcxproj. A future PackageReference in one of those files would evade both regexes and the positive-control-backed check. Include the other project/XML extensions or enumerate project files by content rather than relying on this incomplete extension list. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:895
The exclusions below are applied only after Directory.EnumerateFiles(..., SearchOption.AllDirectories) has already traversed every file under .git, bin, obj, node_modules, and artifacts. This turns a one-test source scan into an unnecessary full-checkout traversal after every build, and can become very expensive when the repo contains generated outputs. Prune those directories during enumeration (or use an EnumerationOptions/directory-recursion helper) instead of filtering only the yielded file paths. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:740
The prerelease wildcard branch is not ordered as a wildcard range: with a central floor such as 2.2.0-preview.1, 2.2.0-preview.* produces preLabelPrefix = "preview", and ComparePrerelease ranks that shorter prefix below preview.1, so a pin that can resolve the floor is reported stale. Add a regression case for a prerelease floor and compare the wildcard's matching prefix/range rather than treating the prefix as a concrete SemVer.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Six unresolved findings remain across changelog wording, registry-test controls, and SDK-pin guard coverage.
Review details
Suppressed comments (6)
Previously missed (1) — in code that hasn't changed since the last review.
CHANGELOG.md:86
JsonFileStore implements IWindowPersistenceStore, which is used for window placement only; spec 063 §1 explicitly says dock layouts use a separate IPersistedStateScope and are untouched. This changelog entry therefore overstates the fix by claiming dock layouts are protected. Please limit the wording to window-placement layouts.
OpenSubKey returns a live RegistryKey, but is not null discards it without disposing it. This positive control intentionally finds the key, so each run leaks a registry handle and can make the later DeleteSubKeyTree cleanup fail, undermining the test's no-residue guarantee. Keep the key in a using declaration before checking it. tests/Reactor.Tests/UnpackagedAppDataStoreTests.cs:189
The positive control only checks that the HKCU\SOFTWARE\<publisher>\<product> key exists, not that the sentinel value written through LocalSettings.Values["control"] was persisted. A runtime/probe that creates the key but drops the value would still set controlSeen to true, allowing the test to treat the roaming-hive observation as valid and draw the follow-up conclusion. Read back the named registry value (or assert an API round-trip) so this control proves the write, not just key creation. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:559
PackageVersionAttr.Match returns the first Version/VersionOverride it encounters, so a legal item that has Version="2.2.0" followed by a stale VersionOverride="2.0.1" is recorded as current even though VersionOverride is the effective CPM override (the same issue applies to child metadata). That leaves the new guard able to pass a real downgrade despite claiming to cover VersionOverride; parse the two forms separately and apply the effective override precedence, or reject ambiguous duplicates. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:510
The scanner's PackageElement regex is documented as covering XML, but .xml is absent from PinScanExtensions. A future XML snippet containing a literal PackageReference or PackageVersion can therefore carry a below-floor SDK pin without this guard reading the file, so the claimed tree-wide coverage does not hold. Include the XML extension (or otherwise align the scanned formats with the regex). tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:559
The scanner claims to cover VersionOverride attributes and child <Version> metadata, but no test exercises either form (the live tree only supplies ordinary PackageReference/PackageVersion attributes). The positive-control check would still pass if either regex branch regressed, because it is satisfied by the other ordinary references, so a future stale pin using one of these legal forms could be silently exempted. Add synthetic scan cases for reverse attribute order, VersionOverride, and child metadata and assert that a below-floor version is reported.
…anches
Round-12 review: 0 new inline comments, 6 suppressed.
VersionOverride precedence was wrong. A single alternation matched whichever
of Version / VersionOverride appeared first, so an item carrying a current
Version followed by a stale VersionOverride was recorded as current while
restore would use the stale value -- the guard could pass a real downgrade
in the very form it claimed to cover. The two are now matched separately
with the override taking precedence, in both attribute and child-element
form.
Those branches were also entirely untested: the live tree only contains
ordinary single-line Include+Version attributes, so reverse ordering, single
quotes, VersionOverride and child metadata were unreachable from the sweep
and the positive control stayed green from the ordinary references. Pulled
the element parsing into an internal ExtractPin and added nine direct cases.
Their fixtures deliberately use an above-floor version: the sweep reads this
file too, and below-floor fixtures made the guard flag its own test data --
which it did on the first run, correctly.
.xml was missing from the scanned extensions even though the element regex
is documented as covering XML, so an XML snippet carrying a below-floor pin
would not have been read at all.
Registry positive control strengthened: it asserted only that the key
existed, so a runtime that created the key but dropped the value would have
satisfied it and let the roaming-hive conclusion rest on nothing. It now
reads the sentinel value back, and disposes the RegistryKey rather than
discarding a live handle that could block the cleanup this suite promises.
CHANGELOG still claimed the concurrency fix protects dock layouts. It does
not -- docking uses IPersistedStateScope, as the previous commit established
for the other entry.
Reactor.Tests 14065/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Three moderate findings and one nit remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
Directory.Build.props:24
The central floor is now 2.2.0, but the current contributor prerequisite still says “Windows App SDK 2.0” in CONTRIBUTING.md:26. That can lead contributors to keep a 2.0 runtime installed even though the repo's framework-dependent apps now require the 2.2 servicing floor; update the prerequisite to the new minimum (or explicitly say >= 2.2.0).
The PR risk section says there is “no behavior change by default,” but this block runs for the default JsonFileStore used by every unpackaged app: writes now create a .lock sidecar, wait up to 250 ms, and can abandon a placement save when the lock is unavailable. Please correct the PR description/risk statement (the spec itself says this fixes the default store) so consumers are not told this change is opt-in. tests/Reactor.Tests/WinAppSDKReferenceGuardTests.cs:865
An unparseable matched pin is silently treated as valid here. That means a real PackageReference, #:package, or $ver value such as a malformed literal can bypass the guard instead of failing, even though restore cannot use it. Report or fail on unparseable discovered versions rather than returning the non-stale result used for intentionally variable pins.
…ble pins
Round-13 review: 1 comment, 3 suppressed.
The PackageReference positive control could be satisfied by this test class's
own InlineData fixtures, so a scanner that stopped recognising real project
and docs XML while still matching its fixtures would leave `found` non-empty
and pass -- the same self-satisfying-control defect fixed in round 8 for the
script shape, which I reintroduced in round 12 when adding the extraction
theory. All three controls now require a match from a production file.
Mutation-verified: breaking PackageElement so it matches only the fixture
shape now reddens the control.
Unparseable pins were silently treated as valid, so a malformed literal that
restore could not use would bypass the guard entirely. They now fail with
the offending file:line. That immediately surfaced two C# interpolation
holes in the widget-creator scaffold, which are generated from the same
central property and are therefore variable pins, not literals -- taught to
ComparePin alongside $(...), with a case.
CONTRIBUTING.md still listed Windows App SDK 2.0 as the prerequisite, two
floors behind.
Also corrected the PR description, which claimed no default behaviour
change. That was wrong: the concurrency fix lands in JsonFileStore, the
auto-detected default for every unpackaged app, so writes now take a lock
file, wait up to 250 ms, and abandon the save if it cannot be acquired. The
risk section now leads with that.
Reactor.Tests 14066/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Round-14 review: 1 finding.
ParsePin matched a leading numeric prefix, so "2.2.0not-a-version" parsed as
2.2.0 -- it compared as current AND bypassed the unparseable check added last
round, defeating both. Now anchored to NuGet's grammar: one to four numeric
components, optional SemVer prerelease, optional build metadata, whole
string or nothing.
PrereleaseLabel had the same prefix weakness and could read a label off a
literal ParsePin rejected, so the two would disagree about what parsed. Both
now share the anchored pattern.
Eleven cases pin the boundary in both directions, including the reported
2.2.0not-a-version plus v-prefixed, over-long and empty-label forms.
Reactor.Tests 14077/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CI: Docs build failed on InlineSnippetLedgerTests -- the opt-in example I
added to windows.md.dt was typed straight into the template, so nothing
compiled it and the docs-snippet-gate could not see it, even though it reads
as code to copy.
Moved it into docs/_pipeline/apps/windows/App.cs behind
<snippet:unpackaged-store> and pointed the template at it, per the repo
convention that doc apps are the source of every guide snippet. The
assignment sits in a method the demo never calls, because switching the
process-wide store would repoint the doc app's own saved placement.
Took the compile path rather than the AllowedInlineExamples ledger: the
ledger is for blocks that genuinely cannot be compiled, and this one
compiles.
The shipped skills keep their inline blocks -- the gate scans only
docs/_pipeline/templates, and hand-written code is the norm in agent
guidance.
Doc app builds clean; Reactor.DocPipeline.Tests 439/0; Reactor.Tests
14077/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The malformed-version filter treats any value beginning with [ or ( as a valid NuGet range without validating its shape. A pin such as [2.2.0 or [not-a-version] is therefore excluded from unparseable; ComparePin either skips the range arm or gets a null lower bound and returns 0, so the guard passes an invalid version instead of flagging it. Validate the interval syntax before exempting it and add malformed-range cases.
…heck
Round-16 review: 0 new findings, 1 previously-missed.
The malformed-version filter exempted anything starting with [ or ( as "a
range" without checking its shape, so "[2.2.0" (unterminated) and
"[not-a-version]" escaped the unparseable check and then compared as 0 --
an invalid literal passing the guard that exists to catch invalid literals.
New IsWellFormedRange validates the brackets and every bound it specifies,
and both ComparePin and the malformed-pin filter now use it, so the two
agree on what counts as a range. Ten cases assert well-formed AND malformed
inputs, so the predicate cannot pass by rejecting everything.
Reactor.Tests 14087/0; Release build matches the pre-change baseline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…loor to 2.2.0
Updates the four Reactor dotnet-new templates for the 0.1.0-preview.16 release:
reactorVersion 0.1.0-preview.14 -> 0.1.0-preview.16
windowsAppSdkVersion documented floor 2.1.3 -> 2.2.0
microsoft/microsoft-ui-reactor#1259 adopts ApplicationData.GetForUnpackaged(),
a Windows App SDK 2.2.0 API, and raises Microsoft.UI.Reactor's dependency on
Microsoft.WindowsAppSDK.WinUI to 2.2.1. Verified that commit is contained in
the v0.1.0-preview.16 tag, and that Directory.Build.props at that tag declares
WindowsAppSDKVersion 2.2.0 / WindowsAppSDKWinUIVersion 2.2.1.
These templates reference the full Microsoft.WindowsAppSDK metapackage (not the
lean WinUI sub-package) and scaffold packaged single-project MSIX apps, so the
correct consumer floor is metapackage 2.2.0 - the first version whose WinUI
dependency is 2.2.1 (2.1.3 -> WinUI 2.1.0; 2.2.0 -> WinUI 2.2.1).
Both edits ship together because the mismatch they prevent is silent, not a
restore error. Measured with a stub package carrying the post-#1259 dependency
shape: pinning --wasdk-version 2.1.3 restores with exit code 0 and zero
warnings, resolving Microsoft.WindowsAppSDK.WinUI 2.2.1 while the metapackage
holds Microsoft.WindowsAppSDK.Runtime at exactly [2.1.3]. WinUI is only ever a
transitive reference, so NuGet takes the highest floor and never reports
NU1605; the result is 2.2.1 reference assemblies over a 2.1.3 runtime, which
builds clean and fails at runtime. The same probe at 2.2.0 resolves Runtime
2.2.0. Since the toolchain raises no error, the description text is the only
guardrail a user pinning explicitly will ever see.
Validated by scaffolding all four templates from a locally packed template
pack: each emits Microsoft.UI.Reactor and Microsoft.UI.Reactor.Devtools at
0.1.0-preview.16, Microsoft.WindowsAppSDK at the default '*', and a
Package.appxmanifest. Also verified the explicit pin path
(--wasdk-version 2.2.0 --reactor-version 0.1.0-preview.16).
Restore/build validation is deferred to CI: 0.1.0-preview.16 is on nuget.org
but has not yet mirrored to the internal package proxy reachable from this
machine.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…loor to 2.2.0 (#6786)
Updates the four Reactor dotnet-new templates for the 0.1.0-preview.16 release:
reactorVersion 0.1.0-preview.14 -> 0.1.0-preview.16
windowsAppSdkVersion documented floor 2.1.3 -> 2.2.0
microsoft/microsoft-ui-reactor#1259 adopts ApplicationData.GetForUnpackaged(),
a Windows App SDK 2.2.0 API, and raises Microsoft.UI.Reactor's dependency on
Microsoft.WindowsAppSDK.WinUI to 2.2.1. Verified that commit is contained in
the v0.1.0-preview.16 tag, and that Directory.Build.props at that tag declares
WindowsAppSDKVersion 2.2.0 / WindowsAppSDKWinUIVersion 2.2.1.
These templates reference the full Microsoft.WindowsAppSDK metapackage (not the
lean WinUI sub-package) and scaffold packaged single-project MSIX apps, so the
correct consumer floor is metapackage 2.2.0 - the first version whose WinUI
dependency is 2.2.1 (2.1.3 -> WinUI 2.1.0; 2.2.0 -> WinUI 2.2.1).
Both edits ship together because the mismatch they prevent is silent, not a
restore error. Measured with a stub package carrying the post-#1259 dependency
shape: pinning --wasdk-version 2.1.3 restores with exit code 0 and zero
warnings, resolving Microsoft.WindowsAppSDK.WinUI 2.2.1 while the metapackage
holds Microsoft.WindowsAppSDK.Runtime at exactly [2.1.3]. WinUI is only ever a
transitive reference, so NuGet takes the highest floor and never reports
NU1605; the result is 2.2.1 reference assemblies over a 2.1.3 runtime, which
builds clean and fails at runtime. The same probe at 2.2.0 resolves Runtime
2.2.0. Since the toolchain raises no error, the description text is the only
guardrail a user pinning explicitly will ever see.
Validated by scaffolding all four templates from a locally packed template
pack: each emits Microsoft.UI.Reactor and Microsoft.UI.Reactor.Devtools at
0.1.0-preview.16, Microsoft.WindowsAppSDK at the default '*', and a
Package.appxmanifest. Also verified the explicit pin path
(--wasdk-version 2.2.0 --reactor-version 0.1.0-preview.16).
Restore/build validation is deferred to CI: 0.1.0-preview.16 is on nuget.org
but has not yet mirrored to the internal package proxy reachable from this
machine.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Remove github-artifacts mcp server (#6684)
* Remove github-artifacts mcp server
* Update review prompts for GitHub MCP migration (#6686)
Remove references to the retired github-artifacts helper and direct issue and PR review workflows to Copilot's standard web, file, and execution tools.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2b34993b-0dcf-4789-94c5-97fd92cce84d
---------
Co-authored-by: Gordon Lam <73506701+yeelam-gordon@users.noreply.github.com>
Copilot-Session: 2b34993b-0dcf-4789-94c5-97fd92cce84d
* MSIX: Always include PackageIntegrity/Content[Enforcement=on] in VS project templates (#6613)
* Bump VSIX build .NET SDK to 10.0.111 (security release) (#6699)
* Pin .NET SDK to patched 10.0.111 (CG DOTNET-Security-10.0)
Component Governance flags a vulnerable .NET SDK (10.0.101) against
/global.json. That version string is not in the repo -- global.json has
no "sdk" block at all, so builds resolve whatever SDK the agent image
happens to carry, and CG attributes what it detects to the nearest
manifest. Note "tools.dotnet" is a non-standard bootstrap field and does
not govern SDK resolution; only an "sdk" block does.
Pins sdk.version to 10.0.111 (runtime 10.0.11, 2026-08-11 security
release). Staying in the 1xx feature band already used here avoids
unrelated MSBuild/SDK/analyzer behavior changes.
Also bumps BuildVSIX 10.0.110 -> 10.0.111 so an SDK satisfying the pin is
actually installed -- rollForward only rolls forward, never backward.
That is independently a security improvement: 10.0.110 carries runtime
10.0.10 and is one security release behind. Follows up #6624.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5474160f-e555-4254-899b-783ad9211b71
* Remove root global.json SDK pin; scope PR to VSIX only
The root global.json `sdk` block is a repo-wide floor for every dotnet/MSBuild
invocation beneath it. Foundation/MRT/PREfast jobs install only .NET SDK 6.0.414
(matched to the shipping net6.0 / net6.0-windows10.0.19041.0 TFMs), and
rollForward never rolls backward across majors -- so Microsoft.NET.Sdk became
unresolvable and 10 CI jobs failed with MSB4236.
Scoping this PR to the BuildVSIX 10.0.110 -> 10.0.111 bump only. The CG
global.json remediation requires installing the pinned SDK in every job that
restores under the root global.json, and will be handled separately.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 484c9277-ea15-4f85-a4a0-0429347ade48
---------
Co-authored-by: Vineeth Thomas Alex <vithoma@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5474160f-e555-4254-899b-783ad9211b71
Copilot-Session: 484c9277-ea15-4f85-a4a0-0429347ade48
* [main] Update dependencies from microsoft/LiftedIXP/DCPP (#6636)
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260717.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1897.260717-1605.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260717.1
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260719.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1898.260719-1733.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260719.1
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260723.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1899.260723-1213.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260723.1
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260727.2
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1900.260726-2015.1
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260727.2
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260729.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1901.260728-1900.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260729.1
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260806.3
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1902.260806-0506.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260806.3
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260814.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1903.260814-0918.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260814.1
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260818.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1904.260818-1237.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260818.1
* Update dependencies from https://dev.azure.com/microsoft/LiftedIXP/_git/DCPP build 20260823.1
On relative base path root
Microsoft.FrameworkUdk , Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage From Version 3.0.0-experimental-27300.1896.260713-1015.0 -> To Version 3.0.0-experimental-27300.1905.260822-1925.0
Microsoft.WindowsAppSDK.InteractiveExperiences From Version 3.0.0-experimental-Rolling.20260713.2 -> To Version 3.0.0-experimental-Rolling.20260823.1
---------
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* [57514396] Use the central TSA area path in Foundation native builds
* Fix "Cancel" button in the C# and C++ wapproj templates so that it returns to the "Configure your new project" menu instead of creating a broken project (#6708)
* Add nuget cli alternative to Test-DotnetnNewTemplates for local execution
* Add check in Build-VSIX-Local to verify that the templates are included in the VSIX
* Add MsBuild parameter to build and install VSIX scripts
* Set WindowsAppSDkVersion, WindowsSdkBuildToolsVersion,and WindowsSdkBuildToolsWinAppVersion to {name}$ (parent template parameter) instead of project template parameter
* Move the Windows platform selection wizard from the Wapproj.vstemplate to the PackagedApp.vstemplate
* Move the Windows platform selection wizard from the C++ Wapproj.vstemplate to the PackagedApp.vstemplate
* Substitute publisher name from parent project in C++ wapproj Package.appxmanifest
* Rename VsVersion in help guide to MsBuildVersion
* Update missing MSBuild error message to include the MsBuildVersion selected in the script
* Remove duplicate problems.Count newline
* Remove extra excludedVersionPrefixes in Test-DotnetNewTemplates.ps1
* Add Reactor (Microsoft.UI.Reactor) dotnet new templates (#6620)
* Add Reactor (Microsoft.UI.Reactor) dotnet new templates
Adds four Reactor project templates to the Microsoft.WindowsAppSDK.WinUI.CSharp.Templates dotnet-new pack, mirroring the WinUI templates but for Microsoft.UI.Reactor (pure C#, no XAML):
- Reactor Blank App (reactor, reactor-blank)
- Reactor MVU App (reactor-mvu)
- Reactor NavigationView (reactor-navview)
- Reactor TabView App (reactor-tabview)
Wired into dev/Templates/templates.props via the DotnetNewOnlyProjectTemplate
slot (dotnet-new only, no VSIX, no manifest; Reactor apps run unpackaged).
Each shares a common csproj referencing Microsoft.UI.Reactor +
Microsoft.WindowsAppSDK, bundles the WinAppSDK runtime self-contained, and
enables the Debug-only Reactor devtools surface. README and
Test-DotnetNewTemplates.ps1 updated to document and validate the new templates.
Validated end-to-end: pack -> install -> dotnet new -> build (Debug + Release)
for all four templates.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Fix Reactor template runtime crash: remove TitleBar icon
TitleBar(...).Icon(FontIcon(...)) threw System.ArgumentException (E_INVALIDARG)
at runtime when Reactor set TitleBar.IconSource on Windows App SDK 2.1.3, so
the scaffolded Blank, MVU, and TabView apps crashed on launch. The glyph was
only a placeholder, so drop the icon; the title bar still shows the title text
and all four templates now launch cleanly (validated with dotnet run).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Bump Reactor templates to preview.12 and restore title-bar icon
microsoft/microsoft-ui-reactor#855 fixes the IconResolver FontSize=NaN crash
(FontIcon in an IconSource slot without an explicit size threw
ArgumentException from TitleBar.set_IconSource). That fix ships in
Microsoft.UI.Reactor 0.1.0-preview.12, so restore the placeholder title-bar
app-mark icon in the Blank, MVU, and TabView templates and bump the default
reactorVersion 0.1.0-preview.11 -> 0.1.0-preview.12 across all four templates.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Provision .NET 10 SDK in dotnet-new templates test stage
The template smoke-test stage installed only .NET SDK 8.0.x, so scaffolding
and building the Reactor templates failed with NETSDK1045 (Reactor targets
net10.0 and declares a [10.0,) sdk-version constraint). The WinUI templates
auto-detect their TFM from the active SDK, so they built net8.0 and passed,
masking the gap.
Add a second UseDotNet@2 step (param reactorDotnetSdkVersion, default 10.0.x)
to the shared Test-Stage template so the agent has the .NET 10 SDK. Verified
locally that the WinUI templates still build cleanly on the .NET 10 SDK, so
existing coverage is unaffected. Both the official and PR pipelines consume
this shared stage.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Address PR review: trim launchSettings comments, use per-template icons
Review feedback from @DinahK-2SO on #6620:
- Remove the explanatory comment block from the Reactor launchSettings.json
files (the content overlapped the package README).
- Give the MVU, NavigationView, and TabView templates their own icon.png
(reusing the existing WinUI mvvm/navigation/tabview template icons) instead
of all reusing the blank-app icon.
The packaging/taskbar-icon, tabs-in-title-bar, and winui-* short-name comments
are still open pending a check with the Reactor team.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add winui-reactor* short-name aliases to Reactor templates
Review feedback from @DinahK-2SO on #6620: give each Reactor template a
winui-reactor* alias (winui-reactor, winui-reactor-mvu, winui-reactor-navview,
winui-reactor-tabview) so they also surface under 'dotnet new list winui'
alongside the other WinUI templates. README updated to note the aliases.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* reactor-tabview: integrate tabs into the title bar
Review feedback from @DinahK-2SO on #6620: instead of stacking a separate
TitleBar row above the TabView, put the tabs up in the title bar (Edge/Terminal
style) to look cleaner and save vertical space.
Uses supported Reactor APIs: UseWindow().NativeWindow with
ExtendsContentIntoTitleBar + SetTitleBar, the TabView TabStripHeader (app name
at the leading edge) and TabStripFooter (draggable region), and UseDpi +
UseWindowSize to reserve the caption-button inset (RightInset) across DPI and
resize. Verified by scaffolding the template, building, running, and
screenshotting the window: the tabs render inline in the title bar with the
caption buttons clear.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Make Reactor templates packaged (MSIX) and match the WinUI templates
Per review direction, the four Reactor templates now:
1. Are single-project MSIX packaged (like the WinUI templates) instead of
unpackaged. Each gains app.manifest, Package.appxmanifest (EntryPoint
Windows.FullTrustApplication since Reactor has no XAML App class), the shared
Assets, and PublishProfiles. The csproj switches to EnableMsixTooling +
Microsoft.Windows.SDK.BuildTools + Microsoft.Windows.SDK.BuildTools.WinApp so
'dotnet run' registers a loose package and launches with MSIX identity.
templates.props sets RenameManifestFrom and the template.json files gain the
publisher / guid / build-tools-version symbols and the \\$
modifier.
2. Visually match their WinUI counterparts:
- reactor -> title bar (app icon) + empty content (blank canvas).
- reactor-mvu -> centered 'Hello, WinUI!' + [+]/[-] icon counter (keeps the
UseReducer MVU pattern under the same UI).
- reactor-navview -> title bar + NavigationView Home/About + built-in Settings;
pages render 'Title' + 'This is the X page'.
- reactor-tabview -> MicaAlt, app-icon TabStripHeader, closable Home/About tabs
with a '+' add button, tabs in the title bar.
Validated end-to-end for all four: pack -> install -> scaffold -> build ->
dotnet run (launches packaged with identity) -> screenshot matches the WinUI
counterpart. README updated (templates are now packaged).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* reactor-mvu: greet 'Hello, Reactor!'; use WinUI type-ramp styles for font parity
- MVU greeting is now 'Hello, Reactor!' (was 'Hello, WinUI!').
- Switch the Reactor text from Heading()/.SemiBold() (raw 28px Bold-700) to the
type-ramp factories Title()/BodyStrong()/Body(), which apply the actual WinUI
styles (TitleTextBlockStyle 28px Semibold-600, BodyStrongTextBlockStyle,
BodyTextBlockStyle). This makes the Reactor headings/body render with the same
weight and Segoe UI Variable optical size as the WinUI templates instead of a
heavier bold. Applied to the mvu, navview, and tabview page text.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* reactor-tabview: give tab pages a solid background to match WinUI
The WinUI TabView template's pages set Background=SolidBackgroundFillColorTertiaryBrush,
which covers the window's Mica in the tab content area. The Reactor version left
the pages transparent, so MicaAlt showed through the content. Wrap the tab page
content in a Border painted with that same tertiary brush so the content is a
solid surface and only the tab strip / title bar show Mica, matching X_Tab.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* reactor-tabview: stretch the tab page surface to fill the content area
The solid tertiary page background only covered the text at the top because
Reactor sizes the tab content to the element's desired size. Force the page
Border to HorizontalAlignment/VerticalAlignment Stretch so the solid surface
fills the whole tab content region, matching the WinUI TabView template
(verified: the content is a uniform solid fill down to the window bottom).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Fill reactor-tabview page surface to window height
Match the WinUI TabView template's uniform tertiary content background by
sizing the shared page surface to the window height, so it fills the whole
tab content area instead of shrinking to its text (two-tone with Mica below).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Use Reactor's navigation system in reactor-navview
Match the WinUI NavigationView template's shell: the title bar owns the back
and pane toggle buttons, and NavigationView's own copies are hidden.
Switch from hand-rolled tag state to Reactor's navigation handle
(UseNavigation + NavigationHost), so the title bar back button is wired by
.WithNavigation(nav) and tracks the back stack instead of never appearing.
Also request the tall title bar layout the WinUI template uses.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Toggle the nav pane on the control instead of mirrored state
NavigationView opens and closes its pane on its own (light dismiss, display
mode changes), so a separate IsPaneOpen state copy drifted out of sync and
the title bar toggle needed two clicks to take effect.
Flip IsPaneOpen on the control directly, the same way the WinUI template's
code-behind does.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Match the WinUI page transition in reactor-navview
Reactor's NavigationHost slides pages in from the side, while WinUI's Frame
defaults to an entrance transition that rises the new page up into place.
Slide from the bottom so switching pages animates the same way.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Trim the Reactor templates to match the XAML templates' style
The Reactor templates carried long comments explaining Reactor concepts and
how they differ from XAML, which made them read like documentation rather
than a starting point. Keep only the brief, functional comments the XAML
templates use, and a single pointer to the Reactor project info.
Also drop the explicit navigation transition from reactor-navview: matching
the XAML Frame's entrance animation added verbosity and called out a
difference between the two, so use Reactor's default instead.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Mark the Reactor templates as experimental
Microsoft.UI.Reactor is prerelease and its API may still change, so label the
templates accordingly: add `(Experimental)` to each template name, an
`Experimental` classification so it shows in the dotnet new tags column, and
a note in each description and in the README.
Also correct the templates.props comment, which still described the Reactor
apps as unpackaged with RenameManifestFrom unset; they are packaged
(single-project MSIX) and do set it.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Fix Reactor analyzer warnings in the MVU and TabView templates
The templates tripped four of Reactor's analyzers, so a freshly scaffolded
project did not build clean:
- REACTOR_A11Y_001 (x2): the MVU counter's icon-only buttons had no
accessible name. Add .AutomationName().
- REACTOR_A11Y_002: the TabView tab strip icon is decorative. Mark it
.AccessibilityHidden().
- REACTOR_HOOKS_013: the initial tab array was reallocated on every render
though only read on the first. Wrap it in UseMemo.
- REACTOR_POOL_001: the page surface set MinHeight via .Set(...), which is
reset when the element returns to the pool, so the tab content fill could
be lost on re-render. Use the .MinHeight() modifier instead.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Use $targetentrypoint$ in the Reactor manifests, like the XAML templates
The Reactor manifests hardcoded EntryPoint=Windows.FullTrustApplication while
every XAML template uses the $targetentrypoint$ token. The token already
resolves to Windows.FullTrustApplication for these projects, so the generated
AppxManifest.xml is unchanged - this just removes an unexplained difference
between the two sets of templates.
The four Reactor manifests are now byte-identical to their XAML counterparts.
Verified: all four scaffold, build warning-free, generate the same
EntryPoint=Windows.FullTrustApplication, and launch with package identity.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Commit the Reactor templates' publish profiles
The root .gitignore has a blanket `*.pubxml` rule, so the three publish
profiles in each Reactor template were silently never added. They existed
only on the authoring machine; a clean clone got none of them. The WinUI
templates force-added theirs in #6407 for the same reason.
Without the profiles the csproj's
`<PublishProfile Condition="Exists(...)">` never resolves, so SelfContained
is never set to true. Combined with PublishTrimmed=true for non-Debug
configurations that fails a Release build outright:
error NETSDK1102: Optimizing assemblies for size is not supported for the
selected publish configuration. Please ensure that you are publishing a
self-contained app.
Force-add all 12 files so Release builds and publish work from a fresh
clone, matching the WinUI templates.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Stop .gitignore from swallowing template publish profiles
The blanket `*.pubxml` rule silently ignored the Reactor templates' publish
profiles, so they were never committed and Release builds failed from a
clean clone (fixed in 8ea4586 by force-adding them).
`git add -f` fixed those 12 files but not the mechanism: the next template
added, or a new RID added to an existing one, hits the same trap and it is
again invisible to anyone whose working tree still has the files on disk.
Add a `!dev/Templates/**/*.pubxml` negation alongside the existing one for
dev/vsix, so template publish profiles are committable by default. Publish
profiles elsewhere in the repo stay ignored.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Match the XAML templates' icon font and size
The Reactor templates hardcoded `Segoe Fluent Icons` on their FontIcons and
left the size at WinUI's default. The XAML templates do neither: they use
`<FontIcon Glyph="..." />` with no FontFamily, inheriting SymbolThemeFontFamily,
which resolves the right icon font for the OS — worth keeping given the
templates support Windows 10 1809 and Segoe Fluent Icons is Windows 11 era.
The MVVM template also sets FontSize="14" on its counter icons, which ours
missed, so the buttons rendered larger than their XAML counterpart:
XAML MVVM 58x41
Reactor MVU 67x51 -> 58x42
Verified against Microsoft.UI.Reactor 0.1.0-preview.12: both templates build
warning-free and the glyphs render correctly (Icon(FontIcon(...)) resolves
through IconResolver, not the Button(string) content path).
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Update Reactor dotnet new templates for 0.1.0-preview.13
Reactor preview.13 adds declarative APIs that let the Navigation and
TabView templates drop their imperative escape hatches:
- NavigationView: use IsBackButtonVisible / IsPaneToggleButtonVisible and
.IsPaneOpen(value, handler) instead of UseRef + OnMount downcast, and
select the built-in Settings item via NavigationViewElement.SettingsTag.
OnSelectedTagChanged still reports null for Settings by design, so
TagToRoute keeps handling both SettingsTag and null.
- TitleBar: use .Tall() instead of the UseEffect + DispatcherQueue hop and
the redundant .Height(48).
- TabView: set FillContentArea, removing the MinHeight workaround.
Also bump the reactorVersion default to 0.1.0-preview.13 in all four
Reactor template.json files.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 38f0823a-2ee4-4c65-8f3f-7dde6a049509
* Let the OS size the window, like the XAML templates
Drop the hardcoded 1000x700 from all four Reactor templates. Reactor
PR #924 makes ReactorApp.Run's width/height optional, so omitting them
lets the OS pick the size, matching the XAML templates which set no
size at all. Verified identical to a freshly scaffolded XAML app.
This requires 0.1.0-preview.13; before #924, omitting width/height
meant a hardcoded 1024x768 rather than an OS-chosen size.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 38f0823a-2ee4-4c65-8f3f-7dde6a049509
* Add the MsixPackage launch profile to the Reactor templates
The Reactor templates were originally unpackaged and their launchSettings.json
only declared `commandName: Project` profiles. Making them single-project MSIX
updated the csproj and added Package.appxmanifest but left launchSettings
behind, so F5 from Visual Studio failed:
...launchSettings.json does not contain a profile with commandName
'MsixPackage'. To debug a packaged single-project MSIX solution, a profile
with command name MsixPackage in launchSettings.json is required.
`dotnet run` goes through Microsoft.Windows.SDK.BuildTools.WinApp instead, so
it worked throughout and the gap went unnoticed.
Declare the same Package / Unpackaged pair the WinUI XAML templates ship, with
Package first so it is the F5 default, and keep the Reactor devtools profile
alongside them.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Float the Windows App SDK version in the Reactor templates
The Reactor templates pinned Microsoft.WindowsAppSDK to 2.1.3 while the WinUI
XAML templates default to `*`. That pinned new projects to an older SDK than
the one Visual Studio offers everywhere else — 2.3.1 is current, so scaffolded
Reactor apps were two releases behind for no reason.
2.1.3 was the version Reactor happened to be built against, not a hard floor:
Microsoft.UI.Reactor 0.1.0-preview.13 depends on Microsoft.WindowsAppSDK.WinUI
>= 2.1.0. Keep the requirement in the parameter description rather than
encoding it as the pin, matching the WinUI templates.
Verified with the floated default: all four templates resolve
Microsoft.WindowsAppSDK 2.3.1, build warning-free, and launch with package
identity.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Accept MSB4181 for the invalid-version restore test on .NET SDK 10
The dotnet-new template test stage previously installed only the .NET 8 SDK.
Adding the .NET 10 SDK (needed because the Reactor templates target net10.0)
means the muxer now selects 10.0.400 for the pre-existing winui negative
tests as well, which changes the diagnostic NuGet emits for an unparseable
package version:
SDK 8.0.424 / 9.0.317 : no error code; "'not-a-version' is not a valid
version string."
SDK 10.0.400 : MSB4181 - RestoreTask returns false without
logging the descriptive error, so no NU code
surfaces at all.
The old assertion short-circuited on any error code being present and
demanded NU1105, so MSB4181 failed the run. (NU1105 never actually
surfaced on any tested SDK; the test passed via the descriptive-message
fallback.) Accept NU1105/MSB4181 or the descriptive message, and stop a
code's presence from bypassing the fallback.
Verified on 8.0.424 / 9.0.317 / 10.0.400. Unrelated failures (NU1101,
NU1301) still fail the assertion, so it is not weakened to "any failure".
Scenario 5's build check (NETSDK1004), scenario 6 (NU1102) and scenario 7
(NU1301) were confirmed unchanged on SDK 10.
Also corrects an adjacent comment that said NETSDK1005 where the check and
the observed behaviour are both NETSDK1004.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Pin the Reactor test-stage .NET SDK instead of floating 10.0.x
Every other UseDotNet@2 task in this repo pins an exact SDK (6.0.414,
6.0.427, 8.0.100, 9.0.200, 10.0.111); #6624 added the BuildVSIX pin so
Component Governance scans a patched toolchain, and #6699 bumped it
deliberately for a security release. reactorDotnetSdkVersion was the only
new floating pin.
Floating is what broke CI: '10.0.x' resolved to 10.0.302 when the task was
added on 2026-07-23, then silently jumped a whole feature band when 10.0.400
shipped on 2026-08-11. 10.0.400 changed the NuGet diagnostic for an
unparseable package version, so the pre-existing winui invalid-version smoke
test started failing with no corresponding source change.
Pinned to 10.0.400 rather than the 10.0.111 used by BuildVSIX. The Reactor
templates cannot use the 1xx servicing band: Microsoft.UI.Reactor's analyzers
and source generators (Reactor.Analyzers, Reactor.Wrappers.Generator,
Reactor.Localization.Generator) reference Roslyn 5.6.0.0, and 10.0.111 ships
Roslyn 5.0.0.0, so all three are silently disabled with CS9057 while the
build still reports success. Verified by building the same scaffolded project
on both SDKs: 10.0.111 -> 3x CS9057, 10.0.400 (Roslyn 5.9.0.0) -> 0 warnings.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Accept NETSDK1005 for the invalid-version build test on .NET SDK 10
Follow-up to 5ff625e, which fixed the restore assertion in the same
scenario. The build assertion immediately after it failed for the same
underlying reason.
On .NET SDK 10 NuGet's RestoreTask fails (MSB4181) but still writes a
partial obj/project.assets.json. The subsequent --no-restore build therefore
reports NETSDK1005 (assets file has no target for the project's TFM) rather
than NETSDK1004 (assets file missing). Accept either, since both mean restore
did not produce usable assets. Scenario 7's list gains NETSDK1005 for the
same reason; it currently resolves to NU1301, but NETSDK1004 is in that list
for the 'restore never succeeded' case whose SDK 10 equivalent is 1005.
This also reverts the NETSDK1005 -> NETSDK1004 comment change from 5ff625e.
That comment was correct as written; I changed it based on a repro that used
a bare net8.0 project with no obj directory, which produces NETSDK1004 and
does not reproduce the real template's behaviour. Re-verified with a faithful
repro (net10.0-windows10.0.26100.0, WinUI props, same build flags), which
reproduces CI exactly: assets file present after the failed restore, build
fails with NETSDK1005.
All four error-code assertions in this script are now backed by faithful
repros on the pinned SDK 10.0.400: restore MSB4181, build NETSDK1005,
scenario 6 NU1102, scenario 7 NU1301. Unrelated failures still throw.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
Copilot-Session: 38f0823a-2ee4-4c65-8f3f-7dde6a049509
* dotnet new WinUI3 templates 0.0.7-alpha (#6729)
* Propagate the PackageReference version into the C# wapproj template's BlankApp project file (#6705)
* Fix AppInstance process handle validation (#6733)
* Fix AppInstance process handle validation
Open non-current processes with SYNCHRONIZE once and transfer the
validated handle into AppInstance before registering the termination wait.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Set ApplicationIcon in the C# app templates (#6728)
* Set ApplicationIcon in the C# app templates
The eight C# app templates ship Assets\AppIcon.ico and reference it from the
window (AppWindow.SetIcon in the XAML templates, TitleBar.Icon in the Reactor
ones), but none of them set <ApplicationIcon>, so the icon was never embedded
into the built executable as a Win32 icon resource. Explorer, shortcuts, and
the taskbar for an unpackaged build therefore showed the default .NET icon.
Guarded with Condition=Exists so the property is simply skipped if a project
does not have the asset, matching the existing PublishProfile pattern in these
same templates.
Applied to all eight app templates that ship the asset -- the four WinUI XAML
ones and the four Reactor ones -- rather than only the Reactor set, so the
templates stay consistent with each other. ClassLibrary, PackagedApp, and
UnitTestApp do not ship an AppIcon.ico and are untouched.
Only ProjectTemplate.csproj is modified: that is the file scaffolded for the
user by both dotnet new and the VSIX. The WinUI.Desktop.Cs.*.csproj files are
VSSDK packaging projects for the VSIX itself, not scaffolded output.
Validated by scaffolding and building all eight templates (Debug, x64): all
build clean and the icon is embedded in the produced exe, confirmed by
ExtractAssociatedIcon returning a 32x32 icon for each. The two warnings from
winui-mvvm are pre-existing MVVMTK0045 diagnostics from the MVVM Toolkit and
are unrelated.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Bump Reactor to 0.1.0-preview.14 and drop the explicit title bar icon
Reactor 0.1.0-preview.14 makes TitleBar(...) inherit the window's icon
(microsoft/microsoft-ui-reactor#1153), so the explicit
.Icon("ms-appx:///Assets/AppIcon.ico") in the templates is now redundant.
Removing it keeps the scaffolded App.cs to the shape a starter template
should have, and the title bar still renders the app mark.
Applies to the three templates that use a TitleBar element: ReactorBlankApp,
ReactorMvuApp and ReactorNavigationApp. ReactorTabViewApp has no TitleBar --
it extends content into the title bar with a custom drag region and renders
its icon through TabStripHeader = Image(...), which is a different surface
that the inherit behaviour does not cover -- so it keeps its explicit
reference and is intentionally untouched.
Validated against the real 0.1.0-preview.14 package, four scaffolded apps,
with the icon line present and absent:
control, .Icon present, preview.14 title bar mark shown
line removed, preview.14 title bar mark shown <- the change
line removed + icon: arg, preview.14 title bar mark shown
line removed, preview.13 NO mark
The last row is the negative control: identical source and capture method,
only the package version differs, so the mark on preview.14 is caused by the
inherit behaviour rather than by something incidental.
All four templates were then scaffolded from the built pack and built clean
on x64: Debug 0 warnings, Release 2 warnings. The Release warnings are
pre-existing IL2104 trim warnings from Microsoft.Windows.SDK.NET and
WinRT.Runtime, unrelated to this change and present without it.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
* Fix errors from missing resource dll with version info (#6725)
* [57514396] Use central TSA config for Foundation source analysis (#6738)
* Normalize line endings in AppInstance.cpp (#6752)
* AppNotificationManager::Show and AppInstance::GetInstances: refactor to noexcept workers (Bug 61688595) (#6714)
Refactor two hot-path WinRT projection entry points so a failed HRESULT does not trigger the deep C++/WinRT projection rethrow path that exhausted the stack via WIL's FormatMessage-based exception logging chain inside ResultFromCaughtException_CppWinRt::catch$8 (the Watson failure signature for this crash bucket).
AppNotificationManager::Show: move the throwing body into a private noexcept HRESULT worker (ShowImpl) using RETURN_HR_IF / RETURN_IF_FAILED / CATCH_RETURN. The public Show keeps the IsSupported() early-return and rethrows any failure HRESULT via THROW_IF_FAILED(ShowImpl(...)).
AppLifecycle::AppInstance::GetInstances: same pattern. The body becomes GetInstancesImpl(IVector<AppInstance>& out) noexcept; the public GetInstances() is a thin THROW_IF_FAILED(GetInstancesImpl(...)) wrapper. Internal callers (RedirectActivationToAsync at line 636) are unaffected because the public signature is unchanged.
These are the two entry points observed in the Watson cabs attached to this bucket: WinUI3 Controls Gallery cabs originate in Show, and the auto-filer cab originates in Photos -> AppInstance::GetInstances (frames 16-22 in the cab call stack). The catch handler one frame deeper is mis-attributed by Watson's PDB walker to SecurityDescriptorHelpers::NonDelegatingGetTrustLevel::catch$0 (lexically-nearest catch symbol).
AB#61688595
Co-authored-by: James Pike <jamespik@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Retire the standalone StoragePickersTestApp (#6779)
Remove the specialized UI harness, its assets, and solution mappings to simplify ongoing build maintenance. Keep the Storage Pickers implementation and automated tests unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Change WindowsAppSDK-DotnetNewTemplates template from Microsoft.NonOfficial to Microsoft.Official template (#6783)
* Add OneBranch.Official.CrossPlat.yml template to WindowsAppSDK-DotnetNewTemplates.yml
* Use Microsoft Official template for dotnet templates
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 30bef3b6-3b91-4f20-96cd-7c242a8a370f
---------
Copilot-Session: 30bef3b6-3b91-4f20-96cd-7c242a8a370f
* Add network isolation to WindowsAppSDK-DotnetNewTemplates.yml (#6787)
* Use path filter for template PR validation (#6782)
* Skip OneBranch tool download when running template change validation on PRs from forks
* Use ADO path filter for template PR validation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6baeec26-f59b-466a-8eab-cd3b09b19380
---------
Copilot-Session: 6baeec26-f59b-466a-8eab-cd3b09b19380
* Reactor templates: bump to 0.1.0-preview.16 and raise WindowsAppSDK floor to 2.2.0 (#6786)
Updates the four Reactor dotnet-new templates for the 0.1.0-preview.16 release:
reactorVersion 0.1.0-preview.14 -> 0.1.0-preview.16
windowsAppSdkVersion documented floor 2.1.3 -> 2.2.0
microsoft/microsoft-ui-reactor#1259 adopts ApplicationData.GetForUnpackaged(),
a Windows App SDK 2.2.0 API, and raises Microsoft.UI.Reactor's dependency on
Microsoft.WindowsAppSDK.WinUI to 2.2.1. Verified that commit is contained in
the v0.1.0-preview.16 tag, and that Directory.Build.props at that tag declares
WindowsAppSDKVersion 2.2.0 / WindowsAppSDKWinUIVersion 2.2.1.
These templates reference the full Microsoft.WindowsAppSDK metapackage (not the
lean WinUI sub-package) and scaffold packaged single-project MSIX apps, so the
correct consumer floor is metapackage 2.2.0 - the first version whose WinUI
dependency is 2.2.1 (2.1.3 -> WinUI 2.1.0; 2.2.0 -> WinUI 2.2.1).
Both edits ship together because the mismatch they prevent is silent, not a
restore error. Measured with a stub package carrying the post-#1259 dependency
shape: pinning --wasdk-version 2.1.3 restores with exit code 0 and zero
warnings, resolving Microsoft.WindowsAppSDK.WinUI 2.2.1 while the metapackage
holds Microsoft.WindowsAppSDK.Runtime at exactly [2.1.3]. WinUI is only ever a
transitive reference, so NuGet takes the highest floor and never reports
NU1605; the result is 2.2.1 reference assemblies over a 2.1.3 runtime, which
builds clean and fails at runtime. The same probe at 2.2.0 resolves Runtime
2.2.0. Since the toolchain raises no error, the description text is the only
guardrail a user pinning explicitly will ever see.
Validated by scaffolding all four templates from a locally packed template
pack: each emits Microsoft.UI.Reactor and Microsoft.UI.Reactor.Devtools at
0.1.0-preview.16, Microsoft.WindowsAppSDK at the default '*', and a
Package.appxmanifest. Also verified the explicit pin path
(--wasdk-version 2.2.0 --reactor-version 0.1.0-preview.16).
Restore/build validation is deferred to CI: 0.1.0-preview.16 is on nuget.org
but has not yet mirrored to the internal package proxy reachable from this
machine.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Pin GitHub Actions to full-length commit SHAs (#6791)
Co-authored-by: Dan Fiedler <danfiedler@microsoft.com>
* Retry transient bootstrap initialization registration failures in tests (#6816)
* Retry transient bootstrap initialization failure
Retry MddBootstrapInitialize only when DDLM registration is temporarily unavailable, using a bounded exponential backoff.
AB#62489699
* Migrate TouchDownBuild-WinAppSDK (#6814)
* Fix TouchDown variable group
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e6b7e13d-59ce-4751-91f4-e8e589787837
* Fix TouchDown service connection
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e6b7e13d-59ce-4751-91f4-e8e589787837
* Use TouchDown Azure resource connection
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e6b7e13d-59ce-4751-91f4-e8e589787837
---------
Copilot-Session: e6b7e13d-59ce-4751-91f4-e8e589787837
---------
Co-authored-by: Roman Sivak (AKVELON INC) <v-romansivak@microsoft.com>
Co-authored-by: ssparach <128866445+ssparach@users.noreply.github.com>
Copilot-Session: e6b7e13d-59ce-4751-91f4-e8e589787837
---------
Co-authored-by: Lauren Ciha <laurenciha@microsoft.com>
Co-authored-by: Gordon Lam <73506701+yeelam-gordon@users.noreply.github.com>
Co-authored-by: Howard Kapustein <howardk@microsoft.com>
Co-authored-by: Vineeth Thomas Alex <vineeththomasalex@gmail.com>
Co-authored-by: Vineeth Thomas Alex <vithoma@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: v-goradojcic <v-goradojcic@microsoft.com>
Co-authored-by: Alexandre Zollinger Chohfi <alzollin@microsoft.com>
Co-authored-by: Dinah Xiaoman G <116714259+DinahK-2SO@users.noreply.github.com>
Co-authored-by: Scott Jones <sjones@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Manodasan Wignarajah <mawign@microsoft.com>
Co-authored-by: Dreynor87 <wall_c.s@outlook.com>
Co-authored-by: James Pike <jamespik@microsoft.com>
Co-authored-by: Dan Fiedler <danfiedler@microsoft.com>
Co-authored-by: romsi1983 <41230962+romsi1983@users.noreply.github.com>
Co-authored-by: Roman Sivak (AKVELON INC) <v-romansivak@microsoft.com>
Co-authored-by: ssparach <128866445+ssparach@users.noreply.github.com>
Copilot-Session: 2b34993b-0dcf-4789-94c5-97fd92cce84d
Copilot-Session: 5474160f-e555-4254-899b-783ad9211b71
Copilot-Session: 484c9277-ea15-4f85-a4a0-0429347ade48
Copilot-Session: 16d7d2fc-3d7c-41e8-9999-0c2084ba9ea1
Copilot-Session: 38f0823a-2ee4-4c65-8f3f-7dde6a049509
Copilot-Session: 30bef3b6-3b91-4f20-96cd-7c242a8a370f
Copilot-Session: 6baeec26-f59b-466a-8eab-cd3b09b19380
Copilot-Session: e6b7e13d-59ce-4751-91f4-e8e589787837
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
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.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Summary
Bumps Windows App SDK 2.1.3 to 2.2.0 and adopts the new
ApplicationData.GetForUnpackaged()API for unpackaged window-placement persistence, closing the row deferred in spec 059 section 5. Along the way it fixes a data-loss bug in the default persistence store, and 22 stale SDK version pins that the previous bump structurally could not see (18 of which ship inside the NuGet agent kit).Linked issue / spec
Implements
docs/specs/063-unpackaged-app-data.md. Amendsdocs/specs/059-titlebar-drag-regions.mdsection 3 (two corrections). Related upstream defect: microsoft/WindowsAppSDK#6559.Test plan
tests/Reactor.Tests/UnpackagedAppDataStoreTests.cs(12 tests). These assert storage location, not round-trip behavior: both stores write the same JSON shape, so a round-trip would pass whichever one ran and would prove nothing about the adoption.tests/Reactor.Tests/JsonFileStoreConcurrencyTests.cs— the oracle is a count of surviving ids, because a round-trip passes whenever the last writer wins, which is exactly the broken behaviour.WinAppSDKReferenceGuardTests.No_literal_SDK_pin_sits_below_the_central_pinned_version— sweeps three pin shapes, with positive controls that must match in production files so scanner decay reddens rather than passing.VersionOverride, sub-package, script$ver) reddens the guard with the offendingfile:line. Breaking the element regex reddens the positive control.dotnet restore Reactor.slnxthendotnet build Reactor.slnx --no-restore -c Release: 0 errors attributable to this change, matching a baseline measured with the changes stashed.dotnet test tests/Reactor.Tests: 14066 total, 0 failed.pr-reviewskill, then 13 rounds of Copilot review with all findings addressed.Risk / breaking changes
Behaviour change in the default store — read this even if you skip the rest. The concurrency fix lands in
JsonFileStore, which is the auto-detected default for every unpackaged app, so it is not opt-in. Writes now take a cross-process lock file (a.locksidecar appears besidereactor-windows.json), wait up to 250 ms for it, and abandon the save if it cannot be acquired. That last point is deliberate: proceeding would merge a stale document and drop another window's entry, which is the bug being fixed, whereas skipping costs at most one window's placement — something the store's best-effort contract already permits. The commit path also moves fromFile.Move(overwrite: true)toFile.Replace.The new API is opt-in.
UnpackagedAppDataStoreis only used if an author assigns it; auto-detection still selectsJsonFileStore. The two stores key data differently (publisher/product vs entry process name), so switching does not migrate existing layouts — an app that opts in starts from an empty layout at the new root, and the old file stays on disk, unread.Consumer floor rises.
Microsoft.UI.Reactoris framework-dependent and flows only the lean WinUI sub-package, so the floor consumers inherit isMicrosoft.WindowsAppSDK.WinUI >= 2.2.1; self-contained/MSIX projects that reference the metapackage themselves move to2.2.0. This is in-place servicing within the same side-by-side runtime family (Microsoft.WindowsAppRuntime.2), so no new runtime generation is required — the same reasoning spec 059 D5 accepted for 2.1.3.New public API, deliberately minimal: a constructor plus the two
IWindowPersistenceStoremethods.Pathisinternal(spec 063 D7) so the public surface stays identical under every outcome of theLocalPath-vs-LocalSettingsrevisit that section 5 leaves open.Scope note: this touches window placement only. Dock layouts use a different mechanism (
WindowPersistedScope, anIPersistedStateScope) and are untouched.Notes for reviewers
The diff is lopsided. About 30 files are mechanical one-line version-pin edits plus a regenerated API index and guide, skimmable in one pass. The parts worth real attention are
UnpackagedAppDataStore.cs,CrossProcessWriteGuard.cs,JsonFileStore.cs, their tests, and spec 063.Why
LocalPathand notLocalSettings. On 2.2.0,GetForUnpackaged().LocalSettingswrites to a roaming registry hive instead of the machine-local key it is contracted to use. That was measured, not taken from the release note, with a three-arm probe so that "found in neither hive" would have been reported as a broken probe rather than as absence of the defect. Window placement is monitor-topology and DPI dependent, so roaming it restores windows onto monitors that do not exist on the current machine.Worth knowing: the published release notes attribute that fix to 2.5.1, and that attribution is wrong — a two-arm experiment places it at metapackage 2.3.1 / Foundation 2.3.5. The follow-up trigger is therefore keyed to an observable condition enforced by a live test, not to a version number. Section 3.2 has the detail, including why behaviour follows the loaded runtime rather than the SDK pin.
Why the pin fixes are in scope. Spec 059 told future bumps to grep
Microsoft.WindowsAppSDK" Version=. That shape cannot match a#:package Microsoft.WindowsAppSDK@<version>header, so all 22 occurrences across 20 files were missed and sat a full major-minor below the floor that PR itself raised. Three of them live in a generator system prompt that was teaching an LLM to emit the stale pin into new apps. Fixing the instances without fixing the class would have guaranteed a third occurrence at the next bump, so the prose instruction is replaced by a guard test whose positive controls must be satisfied from production files.