Skip to content

[Windows] Lifecycle: Add AppInstance activated event - #34883

Closed
mattleibow wants to merge 9 commits into
mainfrom
windows-app-activation-lifecycle
Closed

[Windows] Lifecycle: Add AppInstance activated event#34883
mattleibow wants to merge 9 commits into
mainfrom
windows-app-activation-lifecycle

Conversation

@mattleibow

@mattleibow mattleibow commented Apr 8, 2026

Copy link
Copy Markdown
Member

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Root Cause

On Windows, MAUI exposed launch and window lifecycle hooks but not the underlying AppActivationArguments payload. That made single-instance redirect, protocol or file activation, and follow-on Windows auth work harder to handle cleanly from MauiProgram without app-specific plumbing.

Description of Change

This PR adds a new Windows lifecycle hook:

  • WindowsLifecycle.OnAppInstanceActivated(UI.Xaml.Application, AppActivationArguments) -> bool
  • IWindowsLifecycleBuilder.OnAppInstanceActivated(...)

The name matches the underlying AppInstance.GetCurrent().Activated event.

MauiWinUIApplication now:

  • captures the initial AppInstance.GetActivatedEventArgs() payload,
  • registers once for AppInstance.GetCurrent().Activated,
  • raises the new lifecycle event once MAUI services are available,
  • respects the handler return value so apps can mark an activation as handled and suppress the default launch flow when appropriate.

The Controls sample demonstrates using this from MauiProgram to keep the app single-instanced, redirect later launches back into the running instance, and re-activate the existing MAUI window.

Key Technical Details

  • Initial activation is raised after _services = applicationContext.Services so lifecycle handlers can actually run.
  • Redirected activations and later re-activations flow through the same OnAppInstanceActivated hook.
  • Registration is guarded so AppInstance.Activated is only subscribed once.

Testing

Cross-platform lifecycle event order device tests added in src/Core/tests/DeviceTests/:

  • Windows: validates OnAppInstanceActivatedOnLaunchingOnWindowCreatedOnLaunched (order and exactly-once)
  • Android: validates OnCreateOnStartOnResume (order and exactly-once)
  • iOS/MacCatalyst: validates FinishedLaunchingOnActivated (order and exactly-once)

These are real device tests that run in a platform app context, not console unit tests behind #if WINDOWS.

Issues Fixed

Related to #9973

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 8, 2026 18:02
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34883

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34883"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Windows lifecycle hook to surface WinAppSDK AppActivationArguments to MAUI apps, enabling scenarios like single-instance redirect/protocol/file activation handling from MauiProgram without custom Windows entry-point plumbing.

Changes:

  • Introduces WindowsLifecycle.OnAppActivation(Application, AppActivationArguments) -> bool plus IWindowsLifecycleBuilder.OnAppActivation(...) extension.
  • Wires MauiWinUIApplication to capture the initial AppInstance.GetActivatedEventArgs() payload, subscribe once to AppInstance.Activated, and invoke the lifecycle hook after services are available (with a “handled” short-circuit for launch flow).
  • Updates the Controls sample to demonstrate single-instance redirect and window re-activation; adds a unit test ensuring multiple handlers can be registered and that “handled” aggregates.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Core/tests/UnitTests/LifecycleEvents/LifecycleEventsTests.cs Adds a Windows-only unit test covering registration/invocation semantics for OnAppActivation.
src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt Records newly added public APIs for Windows.
src/Core/src/Platform/Windows/MauiWinUIApplication.cs Captures/propagates activation args and raises the new lifecycle event (initial + subsequent activations).
src/Core/src/LifecycleEvents/Windows/WindowsLifecycleBuilderExtensions.cs Adds the OnAppActivation builder extension.
src/Core/src/LifecycleEvents/Windows/WindowsLifecycle.cs Defines the new OnAppActivation delegate type.
src/Controls/samples/Controls.Sample/MauiProgram.cs Demonstrates using OnAppActivation for WinAppSDK single-instance redirect + window activation.

Comment thread src/Controls/samples/Controls.Sample/MauiProgram.cs
@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The CanAddWindowsOnAppActivationLifecycleEvent unit test is well-structured and directly exercises the new API, but key portions of the fix — the MauiWinUIApplication.OnAppActivation virtual method, RegisterForAppActivation, and the changes to OnLaunched — are not covered. One edge case in the multi-handler accumulation logic is also missing.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34883 — Windows OnAppActivation lifecycle event
Test files evaluated: 1 (LifecycleEventsTests.cs)
Fix files: 4


Overall Verdict

⚠️ Tests need improvement

The unit test correctly exercises the new OnAppActivation delegate, builder extension, and multi-handler accumulation, but the substantial logic added to MauiWinUIApplication (OnAppActivation virtual method, RegisterForAppActivation, and changes to OnLaunched) is not tested. A sticky-handled edge case is also missing.


1. Fix Coverage — ⚠️ Partial

The test covers:

  • ✅ Registering OnAppActivation via builder.AddWindows(w => w.OnAppActivation(...))
  • ✅ Multiple handlers being called when the event is invoked
  • ✅ The wasHandled accumulation pattern (false + true → true)

Not covered:

  • MauiWinUIApplication.OnAppActivation(AppActivationArguments) virtual method (tested only at the ILifecycleEventService layer, not through the class that orchestrates it)
  • ❌ The OnLaunched early-return path when OnAppActivation returns true (window creation is skipped)
  • RegisterForAppActivation() — the subscription to AppInstance.Activated for future activations
  • ❌ The reactivation branch (_application != null && _services != null) where OnAppActivation is now also called first

2. Edge Cases & Gaps — ⚠️ One gap

Covered:

  • Two handlers: first returns false, second returns truewasHandled is true

Missing:

  • Sticky-handled semantics: first handler returns true, second returns falsewasHandled should remain true. The implementation uses wasHandled = del(this, args) || wasHandled, which is correct, but this specific order (true + false → true) is not tested. A malformed implementation could use wasHandled = del(this, args) and pass the current test yet fail this case.
  • No-handler case: when no OnAppActivation handlers are registered, OnAppActivation(args) should return false without throwing — worth a quick explicit test.

3. Test Type Appropriateness — ✅ Appropriate

Current: Unit test (xUnit [Fact])
Recommendation: Correct choice for testing event registration, delegate dispatch, and return-value accumulation — no platform context needed for these behaviors.

The MauiWinUIApplication changes are Windows-platform-specific and would realistically require device tests to cover fully, so the omission there is more understandable, though still worth noting as a gap.


4. Convention Compliance — ✅ No issues

  • [Fact] attribute used correctly
  • Properly gated with #if WINDOWS
  • No naming, attribute, or structural issues detected (script reported 0 convention violations)

5. Flakiness Risk — ✅ Low

Pure unit test with no async/UI/timing concerns. No risk factors identified.


6. Duplicate Coverage — ✅ No duplicates

This is a new API surface. The test follows the same pattern established by CanAddAndroidOnKeyDownLifecycleEvent and similar tests in the same file — consistent, not redundant.


7. Platform Scope — ✅ Appropriate

Fix is entirely Windows-specific (MauiWinUIApplication is Windows-only; AppActivationArguments is WinAppSDK). The new test is correctly guarded by #if WINDOWS. Platform scope is appropriate.


8. Assertion Quality — ✅ Good (for what is tested)

Assert.True(firstHandlerCalled);    // Both handlers were invoked
Assert.True(secondHandlerCalled);
Assert.True(wasHandled);            // Accumulated return value is correct

Assertions are specific and directly verify the intended behavior. The null! arguments passed to the delegate are acceptable for a unit test of registration/accumulation logic.


9. Fix-Test Alignment — ⚠️ Partial alignment

Changed Code Tested?
WindowsLifecycle.OnAppActivation delegate ✅ Yes
WindowsLifecycleBuilderExtensions.OnAppActivation ✅ Yes
MauiWinUIApplication.OnAppActivation(AppActivationArguments) virtual method ❌ No
MauiWinUIApplication.RegisterForAppActivation() ❌ No
OnLaunchedLaunchActivatedEventArgs assignment ❌ No
OnLaunched — early-exit on activation handled ❌ No
OnLaunched — reactivation path now calls OnAppActivation first ❌ No

The test aligns with the public API surface (delegate + builder extension) but does not test the runtime orchestration in MauiWinUIApplication.


Recommendations

  1. Add sticky-handled test — Add a second scenario in CanAddWindowsOnAppActivationLifecycleEvent (or a new [Fact]) where the first handler returns true and the second returns false, asserting wasHandled is still true. This would catch a simple regression in the accumulation logic.

  2. Consider testing the no-handler case — Add a brief test that invokes InvokeEvents(WindowsLifecycle.OnAppActivation) with no registered handlers and verifies wasHandled remains false (mirrors the existing InvokingUnregisteredEventsDoesNotThrow test pattern).

  3. Note for future work — The changes to OnLaunched (early-return when activation is handled, LaunchActivatedEventArgs capture, RegisterForAppActivation) are not covered by any test. These paths are difficult to unit-test because they depend on AppInstance (WinAppSDK), so a device test would be needed. Consider tracking this as a known gap.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

Register OnAppActivation, OnLaunching, OnWindowCreated, and OnLaunched
handlers during Core.DeviceTests startup that record event names into a
static log. Three automated tests validate that all four events fire,
fire in the correct order, and OnAppActivation fires exactly once.

This replaces the unit test that was behind #if WINDOWS in a net10.0
console project (which never actually ran) with real device tests that
execute in a Windows app context.

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

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The unit and device tests cover the happy-path registration and startup ordering well, but the re-activation path and return-value-based suppression — the PR's two most important behaviors — are not tested at all.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34883 — [Windows] Lifecycle: Add app activation event
Test files evaluated: 4 (1 unit test file, 3 device test files)
Fix files: 4 (WindowsLifecycle.cs, WindowsLifecycleBuilderExtensions.cs, MauiWinUIApplication.cs, Controls.Sample/MauiProgram.cs)


Overall Verdict

⚠️ Tests need improvement

The startup-time path is covered (registration wiring, event ordering, fires-once), but two behaviors unique to this PR — the bool return value suppressing the default launch flow, and the re-activation path when _application != null — are entirely unexercised.


1. Fix Coverage — ⚠️

The unit test (CanAddWindowsOnAppActivationLifecycleEvent) verifies that multiple OnAppActivation handlers can be registered and that the wasHandled aggregation works at the service layer. The three device tests verify that OnAppActivation appears in the startup log, fires before OnLaunching, and fires exactly once.

What is NOT covered:

  • The MauiWinUIApplication.OnAppActivation virtual method that actually calls InvokeLifecycleEvents and propagates wasHandled to the caller — this is the glue between the service-layer test and real app behavior.
  • The re-activation branch (if (_application != null && _services != null)) in OnLaunched, which is critical for single-instance scenarios and the primary motivation for the PR.
  • The RegisterForAppActivation path: that AppInstance.Activated is wired up and fires through the same hook on subsequent activations.

2. Edge Cases & Gaps — ⚠️

Covered:

  • Multiple handlers both receive the call (unit test)
  • First handler returning false does not prevent second handler from running (unit test)
  • wasHandled becomes true when any handler returns true (unit test)
  • OnAppActivation fires before OnLaunching / OnWindowCreated / OnLaunched (device test)
  • Fires exactly once during startup (device test)

Missing:

  • Return-value suppressionMauiWinUIApplication.OnLaunched returns early when OnAppActivation returns true, suppressing the default window-creation flow. No test verifies this short-circuit behavior.
  • Re-activation path — The branch if (_application != null && _services != null) executes when an already-running MAUI app is activated again (e.g., single-instance redirect). This is the core single-instance scenario described in the PR description and is completely untested.
  • RegisterForAppActivation guard_isRegisteredForAppActivation ensures the AppInstance.Activated subscription only happens once. No test verifies this guard, meaning double-registration is undetected.
  • Null args — The unit test invokes handlers with null! for both arguments. While acceptable for wiring tests, a test verifying that real AppActivationArguments are forwarded to handlers would strengthen coverage.

3. Test Type Appropriateness — ✅

Current: Unit Test + Device Test
Recommendation: Same — this is the correct choice.

The feature is purely Windows-specific (WinAppSDK AppInstance API). Unit tests appropriately cover the lifecycle event registration/wiring layer. Device tests appropriately cover the startup-time integration behavior that requires a real Windows app instance. No UI test is needed; there are no visual changes.

The re-activation and suppression gaps above could potentially be covered by unit tests against a MauiWinUIApplication subclass mock rather than requiring additional device tests.


4. Convention Compliance — ✅

  • Unit test uses [Fact] attributes (xUnit) ✅
  • Device test uses [Category(TestCategory.Application)]
  • Device test methods use [Fact(DisplayName = "...")] with descriptive names ✅
  • Guarded correctly with #if WINDOWS
  • No naming, attribute, or anti-pattern issues detected by automated checks

5. Flakiness Risk — ⚠️ Medium

The device tests depend on MauiProgram.LifecycleEventLog — a static List(string) populated during app startup. This is effectively read-only after startup, which is low risk. However:

  • Static shared state: if xUnit runs the three LifecycleEventOrderTests methods in parallel (possible without [Collection] isolation), all three read from the same static list. In practice, device test runners typically run tests sequentially, but the static design is fragile — an Assert.Contains check on a list that was populated during app init could silently pass if startup order ever changes.
  • No test for negative case: OnAppActivationFiresExactlyOnce asserts count == 1, but if RegisterForAppActivation were called multiple times and AppInstance.Activated fired twice, this test would catch it. This is actually good; it's worth keeping.

6. Duplicate Coverage — ✅ No duplicates

No existing OnAppActivation tests found in the repository. The unit test (CanAddWindowsOnAppActivationLifecycleEvent) is a new addition to LifecycleEventsTests.cs following the same pattern as the existing CanAddAndroidOnKeyDownLifecycleEvent and sibling tests. This is consistent and not duplicative.


7. Platform Scope — ✅

The fix exclusively modifies Windows-specific files (Platform/Windows/MauiWinUIApplication.cs, LifecycleEvents/Windows/WindowsLifecycle.cs, LifecycleEvents/Windows/WindowsLifecycleBuilderExtensions.cs). The Controls.Sample/MauiProgram.cs change is a cross-platform sample file but only adds #if WINDOWS-guarded code. All tests are appropriately Windows-only (LifecycleEventOrderTests.Windows.cs, #if WINDOWS unit test). Platform scope matches the fix scope.


8. Assertion Quality — ✅

Device tests:

  • Assert.Contains(nameof(WindowsLifecycle.OnAppActivation), log) — precise, named constant ✅
  • Index comparison with diagnostic message $"Expected OnAppActivation before OnLaunching. Log: [{string.Join(...)}]" — excellent, self-diagnosing on failure ✅
  • Assert.Equal(1, count) — precise count assertion ✅

Unit test:

  • Assert.True(firstHandlerCalled) / Assert.True(secondHandlerCalled) — confirms both handlers executed ✅
  • Assert.True(wasHandled) — confirms aggregated return value propagation ✅

The one weakness: the unit test invokes with del(null!, null!). The assertions are correct for wiring purposes but don't validate forwarding of real arguments to handlers.


9. Fix-Test Alignment — ⚠️

Fix Component Covered by Tests?
WindowsLifecycle.OnAppActivation delegate declaration ✅ Unit test
IWindowsLifecycleBuilder.OnAppActivation extension method ✅ Unit test (via AddWindows(windows => windows.OnAppActivation(...)))
MauiWinUIApplication.OnAppActivation (virtual method, invokes service) ⚠️ Indirectly via device test startup; return-value behavior untested
MauiWinUIApplication.OnLaunched — initial activation path ✅ Device test (fires during startup)
MauiWinUIApplication.OnLaunched — re-activation path (_application != null) ❌ Not tested
RegisterForAppActivation — one-time AppInstance.Activated subscription ❌ Not tested
Return value suppresses default launch flow ❌ Not tested

Recommendations

  1. Add a unit test for return-value suppression — Test that when OnAppActivation returns true, MauiWinUIApplication.OnLaunched returns early without calling OnLaunching/OnLaunched. This can be done by subclassing MauiWinUIApplication and overriding the virtual OnAppActivation to return true, then asserting the launching/launched lifecycle events do NOT appear in the event log.

  2. Add a device test for the re-activation path — The re-activation scenario (single-instance redirect calling OnLaunched again while _application != null) is the primary motivation for this PR but is untested. A device test that simulates a second activation while the app is running would validate the most important behavioral path.

  3. Consider adding [CollectionDefinition] or [Collection] isolation for the LifecycleEventOrderTests class to avoid potential issues with the shared static LifecycleEventLog if parallelism is ever enabled.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

…lifecycle tests

Rename the lifecycle delegate and all references from OnAppActivation to
OnAppInstanceActivated to match the underlying AppInstance.Activated
event name.

Remove the #if WINDOWS unit test from LifecycleEventsTests.cs that never
actually ran in the net10.0 console test leg.

Add cross-platform lifecycle event order device tests:
- Windows: OnAppInstanceActivated -> OnLaunching -> OnWindowCreated -> OnLaunched
- Android: OnCreate -> OnStart -> OnResume
- iOS/MacCatalyst: FinishedLaunching -> OnActivated

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

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The tests cover startup event ordering well, but the core feature—handler return value semantics and the re-activation path—are not explicitly verified.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34883 — Windows app activation lifecycle event (OnAppInstanceActivated)
Test files evaluated: 6
Fix files: 4


Overall Verdict

⚠️ Tests need improvement

The device tests validate that OnAppInstanceActivated fires during startup and in the correct relative order, which is valuable. However, the boolean return-value contract (whether activation is "handled" and suppresses the default launch flow) and the re-activation path (via AppInstance.GetCurrent().Activated) are not tested.


1. Fix Coverage — ⚠️ Partial

The tests verify that OnAppInstanceActivated fires during app startup and appears before OnLaunching in the event log. This covers the event registration path.

Not covered: The fix's key behavior is that returning true from the handler short-circuits the normal launch/relaunch flow. The test handler always returns false, so the code path where if (launchActivation is ... && OnAppInstanceActivated(...)) return; actually returns early is never exercised. That branch is the central behavioral change in MauiWinUIApplication.OnLaunched.

2. Edge Cases & Gaps — ⚠️ Gaps present

Covered:

  • OnAppInstanceActivated fires during startup
  • Correct relative order: OnAppInstanceActivatedOnLaunchingOnWindowCreatedOnLaunched
  • Event fires exactly once during startup

Missing:

  • Handled = true path: No test verifies that returning true from the delegate suppresses OnLaunching / OnLaunched. This is the main behavioral contract of the new API and a regression would go undetected.
  • Multiple listeners: The implementation aggregates wasHandled = del(...) || wasHandled across listeners — behavior when one listener returns true and another returns false is untested.
  • Re-activation path: The RegisterForAppInstanceActivated callback (triggered when an already-running app receives a new activation) is not covered. This is the scenario most users of OnAppInstanceActivated will actually care about. Granted, testing this requires launching a second app instance, making it very hard to automate.

3. Test Type Appropriateness — ✅ Appropriate

Current: Device Tests (xUnit [Fact] in *.DeviceTests)
Recommendation: Device tests are the right choice here. App lifecycle events require a real platform context — they cannot be reliably tested with pure unit tests. The startup-ordering tests naturally live here.

One potential addition: the OnAppInstanceActivated aggregation logic in MauiWinUIApplication (wasHandled = del(...) || wasHandled) is pure C# logic that could be validated with a unit test using a mock IServiceProvider, without needing a real Windows context.

4. Convention Compliance — ✅ Pass

  • [Fact] attributes (xUnit) ✅
  • [Category(TestCategory.Lifecycle)] on test class ✅
  • Platform-specific file naming (.Windows.cs, .Android.cs, .iOS.cs) ✅
  • Lifecycle category added to TestCategory.cs
  • MauiProgramDefaults.CreateMauiApp extended with optional configureBuilder to avoid breaking existing callers ✅

No issues detected by automated checks.

5. Flakiness Risk — ✅ Low

The tests read from a static LifecycleEventLog list populated during app startup — no async waits, Task.Delay, or Appium interactions. This is a stable pattern.

Minor note: The log is a shared static List(string). If multiple test runs reuse the same process (which is typical for device tests), the log could accumulate entries from previous runs and affect the "fires exactly once" assertion. Currently each test pass starts fresh, but this is worth keeping in mind if the test suite is extended.

6. Duplicate Coverage — ✅ No duplicates

The Lifecycle category and LifecycleEventOrderTests are new additions. No overlapping existing tests found.

7. Platform Scope — ⚠️ Concern

The fix is Windows-specific, and Windows device tests are included. However, the script flags cross-platform changes: MauiProgramDefaults.cs (shared) and Controls.Sample/MauiProgram.cs are also modified.

The Android and iOS LifecycleEventOrderTests files added in the final commit are a good baseline parity gesture, but they test pre-existing events (OnCreate, FinishedLaunching) rather than anything introduced by this PR. They add value but are tangential to the fix.

MacCatalyst is not explicitly covered by a device test file, though the iOS test compiles for MacCatalyst via .ios.cs. That's acceptable given this is a Windows-only API.

8. Assertion Quality — ✅ Good

  • Assert.Contains(...) confirms each event fired ✅
  • Index comparisons with diagnostic messages ($"Expected X before Y. Log: [...]") make failures readable ✅
  • Assert.Equal(1, count) catches duplicate firing ✅
  • No magic numbers or brittle hardcoded values ✅

9. Fix-Test Alignment — ⚠️ Partial

The tests align with the event registration and ordering aspects of the fix. However, the return-value branching logic in MauiWinUIApplication.OnLaunched — the part that calls OnAppInstanceActivated(...) and conditionally returns early — is not exercised by any test, since the handler in MauiProgram.cs always returns false.


Recommendations

  1. Add a test for the "handled" return path — add a test case (or a variant of MauiProgram) where OnAppInstanceActivated returns true and verify that OnLaunching / OnLaunched do not appear in the log afterward. This directly validates the primary behavioral contract of the new API.
  2. Consider a unit test for the aggregation logic — mock IServiceProvider / ILifecycleEventService to test that MauiWinUIApplication.OnAppInstanceActivated correctly ORs results from multiple delegates (especially true || false → true).
  3. Document the static log limitation — add a comment near LifecycleEventLog noting it is populated once at startup and is intentionally static, so future contributors understand why it isn't reset between test runs.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

}

public static MauiApp CreateMauiApp(Func<IServiceProvider, TestOptions> options)
public static MauiApp CreateMauiApp(Func<IServiceProvider, TestOptions> options, Action<MauiAppBuilder> configureBuilder = null)

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configureBuilder is declared as a non-nullable Action<MauiAppBuilder> but defaults to null. With nullable annotations enabled in the repo this will produce a nullability warning (and can become a build break when warnings are treated as errors). Make the parameter nullable (e.g., Action<MauiAppBuilder>? configureBuilder = null).

Suggested change
public static MauiApp CreateMauiApp(Func<IServiceProvider, TestOptions> options, Action<MauiAppBuilder> configureBuilder = null)
public static MauiApp CreateMauiApp(Func<IServiceProvider, TestOptions> options, Action<MauiAppBuilder>? configureBuilder = null)

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +21
/// <summary>
/// Records platform lifecycle event names in the order they fire during app startup.
/// Used by LifecycleEventOrderTests to validate event ordering.
/// </summary>
public static List<string> LifecycleEventLog { get; } = new();

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LifecycleEventLog is static and never cleared. Since the new tests assert some events fire "exactly once", later activations/lifecycle events (or test host reuse) can make them flaky. Consider clearing the log at the start of CreateMauiApp() so each run starts from a known state.

Copilot uses AI. Check for mistakes.
Comment on lines 5 to 8
public static class WindowsLifecycle
{
public delegate bool OnAppInstanceActivated(UI.Xaml.Application application, AppActivationArguments args);
public delegate void OnActivated(UI.Xaml.Window window, UI.Xaml.WindowActivatedEventArgs args);

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description and metadata refer to a new OnAppActivation(...) hook, but the implementation introduces OnAppInstanceActivated(...) instead. Please update the PR description (or rename the API) so the documented public surface matches the code that will ship.

Copilot uses AI. Check for mistakes.
Address review feedback: clear the static event log at the start of
CreateMauiApp() so test host reuse cannot cause false failures in the
exactly-once assertions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mattleibow mattleibow changed the title [Windows] Lifecycle: Add app activation event [Windows] Lifecycle: Add AppInstance activated event Apr 9, 2026
Display a user-visible alert in the sample app when a redirected
activation brings the window to the foreground, so testers can confirm
single-instance behavior without a debugger.

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

github-actions Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The tests cover the happy-path startup order well, but the key new behavior — a handler returning true to short-circuit OnLaunching/OnLaunched — is not tested at all. The re-activation path (via AppInstance.Activated) is also untested.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34883 — Windows lifecycle: add OnAppInstanceActivated event
Test files evaluated: 6 (3 platform-specific LifecycleEventOrderTests, MauiProgram.cs, MauiProgramDefaults.cs, TestCategory.cs)
Fix files: 4 (WindowsLifecycle.cs, WindowsLifecycleBuilderExtensions.cs, MauiWinUIApplication.cs, Controls.Sample/MauiProgram.cs)


Overall Verdict

⚠️ Tests need improvement

The startup order and presence tests are solid, but the most important new behavior — returning true from OnAppInstanceActivated to short-circuit the normal launch flow — has no test coverage. This is the primary behavioral contract of the new API.


1. Fix Coverage — ⚠️

The tests verify that OnAppInstanceActivated fires during startup, fires in the correct order, and fires exactly once. These are necessary checks. However, the test's registered handler always returns false, meaning the short-circuit path in MauiWinUIApplication.OnLaunched (the if (...OnAppInstanceActivated(...)) return; guard) is never exercised. The most consequential logic added in this PR goes untested.

Covered:

  • OnAppInstanceActivated is present in the log ✅
  • OnAppInstanceActivated fires before OnLaunching
  • OnAppInstanceActivated fires exactly once during startup ✅

Not covered:

  • Handler returns trueOnLaunching/OnLaunched should be suppressed ❌
  • Re-activation path: AppInstance.Activated event fires OnAppInstanceActivated on subsequent launches ❌

2. Edge Cases & Gaps — ⚠️

Covered:

  • Happy-path startup order
  • Single-firing guarantee on startup

Missing:

Gap Description
Short-circuit behavior The bool return value is the key feature. A test should register a handler returning true and assert that OnLaunching/OnLaunched do NOT appear in the log afterward.
Multiple handlers / wasHandled aggregation MauiWinUIApplication.OnAppInstanceActivated uses wasHandled = del(...) || wasHandled so any handler returning true wins. Not tested with multiple handlers where only one returns true.
Re-activation (post-startup) RegisterForAppInstanceActivated wires AppInstance.GetCurrent().Activated. Tests only cover the initial app startup path, not the second-activation path. This path is admittedly hard to simulate in a device test.
Null _services OnAppInstanceActivated guards with _services?.Invoke.... The early-return path if services are null isn't exercised by tests.

3. Test Type Appropriateness — ✅

Current: Device Tests (xUnit [Fact], [Category(TestCategory.Lifecycle)])
Recommendation: Correct choice. The new event relies on AppInstance.GetCurrent() from the WinAppSDK and requires an actual running WinUI app. Unit tests cannot simulate the real startup flow. Device tests are the right level here.

The cross-platform Android and iOS lifecycle tests are a nice addition (baseline coverage for those platforms), even though the primary fix is Windows-only.


4. Convention Compliance — ✅

No convention issues detected by the automated script.

  • [Category(TestCategory.Lifecycle)] at class level (not duplicated on methods)
  • [Fact] attributes with descriptive DisplayName
  • TestCategory.Lifecycle constant added to TestCategory.cs
  • ✅ Platform-specific test files use the correct .Windows.cs / .Android.cs / .iOS.cs extensions

5. Flakiness Risk — ✅ Low

The LifecycleEventLog is a static List(string) populated at app startup (before any test runs) and only read by the tests — no mutation after startup. This is a stable pattern. Index-based ordering assertions include descriptive failure messages with the full log, which aids debugging when tests fail.

Minor note: the iOS test conditionally skips the OnActivated ordering assertion if the event hasn't fired yet (if (activatedIndex >= 0)). This makes the ordering half of the test effectively optional, slightly weakening it, but it's a reasonable pragmatic choice.


6. Duplicate Coverage — ✅ No duplicates

The existing LifecycleEventsTests.cs unit tests cover the generic lifecycle infrastructure (registration, invocation, service wiring) but not platform-specific event ordering. No duplication.


7. Platform Scope — ⚠️

The fix is Windows-only (MauiWinUIApplication.cs, WindowsLifecycle.cs, WindowsLifecycleBuilderExtensions.cs). The Windows device test correctly covers the changed platform. The Android and iOS tests are a welcome bonus for cross-platform baseline coverage.

Minor gap: MacCatalyst is not covered by the cross-platform lifecycle order tests (the iOS test file uses .iOS.cs, which does compile for MacCatalyst, but there are no MacCatalyst-specific lifecycle events being asserted).


8. Assertion Quality — ✅

Assertions are specific and well-written:

  • Assert.Contains confirms event presence
  • Assert.True(indexA < indexB, descriptiveMessage) confirms ordering with a full log dump on failure
  • Assert.Equal(1, count) confirms exactly-once firing

The error messages include the full log snapshot, which is excellent for diagnosability.


9. Fix-Test Alignment — ⚠️

The tests and fix are aligned for the startup presence/ordering concern. However the central behavioral contract of the fix — "a handler returning true suppresses subsequent lifecycle events" — is what distinguishes this new event from existing ones (which have void delegates). The test registers a handler that always returns false and never validates what happens when true is returned. A test for the true case would fully validate the most important code path added in MauiWinUIApplication.cs.


Recommendations

  1. Add a test for return true short-circuit behavior (highest priority): Register an OnAppInstanceActivated handler that returns true and assert that OnLaunching and OnLaunched do not appear in the event log. This is the key behavioral guarantee of the new API and currently has zero coverage.

  2. Add a multi-handler test: Register two handlers where only the second returns true. Verify the first still runs (both are always invoked) and that the combined result correctly suppresses OnLaunching/OnLaunched. This validates the wasHandled = del() || wasHandled aggregation logic.

  3. Consider noting re-activation as a known gap in a test comment: The second-activation path via AppInstance.Activated is difficult to test in the current device-test infrastructure (it requires simulating a second OS-level activation). Documenting this known gap in a comment would help future contributors know this is intentional rather than overlooked.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@MauiBot

This comment has been minimized.

@kubaflo

kubaflo commented May 24, 2026

Copy link
Copy Markdown
Contributor

/review -b feature/refactor-copilot-yml

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 1 findings

See inline comments for details.

[Fact(DisplayName = "Android lifecycle events fire during startup")]
public void AndroidLifecycleEventsFireDuringStartup()
{
var log = MauiProgram.LifecycleEventLog;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Device test reliability - These Android tests read LifecycleEventLog immediately, but OnStart/OnResume can be delivered after the test runner starts. That makes the assertions race startup lifecycle delivery and can fail with missing OnStart/OnResume. Please wait for Android startup to reach OnResume (or expose a startup-complete signal) before asserting the log/order.

@MauiBot MauiBot added the s/agent-fix-win AI found a better alternative fix than the PR label May 24, 2026
MauiBot

This comment was marked as outdated.

@IlGalvo

IlGalvo commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

I opened #36597 as a ready-for-review stacked follow-up to this PR.

It addresses the remaining actionable review feedback:

  • later AppInstance.Activated callbacks are dispatched through the MAUI application dispatcher while initial activation remains synchronous;
  • the missing Core Windows Public API baseline entries are restored;
  • the redundant Controls Sample dispatch is removed;
  • the Windows lifecycle order test now rejects missing events before comparing order;
  • the Windows WebAuthenticator test follows the existing Tests/Windows/ layout and the callback-routing comments are clarified.

The Android and iOS/MacCatalyst lifecycle tests added here are intentionally removed: Android has a real startup race with OnStart/OnResume, while the Apple headless runner does not traverse the normal MauiUIApplicationDelegate lifecycle path. These tests do not exist on main, so established coverage is unaffected.

Three review points are intentionally not code changes:

  • MauiProgramDefaults belongs to a nullable-disabled project and follows its existing optional-delegate convention;
  • System.Threading.Tasks is required for .AsTask() on RedirectActivationToAsync;
  • throwing for application-owned AppInstance keys would break the supported single-instance pattern where the app redirects protocol activation to the original instance.

Local validation passed: Core, Essentials, and Controls Sample Windows builds; WebUtils_Tests 20/20; Core lifecycle device tests 3/3; Essentials WebAuthenticator helper device tests 14/14. The Controls Sample second-instance redirect and the real Windows WebAuthenticator browser/callback flow were also manually validated.

@mattleibow @kubaflo, could you please review and merge #36597 into windows-app-activation-lifecycle, update this PR's Testing section to reflect the Windows-only lifecycle tests, request the Windows device-test runs if needed, and then run a fresh /review rerun?

…36597)

## Summary

Follow-up to #34883 addressing the remaining review feedback without
changing the public surface introduced by the parent PR.

- Dispatches later `AppInstance.Activated` notifications through the
MAUI application dispatcher while keeping the initial activation
synchronous and able to short-circuit window creation.
- Restores the four missing Core Windows Public API baseline entries.
- Removes the now-redundant window-level dispatch from the Controls
Sample.
- Hardens the Windows lifecycle order test against missing-event false
positives.
- Limits the new lifecycle device-test coverage to Windows.
- Clarifies the Windows WebAuthenticator callback-routing contract and
moves its Windows-only tests into the existing `Tests/Windows/` layout.

## Threading and lifecycle behavior

`MauiWinUIApplication` captures the application dispatcher after
app-level services are available. Initial activation continues to run
synchronously before `OnLaunching` and window creation, preserving the
existing `bool handled` contract. Only later `AppInstance.Activated`
callbacks are dispatched when required; if dispatch is rejected during
shutdown, the callback is not run off-thread.

## Test scope

The Android and iOS/MacCatalyst lifecycle tests added by the parent PR
are intentionally removed:

- The change is Windows-specific, while those tests cover pre-existing
platform lifecycle events.
- The Android headless runner can begin executing tests before
`OnStart`/`OnResume` delivery completes, creating a startup race.
- The Apple headless runner uses `MauiTestApplicationDelegate` rather
than the normal `MauiUIApplicationDelegate` lifecycle path.

Those files do not exist on `main`, so this does not remove established
MAUI coverage. Addressing the cross-platform runners belongs in a
separate change.

## Intentional review decisions

- No exception is added for application-owned `AppInstance` keys.
Single-instance apps preserve their own key and redirect protocol
activations to the original instance, where the MAUI lifecycle callback
completes WebAuthenticator. Throwing would break that supported pattern.
- `System.Threading.Tasks` remains in the Controls Sample because it
provides the `.AsTask()` extension used with
`RedirectActivationToAsync`.
- `MauiProgramDefaults` remains unchanged because
`Core.DeviceTests.Shared` has nullable annotations disabled; its
optional delegate follows the existing project convention.

## Validation

Automated/local:

- Core Windows build: passed
- Essentials Windows build: passed
- Controls Sample Windows build: passed
- `WebUtils_Tests`: 20/20 passed
- Core Windows lifecycle device tests: 3/3 passed
- Essentials Windows WebAuthenticator helper device tests: 14/14 passed
- Core and Essentials Windows DeviceTests projects: built successfully

Manual Windows validation:

- A true second Controls Sample instance redirected to the original
instance and terminated; the original window handled the activation on
the UI thread.
- The real WebAuthenticator browser/callback flow was validated on
Windows.

Android, iOS, and MacCatalyst runtime tests were not run for this
Windows-only follow-up.

Thanks in advance,
@mattleibow
@kubaflo
Copilot AI review requested due to automatic review settings July 15, 2026 20:30
@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Comment on lines 5 to 9
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;
using Maui.Controls.Sample.Controls;
using Maui.Controls.Sample.Pages;

previousResponse?.TrySetCanceled();

using (cancellationToken.Register(() => response.TrySetCanceled()))
Comment on lines +51 to +56
[Fact(DisplayName = "OnAppInstanceActivated fires exactly once during startup")]
public void OnAppInstanceActivatedFiresExactlyOnce()
{
var count = MauiProgram.LifecycleEventLog.Count(e => e == nameof(WindowsLifecycle.OnAppInstanceActivated));
Assert.Equal(1, count);
}
Comment on lines +17 to +22
/// <summary>
/// Records platform lifecycle event names in the order they fire during app startup.
/// Used by LifecycleEventOrderTests to validate event ordering.
/// </summary>
public static List<string> LifecycleEventLog { get; } = new();

Comment on lines +88 to +89
public async Task<WebAuthenticatorResult> AuthenticateAsync(WebAuthenticatorOptions webAuthenticatorOptions)
=> await AuthenticateAsync(webAuthenticatorOptions, CancellationToken.None);
@kubaflo

kubaflo commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

@IlGalvo Thank you sooo much for taking initiative in driving this PR to merge <3 If you want you can create a new PR and we close this one so that you don't have create new PRs in case of new suggestions

@MauiBot MauiBot added s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates and removed s/agent-fix-win AI found a better alternative fix than the PR labels Jul 15, 2026

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review Summary

@mattleibow — new AI review results are available based on this last commit: 144023a. To request a fresh review after new comments or commits, comment /review rerun.

Gate Inconclusive Confidence Low Platform Windows


🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ INCONCLUSIVE

Platform: WINDOWS · Base: main · Merge base: a96cb62b

Test Without Fix (expect FAIL) With Fix (expect PASS)
📱 LifecycleEventOrderTests (WindowsLifecycleEventsFireDuringStartup, WindowsLifecycleEventsFireInCorrectOrder, OnAppInstanceActivatedFiresExactlyOnce) Category=Lifecycle 🛠️ BUILD ERROR ⚠️ ENV ERROR
📱 WebAuthenticator_Tests WebAuthenticator_Tests 🛠️ BUILD ERROR ⚠️ ENV ERROR
📱 WebAuthenticator_Windows_Tests (CreateCallbackRouteKeyUsesSchemeOnly, CanRegisterCallbackRoutePreservesApplicationKeys, IsSameCallbackRouteUsesSchemeOnly) Category=WebAuthenticator 🛠️ BUILD ERROR ⚠️ ENV ERROR
🧪 WebUtils_Tests WebUtils_Tests ❌ PASS — 28s ✅ PASS — 13s
🔴 Without fix — 📱 LifecycleEventOrderTests (WindowsLifecycleEventsFireDuringStartup, WindowsLifecycleEventsFireInCorrectOrder, OnAppInstanceActivatedFiresExactlyOnce): 🛠️ BUILD ERROR · 172s

Error-relevant lines (filtered from the build log):

D:\a\1\s\src\Core\tests\DeviceTests\MauiProgram.cs(49,15): error CS1061: 'IWindowsLifecycleBuilder' does not contain a definition for 'OnAppInstanceActivated' and no accessible extension method 'OnAppInstanceActivated' accepting a first argument of type 'IWindowsLifecycleBuilder' could be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\src\Core\tests\DeviceTests\Core.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Core\tests\DeviceTests\LifecycleEventOrderTests.Windows.cs(16,44): error CS0117: 'WindowsLifecycle' does not contain a definition for 'OnAppInstanceActivated' [D:\a\1\s\src\Core\tests\DeviceTests\Core.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Core\tests\DeviceTests\LifecycleEventOrderTests.Windows.cs(27,71): error CS0117: 'WindowsLifecycle' does not contain a definition for 'OnAppInstanceActivated' [D:\a\1\s\src\Core\tests\DeviceTests\Core.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Core\tests\DeviceTests\LifecycleEventOrderTests.Windows.cs(54,86): error CS0117: 'WindowsLifecycle' does not contain a definition for 'OnAppInstanceActivated' [D:\a\1\s\src\Core\tests\DeviceTests\Core.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
Build FAILED.
🟢 With fix — 📱 LifecycleEventOrderTests (WindowsLifecycleEventsFireDuringStartup, WindowsLifecycleEventsFireInCorrectOrder, OnAppInstanceActivatedFiresExactlyOnce): ⚠️ ENV ERROR · 189s

No log file found

🔴 Without fix — 📱 WebAuthenticator_Tests: 🛠️ BUILD ERROR · 31s

Error-relevant lines (filtered from the build log):

D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(17,50): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'CreateCallbackRouteKey' [D:\a\1\s\src\Essentials\test\DeviceTests\Essentials.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(31,48): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'CanRegisterCallbackRoute' [D:\a\1\s\src\Essentials\test\DeviceTests\Essentials.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(43,48): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'IsSameCallbackRoute' [D:\a\1\s\src\Essentials\test\DeviceTests\Essentials.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
Build FAILED.
🟢 With fix — 📱 WebAuthenticator_Tests: ⚠️ ENV ERROR · 71s

No log file found

🔴 Without fix — 📱 WebAuthenticator_Windows_Tests (CreateCallbackRouteKeyUsesSchemeOnly, CanRegisterCallbackRoutePreservesApplicationKeys, IsSameCallbackRouteUsesSchemeOnly): 🛠️ BUILD ERROR · 28s

Error-relevant lines (filtered from the build log):

D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(17,50): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'CreateCallbackRouteKey' [D:\a\1\s\src\Essentials\test\DeviceTests\Essentials.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(31,48): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'CanRegisterCallbackRoute' [D:\a\1\s\src\Essentials\test\DeviceTests\Essentials.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(43,48): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'IsSameCallbackRoute' [D:\a\1\s\src\Essentials\test\DeviceTests\Essentials.DeviceTests.csproj::TargetFramework=net10.0-windows10.0.19041.0]
Build FAILED.
🟢 With fix — 📱 WebAuthenticator_Windows_Tests (CreateCallbackRouteKeyUsesSchemeOnly, CanRegisterCallbackRoutePreservesApplicationKeys, IsSameCallbackRouteUsesSchemeOnly): ⚠️ ENV ERROR · 34s

No log file found

🔴 Without fix — 🧪 WebUtils_Tests: PASS ❌ · 28s

(no coded error found; showing last 1200 chars)

ackUrl: "maui-auth://callback?code=123", expected: True) [< 1 ms]
  Passed Tests.WebUtils_Tests.CanHandleCallback_ReturnsExpected(expectedUrl: "maui-auth://callback", callbackUrl: "other-auth://callback?code=123", expected: False) [< 1 ms]
  Passed Tests.WebUtils_Tests.CanHandleCallback_ReturnsExpected(expectedUrl: "MAUI-AUTH://", callbackUrl: "maui-auth://callback?code=123", expected: True) [< 1 ms]
  Passed Tests.WebUtils_Tests.CanHandleCallback_ReturnsExpected(expectedUrl: "maui-auth://callback", callbackUrl: "MAUI-AUTH://CALLBACK?code=123", expected: True) [< 1 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_EncodedDotDot_HandledCorrectly [3 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_RootRequest_ReturnsEmpty [< 1 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_DifferentOrigin_ReturnsNull [< 1 ms]
[xUnit.net 00:00:00.45]   Finished:    Microsoft.Maui.Essentials.UnitTests
  Passed Tests.WebUtils_Tests.ResolveRelativePath_DoubleSlash_MakeRelativeUri_ProducesRooted_ReturnsNull [< 1 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_SubPath_ReturnsPath [< 1 ms]

Test Run Successful.
Total tests: 20
     Passed: 20
 Total time: 1.2032 Seconds

🟢 With fix — 🧪 WebUtils_Tests: PASS ✅ · 13s

(no coded error found; showing last 1200 chars)

ackUrl: "maui-auth://callback?code=123", expected: True) [< 1 ms]
  Passed Tests.WebUtils_Tests.CanHandleCallback_ReturnsExpected(expectedUrl: "maui-auth://callback", callbackUrl: "other-auth://callback?code=123", expected: False) [< 1 ms]
  Passed Tests.WebUtils_Tests.CanHandleCallback_ReturnsExpected(expectedUrl: "MAUI-AUTH://", callbackUrl: "maui-auth://callback?code=123", expected: True) [< 1 ms]
  Passed Tests.WebUtils_Tests.CanHandleCallback_ReturnsExpected(expectedUrl: "maui-auth://callback", callbackUrl: "MAUI-AUTH://CALLBACK?code=123", expected: True) [< 1 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_EncodedDotDot_HandledCorrectly [3 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_RootRequest_ReturnsEmpty [< 1 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_DifferentOrigin_ReturnsNull [< 1 ms]
[xUnit.net 00:00:00.42]   Finished:    Microsoft.Maui.Essentials.UnitTests
  Passed Tests.WebUtils_Tests.ResolveRelativePath_DoubleSlash_MakeRelativeUri_ProducesRooted_ReturnsNull [< 1 ms]
  Passed Tests.WebUtils_Tests.ResolveRelativePath_SubPath_ReturnsPath [< 1 ms]

Test Run Successful.
Total tests: 20
     Passed: 20
 Total time: 1.1236 Seconds

⚠️ Failure Details (7 tests)
  • 🛠️ LifecycleEventOrderTests (WindowsLifecycleEventsFireDuringStartup, WindowsLifecycleEventsFireInCorrectOrder, OnAppInstanceActivatedFiresExactlyOnce) without fix: build failed before tests could run
    • D:\a\1\s\src\Core\tests\DeviceTests\MauiProgram.cs(49,15): error CS1061: 'IWindowsLifecycleBuilder' does not contain a definition for 'OnAppInstanceActivated' and no accessible extension method 'OnApp...
  • 🛠️ WebAuthenticator_Tests without fix: build failed before tests could run
    • D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(17,50): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'CreateCallbackRouteKe...
  • 🛠️ WebAuthenticator_Windows_Tests (CreateCallbackRouteKeyUsesSchemeOnly, CanRegisterCallbackRoutePreservesApplicationKeys, IsSameCallbackRouteUsesSchemeOnly) without fix: build failed before tests could run
    • D:\a\1\s\src\Essentials\test\DeviceTests\Tests\Windows\WebAuthenticator_Windows_Tests.cs(17,50): error CS0117: 'WebAuthenticatorImplementation' does not contain a definition for 'CreateCallbackRouteKe...
  • WebUtils_Tests PASSED without fix (should fail) — tests don't catch the bug
  • ⚠️ LifecycleEventOrderTests (WindowsLifecycleEventsFireDuringStartup, WindowsLifecycleEventsFireInCorrectOrder, OnAppInstanceActivatedFiresExactlyOnce) with fix: You cannot call a method on a null-valued expression.
  • ⚠️ WebAuthenticator_Tests with fix: You cannot call a method on a null-valued expression.
  • ⚠️ WebAuthenticator_Windows_Tests (CreateCallbackRouteKeyUsesSchemeOnly, CanRegisterCallbackRoutePreservesApplicationKeys, IsSameCallbackRouteUsesSchemeOnly) with fix: You cannot call a method on a null-valued expression.
📁 Fix files reverted (11 files)
  • src/Controls/samples/Controls.Sample/MauiProgram.cs
  • src/Core/src/Hosting/EssentialsMauiAppBuilderExtensions.cs
  • src/Core/src/LifecycleEvents/Windows/WindowsLifecycle.cs
  • src/Core/src/LifecycleEvents/Windows/WindowsLifecycleBuilderExtensions.cs
  • src/Core/src/Platform/Windows/MauiWinUIApplication.cs
  • src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
  • src/Essentials/src/Platform/Platform.shared.cs
  • src/Essentials/src/Platform/PlatformMethods.windows.cs
  • src/Essentials/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
  • src/Essentials/src/WebAuthenticator/WebAuthenticator.shared.cs
  • src/Essentials/src/WebAuthenticator/WebAuthenticator.windows.cs

📱 UI Tests — Essentials,ViewBaseTests

Detected UI test categories: Essentials,ViewBaseTests

Deep UI tests — 115 passed, 1 failed across 2 categories on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category Tests Snapshot diffs
Essentials 0/1 (1 ❌)
ViewBaseTests 115/115 ✓
Essentials — 1 failed test
MicrophonePermissionCheckDoesNotCrash
System.TimeoutException : Timed out waiting for element...
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2757
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2784
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 793
   at Microsoft.Maui.TestCases.Tests.Issues.Issue32989.MicrophonePermissionCheckDoesNotCrash() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32989.cs:line 17
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at Sy
...
🔍 AI analysis of failures — PR-related vs unrelated

🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.

Likely unrelated: the failures appear pre-existing, flaky, or infrastructure.

  • ● Unrelated — Windows UI navigation/element timeout (~1 test): MicrophonePermissionCheckDoesNotCrash timed out before finding the page button, so it never reached the microphone permission or changed WebAuthenticator/lifecycle code paths, matching the common flaky "Timed out waiting for element" pattern rather than a PR-specific assertion failure.

Strongest signal: the PR touches Windows lifecycle/WebAuthenticator infrastructure but not the Issue32989 page or Permissions.Microphone, and the failure occurs before the test action.

📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)


📋 Pre-Flight — Context & Validation

Issue: #9973 - Simplify and make more robust the process for Single-instance WinUI apps
PR: #34883 - [Windows] Lifecycle: Add AppInstance activated event
Platforms Affected: Windows
Files Changed: 15 implementation, 4 test

Key Findings

  • PR adds a Windows lifecycle event exposing WinAppSDK AppActivationArguments, with handled-return semantics that can suppress normal launch flow.
  • Essentials/WebAuthenticator now participates in Windows AppInstance activation routing and stores process-wide route/authentication state.
  • The main correctness concern is synchronous RedirectActivationToAsync(...).GetResult() in WebAuthenticator and sample activation paths.
  • Prior test reviews repeatedly noted missing coverage for return-value suppression and later reactivation; current Windows startup order coverage is useful but incomplete.

Code Review Summary

Verdict: NEEDS_CHANGES
Confidence: low
Errors: 1 | Warnings: 2 | Suggestions: 0

Key code review findings:

  • Error src/Essentials/src/WebAuthenticator/WebAuthenticator.windows.cs:279: synchronous WinRT redirect wait can hang/deadlock when invoked from MAUI-dispatched activation callbacks.
  • Warning src/Controls/samples/Controls.Sample/MauiProgram.cs:332: sample demonstrates the same blocking redirect pattern.
  • Warning src/Core/src/Platform/Windows/MauiWinUIApplication.cs:91: subsequent activation may be delivered through both OnLaunched and AppInstance.Activated; duplicate delivery is not covered.

Fix Candidates

# Source Approach Test Result Files Changed Notes
PR PR #34883 Add public sync Windows OnAppInstanceActivated lifecycle hook and wire Essentials/WebAuthenticator to it. Gate INCONCLUSIVE (pre-run build/environment error) MauiWinUIApplication.cs, Windows lifecycle files, Essentials WebAuthenticator/platform files, tests, sample Original PR; code review found synchronous redirect risk.

🔬 Code Review — Deep Analysis

Code Review - PR #34883

Independent Assessment

What this changes: Adds a Windows OnAppInstanceActivated lifecycle hook that exposes WinAppSDK AppActivationArguments, invokes it during initial launch and later AppInstance.Activated callbacks, and wires Essentials/WebAuthenticator plus sample single-instancing through it.
Inferred motivation: Enable protocol/file/single-instance activation handling from MAUI lifecycle events instead of custom Windows entry-point plumbing.

Reconciliation with PR Narrative

Author claims: The PR adds WindowsLifecycle.OnAppInstanceActivated, raises it once services are available, honors its bool handled result to suppress launch flow, and uses it for Windows WebAuthenticator/follow-on activation scenarios.
Agreement/disagreement: The implementation matches the claimed API and startup ordering. The current redirect paths are not fully safe because they synchronously block on WinAppSDK async activation redirection from lifecycle/UI-dispatched paths.

Prior Review Reconciliation

Prior Error Finding Source Status Evidence
Android lifecycle device tests race OnStart/OnResume MauiBot major Obsolete Current diff no longer contains Android/iOS lifecycle test files; only LifecycleEventOrderTests.Windows.cs remains.
AppInstance.Activated invoked lifecycle callbacks off UI thread MauiBot major Fixed MauiWinUIApplication.cs:86-103 captures MAUI dispatcher and dispatches before OnAppInstanceActivated(args).
Missing Core Windows PublicAPI entries MauiBot major Fixed src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt:2-7 contains the new delegate, extension, invoke, and virtual method entries.
Windows order test could pass with missing events due raw IndexOf MauiBot moderate Fixed LifecycleEventOrderTests.Windows.cs:40-48 now asserts index >= 0 before comparisons.
WebAuthenticator app-owned key route can leave callback without route owner MauiBot major Partially unresolved / design-dependent RegisterCallbackRoute still silently returns for app-owned keys at WebAuthenticator.windows.cs:240-243.

Blast Radius Assessment

  • Runs for all instances: Yes for all Windows MAUI apps using Essentials lifecycle wiring; the new hook is invoked during startup and later activations.
  • Startup impact: Yes. MauiWinUIApplication.OnLaunched now calls OnAppInstanceActivated before OnLaunching/window creation.
  • Static/shared state: Yes. WebAuthenticator stores process-wide current auth state and AppInstance route ownership.

CI Status

  • Required-check result: undetermined via gh because GitHub CLI is unauthenticated in this environment.
  • Classification: undetermined / pending; user-provided gate result was inconclusive due build/environment error.
  • Action taken: Confidence capped low; no LGTM.

Findings

Error - WebAuthenticator blocks synchronously during activation redirection

src/Essentials/src/WebAuthenticator/WebAuthenticator.windows.cs:279

RedirectActivationAndExit calls routeOwner.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult(). This is reached from OnAppInstanceActivatedCallback, which is invoked through MAUI's Windows lifecycle. Later AppInstance.Activated callbacks are dispatched to the application dispatcher, so this can run on the UI thread of an already-running activation path. Blocking synchronously on a WinRT async operation from that thread risks a deadlock/hung activation instead of redirecting and exiting.

Warning - Sample demonstrates the same blocking redirect pattern

src/Controls/samples/Controls.Sample/MauiProgram.cs:332

Warning - Reactivation path may double-invoke lifecycle handlers for one activation

src/Core/src/Platform/Windows/MauiWinUIApplication.cs:91

Failure-Mode Probing

  • Protocol callback arrives while an app window is already active: The callback is dispatched to MAUI's dispatcher, then WebAuthenticator can synchronously block that dispatched lifecycle path on RedirectActivationToAsync.
  • Callback scheme belongs to another process route owner: RedirectActivationAndExit blocks before killing the transient process; if the async redirect stalls, the transient process remains alive.
  • App uses an app-owned AppInstance key: WebAuthenticator skips route registration. This is safe only if app code reliably redirects protocol activations to the auth-owning instance; otherwise the pending auth task can hang until cancellation.
  • Re-launch after _application exists: OnLaunched and AppInstance.Activated are both potential entry points; no current test covers duplicate delivery for this scenario.

Verdict: NEEDS_CHANGES

Confidence: low - Windows startup/lifecycle infrastructure plus pending/undetermined CI caps confidence.
Summary: The API shape and lifecycle wiring largely match the PR narrative, but the synchronous RedirectActivationToAsync(...).GetResult() in product code is a concrete activation hang/deadlock risk.


🛠️ Fix — Analysis & Comparison

Fix Candidates

# Source Approach Test Result Files Changed Notes
1 try-fix Non-blocking WebAuthenticator redirect continuation PARTIAL PASS - Windows Essentials build passed; tests not fully available 1 file Smallest change; avoids blocking but returns handled before redirect completion.
2 try-fix Public async Windows lifecycle API PARTIAL PASS - Windows Essentials/Core builds passed; tests not fully available 9 files Correct async semantics but too broad; adds public API and async lifecycle behavior.
3 try-fix Internal async pre-lifecycle WebAuthenticator hook PARTIAL PASS - Windows Core build passed; tests not fully available 7 files Avoids public API but may hide internally handled activations from public lifecycle observers.
4 try-fix Post-public internal async redirect hook PARTIAL PASS - Windows Core build passed; tests not fully available 7 files Best alternative: preserves public lifecycle fan-out, avoids public async API, and awaits redirect without blocking.
PR PR #34883 Public sync Windows OnAppInstanceActivated lifecycle hook plus synchronous WebAuthenticator redirect wait Gate INCONCLUSIVE (pre-run build/environment error) 19 files Original PR; code review found synchronous redirect hang/deadlock risk.

Cross-Pollination

Model Round New Ideas? Details
maui-expert-reviewer 1 Yes Candidate 1: non-blocking redirect continuation.
maui-expert-reviewer 2 Yes Candidate 2: public async Windows lifecycle API.
maui-expert-reviewer 3 Yes Candidate 3: internal async pre-lifecycle hook.
maui-expert-reviewer 4 Yes Candidate 4: public fan-out first, then internal async redirect hook.
maui-expert-reviewer 5 No NO NEW IDEAS: Candidate 4 is the best tradeoff; alternatives require app-wide single-instancing, public async lifecycle surface, or reduced public lifecycle visibility.

Exhausted: Yes
Selected Fix: Candidate #4 - It is the strongest alternative because it removes the synchronous WinRT wait, preserves public lifecycle observer visibility, avoids adding public async lifecycle API, and suppresses transient startup while the internal WebAuthenticator redirect completes. It is not fully gate-verified in this environment; available Windows builds passed.


📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current title is good, but the description should mention the winning reviewer-applied async redirect change and avoid implying the raw synchronous redirect implementation is final.

Recommended title

[Windows] Lifecycle: Add AppInstance activated event

Recommended description

### Root Cause

On Windows, MAUI exposed launch and window lifecycle hooks but not the underlying `AppActivationArguments` payload. That made single-instance redirect, protocol or file activation, and follow-on Windows auth work harder to handle cleanly from `MauiProgram` without app-specific plumbing.

### Description of Change

This PR adds a new Windows lifecycle hook:

- `WindowsLifecycle.OnAppInstanceActivated(UI.Xaml.Application, AppActivationArguments) -> bool`
- `IWindowsLifecycleBuilder.OnAppInstanceActivated(...)`

The name matches the underlying `AppInstance.GetCurrent().Activated` event.

`MauiWinUIApplication` now:

- captures the initial `AppInstance.GetActivatedEventArgs()` payload,
- registers once for `AppInstance.GetCurrent().Activated`,
- raises the new lifecycle event once MAUI services are available,
- respects the handler return value so apps can mark an activation as handled and suppress the default launch flow when appropriate.

The Controls sample demonstrates using this from `MauiProgram` to keep the app single-instanced, redirect later launches back into the running instance, and re-activate the existing MAUI window.

### Key Technical Details

- Initial activation is raised after `_services = applicationContext.Services` so lifecycle handlers can actually run.
- Redirected activations and later re-activations flow through the same `OnAppInstanceActivated` hook.
- Registration is guarded so `AppInstance.Activated` is only subscribed once.
- WebAuthenticator redirect routing uses an internal async unhandled-activation path so `RedirectActivationToAsync(...)` can be awaited instead of synchronously blocking the Windows UI/STA activation path.
- The public lifecycle event remains synchronous and observable first; the internal async WebAuthenticator routing only runs when public handlers do not handle the activation.

### Testing

Cross-platform lifecycle event order device tests added in `src/Core/tests/DeviceTests/`:

- **Windows**: validates `OnAppInstanceActivated` -> `OnLaunching` -> `OnWindowCreated` -> `OnLaunched` (order and exactly-once)
- **Android**: validates `OnCreate` -> `OnStart` -> `OnResume` (order and exactly-once)
- **iOS/MacCatalyst**: validates `FinishedLaunching` -> `OnActivated` (order and exactly-once)

These are real device tests that run in a platform app context, not console unit tests behind `#if WINDOWS`.

### Issues Fixed

Related to #9973

🏁 Report — Final Recommendation

Comparative Analysis — PR #34883

Candidates compared

Rank Candidate Test status Assessment
1 pr-plus-reviewer Not fully gate-verified; sandbox diff check passed Best candidate. Preserves the public PR API, applies the strongest try-fix-4 async internal redirect approach, and also fixes the sample's blocking redirect anti-pattern flagged by the expert reviewer.
2 try-fix-4 PARTIAL PASS — Windows Core build passed; full gate/device coverage unavailable Best STEP 5a alternative. It preserves public lifecycle visibility, avoids public async API expansion, and awaits WebAuthenticator redirect without blocking. Ranked below pr-plus-reviewer because it does not address the sample's unsafe blocking pattern.
3 try-fix-1 PARTIAL PASS — Windows Essentials build passed; full gate/device coverage unavailable Smallest product change and removes the direct blocking wait, but returns handled before redirect completion. That can suppress launch flow before the redirect outcome is known.
4 try-fix-3 PARTIAL PASS — Windows Core build passed; full gate/device coverage unavailable Avoids public async API and blocking, but can let internal WebAuthenticator handling short-circuit before public lifecycle observers see the activation. That weakens the new public lifecycle event's visibility.
5 try-fix-2 PARTIAL PASS — Windows Essentials/Core builds passed; full gate/device coverage unavailable Correctly models awaiting redirect completion, but expands public API surface with async lifecycle delegates for a narrow internal WebAuthenticator need. This is a broader API commitment than necessary.
6 pr Gate INCONCLUSIVE; expert review found NEEDS_CHANGES Raw PR implements the desired Windows lifecycle feature, tests, and WebAuthenticator routing, but leaves a critical synchronous WinRT redirect wait in product code and the same pattern in the sample.

No candidate had a confirmed regression-test failure. The supplied gate for the raw PR was inconclusive due build/environment errors and must not be treated as a failing fix. STEP 5a candidates had partial build evidence only, not complete Windows activation/device verification.

Why pr-plus-reviewer wins

The core tradeoff is whether to keep the PR's public synchronous lifecycle API while making the internal WebAuthenticator redirect path safe. pr-plus-reviewer does that: public OnAppInstanceActivated observers still run first and can handle/suppress launch flow, while MAUI then awaits an internal unhandled-activation hook for WebAuthenticator redirect routing. This avoids the critical UI/STA blocking risk without introducing a new public async lifecycle API.

Compared with the raw PR, pr-plus-reviewer fixes the highest-severity product issue (RedirectActivationToAsync(...).GetResult()) and the sample issue. Compared with try-fix-4, it is the same strong architectural approach plus the additional sample correction from the expert review. The unresolved duplicate-delivery concern is shared by the leading candidates and should be tracked as a follow-up coverage gap rather than a reason to prefer a weaker candidate.

Winning candidate

Winner: pr-plus-reviewer
Rationale: It preserves the PR's API and intended lifecycle semantics while applying the expert review's actionable async/threading feedback. It ranks above all STEP 5a candidates because it combines the best alternative (try-fix-4) with the additional sample fix, and no candidate has stronger verification evidence.


🧭 Next Steps — review latest findings

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

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 3 findings

See inline comments for details.

try
{
// Complete redirection before terminating this transient callback process.
routeOwner.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[critical] Async and Threading SafetyrouteOwner.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult() blocks synchronously. RedirectActivationAndExit is reached from OnAppInstanceActivatedCallback, which MauiWinUIApplication invokes either directly on the UI thread during OnLaunched (before window creation) or via HandleAppInstanceActivateddispatcher.Dispatch(() => OnAppInstanceActivated(args)) on the UI thread of an already-running app. Blocking that thread on a WinRT cross-process async call whose completion can be marshaled back onto the same thread's message queue (STA semantics) risks a real deadlock rather than the intended redirect+exit — Microsoft's own single-instancing guidance uses await inside async void OnLaunched for exactly this call. Recommend restructuring so this is awaited instead of blocked on with GetAwaiter().GetResult().

// The WinAppSDK single-instance guidance redirects the activation and then
// terminates the losing instance immediately. Using Kill here avoids leaving
// a headless process around that can continue to hold build outputs open.
keyInstance.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Async and Threading Safety — Same blocking pattern as the library code: keyInstance.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult() runs inside HandleWindowsAppInstanceActivated, invoked as a WindowsLifecycle.OnAppInstanceActivated handler on the UI thread (either directly from OnLaunched or dispatched from MauiWinUIApplication.HandleAppInstanceActivated). This is presented to app authors as recommended single-instancing boilerplate, but it teaches the same deadlock/hang-prone anti-pattern flagged in the product code instead of awaiting the redirect.

// Reuse the existing services and let activation handlers short-circuit the relaunch path.
if (_application != null && _services != null)
{
if (launchActivation is AppActivationArguments activatedEventArgs && OnAppInstanceActivated(activatedEventArgs))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[moderate] Regression Prevention and Test Coverage — When OnLaunched fires again for an already-running instance (_application != null && _services != null), this calls OnAppInstanceActivated(activatedEventArgs) directly using AppInstance.GetCurrent().GetActivatedEventArgs(), while AppInstance.GetCurrent().Activated (line 91, wired up on the prior launch) remains subscribed via HandleAppInstanceActivated. If WinAppSDK delivers the same reactivation through both paths, WindowsLifecycle.OnAppInstanceActivated handlers (including WebAuthenticator.OnAppInstanceActivatedCallback) can run twice for one logical activation with no de-duplication guard. LifecycleEventOrderTests.Windows.cs's new OnAppInstanceActivatedFiresExactlyOnce test only covers the initial single-launch case, not this already-running-instance reactivation path, so a double-invocation regression here would go undetected.

@MauiBot MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 15, 2026
@kubaflo

kubaflo commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

@kubaflo

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Tests Failure Analysis

@mattleibow — test-failure review results are available based on commit 144023a.
To request a fresh review after new comments, commits, or CI runs, comment /review tests.

Overall Not ready Failures 46 Regressed vs base 1 Baseline 2 on base

Test Failure Review: Not ready - click to expand

Overall verdict: Not ready. One WinUI failure (DatePickerOpenedAndClosedEventsAreRaised) is red on this PR but green across all 5 recent main base builds of maui-pr-uitests, making it a deterministic regression vs base — and this PR targets Windows lifecycle, so it is the prime suspect. The remaining ~43 failures are mostly a WinAppDriver-crash cascade and provisioning/timeout flakiness that appear intermittently on main; only 2 (platform-tools;35.0.2) also fail on the sampled base builds, and device tests (maui-pr-devicetests) were all clean.

Coverage: 162 checks · 150 passing · 12 failing · 0 pending · 0 inaccessible · 1 unmapped · 23 unexplained build legs · 0 unaccounted failing checks · 4 aborted failing checks · 0 canceled-build checks · 0 device-test unverified · 45 unattributed · 1 regressed-vs-base. Deterministic ceiling: Not ready — 1 leg regressed vs base and 45 failures could not be attributed deterministically.

Builds (this PR): maui-pr-uitests 1512263, maui-pr-devicetests 1512264. Base sampling (main, 5 recent builds per definition): 1503618, 1503332, 1503036.

Recommended action

Investigate DatePickerOpenedAndClosedEventsAreRaised (WinUI NRE) against this PR's Windows lifecycle changes, since it is a clean regression vs base; the WinAppDriver-crash cascade and provisioning flakiness need a human to confirm they are infra noise before merging.

@kubaflo

kubaflo commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🔍 This is an AI-generated comment posted on @kubaflo's behalf.

Review triage — post-#36597, head 144023a2

I re-checked all open review threads against the current head. Most trivial findings are already addressed by #36597; the remainder are false-positives or a design-sensitive cluster best left to the branch owner. Breakdown:

✓ Already addressed on the current head — these threads can be resolved

  • IndexOf == -1 false positives (LifecycleEventOrderTests.Windows.cs) — already guarded by the AssertEventIndex local helper, which does Assert.True(index >= 0, …) before any index comparison.
  • LifecycleEventLog never cleared (Core/tests/DeviceTests/MauiProgram.cs) — already cleared via LifecycleEventLog.Clear() at the top of CreateMauiApp(), so the "exactly once" asserts are reliable.
  • Public API baseline (WindowsLifecycleBuilderExtensions / MauiWinUIApplication) — the new surface is already recorded in src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt (WindowsLifecycle.OnAppInstanceActivated, the WindowsLifecycleBuilderExtensions.OnAppInstanceActivated(...) extension, and MauiWinUIApplication.OnAppInstanceActivated(...)).

ℹ False-positive / not applicable

  • configureBuilder non-nullable defaults to null (DeviceTests.Shared/MauiProgramDefaults.cs) — Nullable is disabled in Core.DeviceTests.Shared.csproj (the <Nullable>enable</Nullable> line is commented out), so = null emits no warning. Adding ? would instead risk CS8632 (annotation in a non-annotated context).

● Coupled to the threading work — resolve together, not separately

  • Unused using System.Threading.Tasks; (Controls.Sample/MauiProgram.cs:7) — it's tied to the blocking keyInstance.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult() at L332, which is itself one of the design-sensitive findings below. Removing the using in isolation is risky; best handled when L332 is revisited.

✗ Design-sensitive — needs the author's call (@IlGalvo, actively iterating via #36415/#36597)

These change the PR's core Windows-lifecycle / WebAuthenticator semantics, so they are intentionally not auto-fixed here:

  • [critical] WebAuthenticator.windows.cs:279 — blocking async on RedirectActivation….
  • [major] MauiWinUIApplication.cs:103AppInstance.Activated can fire off the UI thread; the handler needs explicit marshalling.
  • [major] Controls.Sample/MauiProgram.cs:332 — same blocking .AsTask().GetAwaiter().GetResult() pattern.
  • [major] WebAuthenticator.windows.cs callback/cancellation flow (L122 TrySetCanceled() drops the token; L89 redundant async/await forwarding overload).

CI status

  • maui-pr (main build) and maui-pr-devicetests are green on 144023a2.
  • maui-pr-uitests is red, but the failures spread across unrelated categories (Android Accessibility/ActionSheet/CollectionView, WinUI Cells/CheckBox/Dispatcher, macOS ListView) with no lifecycle/activation test among them — consistent with the known UI-test flakiness, not a regression from this change.

Merge blocker

Independent of everything above, the hard blocker is a sticky CHANGES_REQUESTED review from @kubaflo (2026-07-05) — the PR stays BLOCKED until that review is dismissed or converted.


Method: threads re-evaluated against head 144023a2; "already addressed" items verified by reading the current source, PublicAPI baseline, and csproj. Threading findings left for the author as they affect runtime semantics.

@IlGalvo

IlGalvo commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@IlGalvo Thank you sooo much for taking initiative in driving this PR to merge <3 If you want you can create a new PR and we close this one so that you don't have create new PRs in case of new suggestions

Absolutely! Thank you so much for the trust and for the opportunity. 😄 I’m already working on the new PR and I’ll link it here as soon as it’s ready. I’m really excited to help drive this forward and get it merged! 🚀

@kubaflo

kubaflo commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

closed in favour of #36640

@kubaflo kubaflo closed this Jul 18, 2026
kubaflo pushed a commit that referenced this pull request Jul 27, 2026
…tor support (#36640)

### Description of Change

.NET MAUI on Windows currently has two related gaps: it doesn't expose
the Windows App SDK `AppActivationArguments` through its lifecycle
events, and `WebAuthenticator` isn't supported on Windows. As a result,
single-instancing, protocol and file activation, and browser-based
authentication callbacks require additional app-specific plumbing.

This PR addresses both areas by adding:

- a Windows `OnAppInstanceActivated` lifecycle hook with handled
semantics;
- initial and subsequent `AppInstance` activation delivery through the
MAUI lifecycle;
- lifetime-safe UI dispatching for later activation callbacks;
- Windows `WebAuthenticator` support using the system browser, protocol
activation, callback routing, result completion, asynchronous activation
redirection, and best-effort window foreground activation;
- packaged manifest and unpackaged protocol-registration validation;
- a Controls Sample implementation demonstrating single-instancing and
activation redirection;
- Windows lifecycle, WebAuthenticator routing, and callback validation
tests.

The implementation keeps the existing cross-platform `WebAuthenticator`
contract and uses the established `Launcher` + callback route + pending
task model.

### Related Work

This PR fixes #2702 and is also related to #9973.

It is a fresh, self-contained alternative to:

#34883, which introduces the Windows AppInstance lifecycle hook;
#34887, which adds Windows WebAuthenticator support as a stacked
OAuth2Manager-based follow-up to #34883.

This version combines both areas in one PR based on the current
`net11.0` branch while preserving the existing MAUI WebAuthenticator
model.

Related to #9973.

### Testing

- Debug and Release x64 builds passed for Core, Essentials, Controls
Sample, Core DeviceTests, and Essentials DeviceTests.
- `WebUtils_Tests`: 20/20 passed.
- Windows WebAuthenticator routing helpers: 14/14 passed.
- Windows lifecycle device tests: 3/3 passed.
- Public API validation passed.
- Controls Sample single-instance redirection was manually validated,
including suspending the route owner and resuming it after a second
instance attempted redirection.
- Packaged and framework-dependent unpackaged browser/protocol callback
transports completed successfully and returned to the existing runner
without leaving a second window.
- Application-owned routing completed `owned-ok` in the main process
while preserving the `Pr36640.AppOwned` key; the transient process
exited and the main process remained alive.
- Sequential authentications using `pr36640-a` and `pr36640-b` both
completed in one process, with the route key updating to
`Microsoft.Maui.WebAuthenticator:pr36640-b`.
- A focused compatibility harness confirmed the original abstract-member
break (`CS0535`), source and binary compatibility with the default
member, a default result of `false`, and an overriding result of `true`.
- The public test endpoint returned its literal `ACCESS_TOKEN_VALUE`
placeholder instead of `testtokenvalue`, so transport completed but that
external-data assertion remained unsuitable as a deterministic pass
criterion.

### Review Notes

- **Asynchronous activation redirect:** the lifecycle callback now marks
the transient activation handled immediately and starts an asynchronous
helper. The helper awaits `RedirectActivationToAsync` and terminates the
transient process only after the native operation completes or fails,
removing the previous sync-over-async wait from the activation path.
- **Why there is no destructive timeout:** a five-second `WaitAsync`
followed by process termination was tested with the route owner
suspended. The transient exited at about 5.4 seconds, but resuming the
owner produced an unhandled Windows App Runtime failure in
`RedirectionRequest.cpp` (`0x8003001E`, `wil::ResultException`). The
managed timeout does not safely cancel the pending native redirection.
This matches the Windows App SDK guidance that the caller must await
redirection before exiting:
https://learn.microsoft.com/windows/apps/windows-app-sdk/applifecycle/applifecycle-instancing
- **Redirected activation lifetime:** Windows App SDK keeps the
redirected activation request alive through the synchronous
`AppInstance.Activated` event. MAUI now waits for the dispatched UI
lifecycle handlers to finish before that event callback returns. This
prevents the redirecting process from releasing marshaled resources
before the UI consumes the activation, while preserving lifecycle
exception behavior and avoiding a destructive timeout.
- **Public interface compatibility:** the new Windows member on the
already-shipped `IPlatformWebAuthenticatorCallback` has a default
implementation returning `false`. Existing custom implementers therefore
remain source- and binary-compatible, while implementations that opt in
can override it.
- **Warm activation delivery:** the same activation might theoretically
reach both `OnLaunched` and `AppInstance.Activated`. This has not been
reproduced and should be verified before adding deduplication.
- **Application-owned `AppInstance` key:** when an app owns the key, it
also owns activation routing and must redirect protocol callbacks to the
instance that started authentication. The Controls Sample now calls this
out explicitly. A general fail-fast cannot distinguish a complete
cooperative router from an incomplete one without rejecting the
supported app-owned scenario.
- **Sequential callback schemes:** Windows App SDK 1.8 can rebind the
current instance to a different key without calling `UnregisterKey`; the
tested A-to-B flow completed successfully. MAUI intentionally avoids
`UnregisterKey` because registering another key after it is unsafe
(`microsoft/WindowsAppSDK#4420`).

Thank you @kubaflo for suggesting a fresh PR and for the previous
reviews.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p/0 Current heighest priority issues that we are targeting for a release. s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants