Make closing winapp's Sandbox window end the Sandbox, after Windows asks

docs/sandbox-execution.md

@@ -13,7 +13,8 @@ winapp ui invoke --on sandbox SubmitButton -a MyApp
 
 Replace `MyApp` with your app name or the guest PID printed by `run`. `--detach` returns
 after launch so the next command can inspect the app; without it, `run` waits for the
-app to exit. The Sandbox stays running between commands and rebuilds.
+app to exit. The Sandbox stays running between commands and rebuilds until it is ended; see
+[Removing an app and ending the Sandbox](#removing-an-app-and-ending-the-sandbox).
 
 ## Before you start
 
@@ -36,9 +37,10 @@ it stops with setup instructions; an observed pending Windows restart is reporte
 
 A cold connection or reconnect can briefly take focus. Once connected, winapp puts
 the Sandbox window behind the window you were using, without activating it; switch to
-**Windows Sandbox** in the taskbar to watch the guest. Unlike a Sandbox opened from
-Start, closing this window does not end the Sandbox; it keeps running. To see it again,
-run `wsb list`, then `wsb connect --id <id>`. To end it, see
+**Windows Sandbox** in the taskbar to watch the guest. When winapp started the Sandbox,
+closing that window ends it, just like a Sandbox opened from Start: Windows first asks you
+to confirm, because everything in it is discarded. The next `--on sandbox` command starts a
+fresh one. A Sandbox started with `wsb start` keeps running when its window is closed. See
 [Removing an app and ending the Sandbox](#removing-an-app-and-ending-the-sandbox).
 
 > [!IMPORTANT]
@@ -335,17 +337,18 @@ An externally installed package is left alone, even if its identity matches.
 A `.cs` input isn't supported with `--on`; pass its manifest instead. Unpackaged apps have no
 package registration to remove.
 
-The Sandbox remains running, even after you close its window. Manage its lifetime with
-Windows Sandbox's own CLI:
+To end the Sandbox, close its window and confirm. If winapp is using a Sandbox that was
+started with `wsb start`, for example by another tool or by winapp 0.7.1 or earlier,
+closing its window leaves it running. Manage any Sandbox with Windows Sandbox's own CLI:
 
 ```powershell
 wsb list
 wsb connect --id <id>
 wsb stop --id <id>
 ```
 
-Stopping discards the guest and its work. Save needed evidence first, and obtain the
-user's consent before stopping an instance they may be using. Later winapp commands
+Ending a Sandbox discards the guest and its work. Save needed evidence first, and obtain the
+user's consent before ending an instance they may be using. Later winapp commands
 can create a fresh Sandbox; rediscover all app targets afterwards.
 
 ## Troubleshooting
@@ -366,7 +369,7 @@ copying a suggestion keeps it on the same execution target.
 | `sandbox_setup_incomplete` | Open Windows Sandbox from Start and finish client setup/update, then retry |
 | `sandbox_unmanaged_instance`, `sandbox_target_ambiguous` | Inspect the reported instances/windows; do not stop unrelated work to resolve ambiguity |
 | `sandbox_input_not_ready`, `sandbox_no_interactive_session` | Restore the existing client or reconnect as directed, then retry |
-| `sandbox_agent_incompatible` | Follow the version error; upgrade the installed CLI using its installation method if requested. If it asks you to stop the Sandbox, save any work you need, then run `wsb stop --id <id>` (with consent) and retry; closing its window does not stop it |
+| `sandbox_agent_incompatible` | Follow the version error; upgrade the installed CLI using its installation method if requested. If it asks you to stop the Sandbox, save any work you need, then run `wsb stop --id <id>` (with consent) and retry; it works however the Sandbox was started |
 | `sandbox_transport_failed` | Follow the error's `userAction`. If a folder share failed with `0x80070005`, Windows Sandbox (which shares folders as the SYSTEM account) can't read the reported folder; grant access, then retry: `icacls "<folder>" /grant "*S-1-5-18:(OI)(CI)F"` (`S-1-5-18` is SYSTEM) |
 | `sandbox_agent_busy` | Wait for another command to finish, then retry |
 | `sandbox_terminated`, `sandbox_target_stale`, `sandbox_stale_handle` | Rerun the app and rediscover guest PIDs/windows |

plugins/winapp/skills/winapp-sandbox/SKILL.md

@@ -27,8 +27,9 @@ description: Run, debug, and UI-automate a Windows app in a persistent Windows S
   and finish its client setup/update. Do not repeatedly retry an unchanged prerequisite.
 - Connection or reconnect may briefly take focus; do not promise zero desktop interruption.
 - Existing Sandbox instances are reused and changed, not discarded. Never close one
-  or run `wsb stop` without user consent. Closing the Sandbox window does not end the
-  Sandbox; `wsb connect --id <id>` shows it again.
+  or run `wsb stop` without user consent. Closing the window of a Sandbox winapp started
+  asks for confirmation, then ends it and discards its state; the next command starts a
+  fresh one. A Sandbox started with `wsb start` keeps running when its window is closed.
 
 ## Launch, inspect, act, verify
 
@@ -163,7 +164,7 @@ suppressed with `--quiet`/`--json`.
   window, `wsb list` then `wsb connect --id <id>`. Never stop it without consent.
 - Incompatible CLI: follow the error; upgrade the installed CLI through its install method,
   **not `winapp update`**. Obtain consent before stopping a Sandbox (`wsb stop --id <id>`)
-  for a version change; closing its window does not stop it.
+  for a version change; it works however the Sandbox was started.
 - Missing/unsupported runtime: use the named requirement and configuration in the error.
   Do not assume any newer same-major runtime is compatible or substitute architectures.
 - Folder share refused (`sandbox_transport_failed`, `0x80070005`): Sandbox shares as SYSTEM.

src/winapp-CLI/WinApp.Cli.Tests/SandboxAdoptionTests.cs

@@ -683,6 +683,31 @@ public async Task ClosedClient_WhoseReconnectFails_SurfacesThatFailure()
         Assert.AreEqual(ExecutionTargetErrorCodes.NoInteractiveSession, failure.Error.Code);
     }
 
+    /// <summary>
+    /// When winapp's window loses the singleton to a Sandbox started headless, that Sandbox is
+    /// treated like any other adopted one, so it gets the one window it is missing.
+    /// </summary>
+    /// <remarks>
+    /// Measured live: a <c>wsb start</c> in the same moment as winapp's launch wins, winapp's window
+    /// shows "Only one running instance of Windows Sandbox is allowed", and the listed Sandbox has no
+    /// window at all. Assuming a window was attached made the agent wait out its full deadline and
+    /// fail.
+    /// </remarks>
+    [TestMethod]
+    public async Task OpenedSandboxWhoseWindowLostTheRace_IsConnectedWhenNobodyIsAttached()
+    {
+        using var harness = new AdoptionHarness();
+        harness.Cli.LaunchLosesToWindowlessSandbox = true;
+        harness.Cli.Session = GuestSessionAvailability.NoLoginSession;
+
+        await harness.RunUntilAgentLaunchAsync(TestContext.CancellationToken);
+
+        Assert.AreEqual(nameof(SandboxInstanceOrigin.Adopted), harness.ReadState()!.InstanceOrigin);
+        Assert.AreEqual(
+            1,
+            harness.Cli.Operations.Count(op => op.StartsWith("connect:", StringComparison.Ordinal)));
+    }
+
     [TestMethod]
     public async Task AdoptedInstance_WithAHealthyClient_IsNeverReconnected()
     {
@@ -699,48 +724,37 @@ public async Task AdoptedInstance_WithAHealthyClient_IsNeverReconnected()
     }
 
     /// <summary>
-    /// An instance winapp started itself connects unless the guest positively says it has a session.
+    /// A Sandbox winapp opened, but whose window it cannot prove is its own, is used as adopted and
+    /// never given a second window.
     /// </summary>
     /// <remarks>
-    /// <c>wsb start</c> attaches no client, so for a Created or RecoveredStart instance the absence
-    /// of one is known, not guessed. Skipping the connect on an inconclusive probe would leave the
-    /// agent — which runs as <c>ExistingLogin</c> — with no session to launch into, and the whole
-    /// heartbeat window would be spent discovering that.
+    /// That happens when winapp's window is slow to show a session, or when someone opens Windows
+    /// Sandbox from Start in the same moment. Either way a window is already attached, winapp's or
+    /// the one Start opened, and the guest may still be signing in, so even "no login session" does
+    /// not mean nobody is there. A <c>wsb connect</c> would put a second window on screen.
     /// </remarks>
     [TestMethod]
+    [DataRow((int)GuestSessionAvailability.Ready, DisplayName = "guest has a session")]
     [DataRow((int)GuestSessionAvailability.Unknown, DisplayName = "probe could not answer")]
-    [DataRow((int)GuestSessionAvailability.NoLoginSession, DisplayName = "probe confirmed no session")]
-    public async Task CreatedInstance_ConnectsUnlessTheGuestSaysItHasASession(int session)
+    [DataRow((int)GuestSessionAvailability.NoLoginSession, DisplayName = "still signing in")]
+    public async Task OpenedSandboxWithAnUnidentifiedWindow_IsNeverGivenASecondWindow(int session)
     {
         using var harness = new AdoptionHarness();
         harness.Cli.Session = (GuestSessionAvailability)session;
 
         await harness.RunUntilAgentLaunchAsync(TestContext.CancellationToken);
 
-        Assert.AreEqual(
-            1,
-            harness.Cli.Operations.Count(op => op.StartsWith("connect:", StringComparison.Ordinal)),
-            "A Sandbox winapp started headless needs exactly one client.");
-    }
-
-    [TestMethod]
-    public async Task CreatedInstance_WhoseGuestAlreadyHasASession_IsNotConnected()
-    {
-        // The client installer can open a Sandbox that winapp then recovers; if a session already
-        // exists, adding another client would duplicate it.
-        using var harness = new AdoptionHarness();
-        harness.Cli.Session = GuestSessionAvailability.Ready;
-
-        await harness.RunUntilAgentLaunchAsync(TestContext.CancellationToken);
-
+        Assert.AreEqual(nameof(SandboxInstanceOrigin.Adopted), harness.ReadState()!.InstanceOrigin);
+        Assert.IsFalse(harness.ReadState()!.ClientOwnedByWinapp);
         Assert.IsFalse(
-            harness.Cli.Operations.Any(op => op.StartsWith("connect:", StringComparison.Ordinal)));
+            harness.Cli.Operations.Any(op => op.StartsWith("connect:", StringComparison.Ordinal)),
+            "The Sandbox this command opened already has a window; a second must not be connected.");
     }
 
     [TestMethod]
     public async Task RecoveredInstance_WithAnInconclusiveProbe_IsConnected()
     {
-        // Recovered from winapp's own unconfirmed start, so it was started headless too.
+        // Recovered from an older winapp's unconfirmed `wsb start`, so it was started headless.
         using var harness = new AdoptionHarness();
         harness.Cli.SetRunning(RecoveredInstanceId);
         harness.MarkPendingStart(RecoveredInstanceId);
@@ -919,13 +933,27 @@ public Task<IReadOnlyList<string>> ListAsync(CancellationToken cancellationToken
             return Task.FromResult<IReadOnlyList<string>>([.. _running]);
         }
 
-        public Task<string> StartAsync(string instanceId, string? configuration, CancellationToken cancellationToken)
+        public Task<SandboxConnectAttempt> LaunchAsync(
+            Action<SandboxConnectAttempt> onLaunched,
+            CancellationToken cancellationToken)
         {
-            Operations.Add($"start:{instanceId}");
-            _running.Add(instanceId);
-            return Task.FromResult(instanceId);
+            Operations.Add($"launch:{LaunchedId}");
+            _running.Add(LaunchedId);
+            var attempt = SandboxConnectAttempt.ForLauncher(LauncherProcessId, LauncherStartTicks);
+            attempt.LostToWindowlessSandbox = LaunchLosesToWindowlessSandbox;
+            onLaunched(attempt);
+            return Task.FromResult(attempt);
         }
 
+        /// <summary>
+        /// When set, the window winapp launches shows "Only one running instance of Windows Sandbox
+        /// is allowed" and no other Sandbox window is open: a headless Sandbox won the singleton.
+        /// </summary>
+        public bool LaunchLosesToWindowlessSandbox { get; set; }
+
+        /// <summary>The ID Windows gives the Sandbox a launched window opens.</summary>
+        public string LaunchedId { get; set; } = "sandbox-launched";
+
         public Task StopAsync(string id, CancellationToken cancellationToken)
         {
             Stopped.Add(id);

src/winapp-CLI/WinApp.Cli.Tests/SandboxLiveAdoptionTests.cs

@@ -150,13 +150,12 @@ public async Task ManuallyStartedSandbox_IsAdoptedWithoutLosingWhatIsInIt()
 
         var cli = CreateCli();
 
-        // Started exactly as a user would, and deliberately not through winapp: no state record
-        // exists for it, so winapp must treat it as an instance it did not create.
-        var manualId = WindowsSandboxLifecycle.GenerateInstanceId();
-        var reportedId = await cli.StartAsync(manualId, configuration: null, timeout.Token);
+        // Started the way another tool, or an older winapp, starts one, and deliberately not through
+        // winapp's lifecycle: no state record exists for it, so winapp must treat it as an instance
+        // it did not create.
+        var manualId = await StartWithWsbAsync(timeout.Token);
         _startedByThisTest = manualId;
 
-        Assert.AreEqual(manualId, reportedId, "wsb start must honour the caller-assigned instance ID.");
         CollectionAssert.Contains(
             (await cli.ListAsync(timeout.Token)).ToArray(),
             manualId,
@@ -265,36 +264,19 @@ await cli.ExecuteAsync(
         Assert.AreEqual(manualId, second.DescribeForDiagnostics()["sandboxId"]);
     }
 
-    /// <summary>
-    /// <c>wsb start --id</c> honours the caller's GUID, which is what makes recovery possible.
-    /// </summary>
-    /// <remarks>
-    /// The whole partial-start recovery design rests on this: winapp writes down an ID before
-    /// starting, and reconciles that exact ID afterwards. If <c>wsb</c> ever stopped honouring it,
-    /// recovery would silently degrade into guessing from a list, so it is verified against the real
-    /// tool rather than assumed.
-    /// </remarks>
-    [TestMethod]
-    public async Task WsbStart_HonoursTheCallerAssignedInstanceId()
+    /// <summary>Starts a headless Sandbox with <c>wsb start</c> and returns its ID.</summary>
+    private static async Task<string> StartWithWsbAsync(CancellationToken cancellationToken)
     {
-        await SkipUnlessTheMachineIsFreeAsync();
-
-        using var timeout = CancellationTokenSource.CreateLinkedTokenSource(TestContext.CancellationToken);
-        timeout.CancelAfter(CommandTimeout);
-
-        var cli = CreateCli();
-        var assignedId = WindowsSandboxLifecycle.GenerateInstanceId();
-
-        var reportedId = await cli.StartAsync(assignedId, configuration: null, timeout.Token);
-        _startedByThisTest = assignedId;
-
-        Assert.AreEqual(assignedId, reportedId);
+        var wsb = WindowsSandboxHostProbe.ResolveTrustedAlias()
+            ?? throw new AssertInconclusiveException("wsb.exe is not available.");
+        var instanceId = Guid.NewGuid().ToString("D");
 
-        await WaitUntilResolvableAsync(cli, assignedId, timeout.Token);
+        var result = await new ProcessRunner().RunAsync(
+            new ProcessRunRequest(wsb, ["start", "--id", instanceId, "--raw"]),
+            cancellationToken: cancellationToken);
 
-        Assert.IsTrue(
-            await cli.IsResolvableAsync(assignedId, timeout.Token),
-            "An instance winapp claims must be reachable before anything is prepared in it.");
+        Assert.AreEqual(0, result.ExitCode, $"wsb start failed: {result.StandardError}");
+        return instanceId;
     }
 
     /// <summary>Puts a file and a running process into the guest before winapp sees it.</summary>

src/winapp-CLI/WinApp.Cli.Tests/SandboxUxRegressionTests.cs

@@ -220,19 +220,34 @@ await harness.RunUntilAgentLaunchAsync(
             "A guest whose bootstrap never finished has no interactive session yet.");
     }
 
-    /// <summary>A first bootstrap still connects the client, because there is no session yet.</summary>
+    /// <summary>
+    /// A first bootstrap uses the window that opened the Sandbox and never connects a second one.
+    /// </summary>
+    /// <remarks>
+    /// winapp opens a new Sandbox the way Start does, so its window owns it: closing it asks the user,
+    /// then ends the Sandbox. A <c>wsb connect</c> on top would put a second window on screen, and
+    /// closing that one would leave the Sandbox running.
+    /// </remarks>
     [TestMethod]
-    public async Task FirstBootstrap_StillConnectsTheClient()
+    public async Task FirstBootstrap_UsesTheWindowThatOpenedTheSandbox()
     {
-        using var harness = new BackendHarness();
+        var client = new SandboxClientWindow((nint)0x2468, 4343, 1_000_000);
+        var windows = new PlacedWindowController(client);
+        using var harness = new BackendHarness(windows);
 
         await harness.RunUntilAgentLaunchAsync(
             TestContext.CancellationToken,
-            requireInteractiveDesktop: false);
+            requireInteractiveDesktop: true);
 
-        Assert.IsTrue(
+        Assert.AreEqual(1, harness.Cli.Operations.Count(op => op == "launch"));
+        Assert.IsFalse(
             harness.Cli.Operations.Any(op => op.StartsWith("connect", StringComparison.Ordinal)),
-            "A brand-new instance has no interactive session until a client connects.");
+            "The window that opened the Sandbox is its session; a second window must not be connected.");
+        Assert.IsFalse(
+            harness.Cli.Operations.Contains("probe-session"),
+            "A just-opened window is still signing in, so the session probe cannot say anything useful.");
+        Assert.AreEqual(client, harness.ReadClient(), "The owning window is recorded as winapp's.");
+        Assert.AreEqual(client, windows.Placed, "The owning window is moved behind the user's window.");
     }
 
     [TestMethod]
@@ -241,6 +256,7 @@ public async Task ConnectCancellationAfterPlacement_PersistsTheParkedClient()
         var client = new SandboxClientWindow((nint)0x1234, 4321, 638_900_000_000_000_000);
         var windows = new PlacedWindowController(client);
         using var harness = new BackendHarness(windows);
+        harness.MarkInstanceOwnedButNeverBootstrapped();
         harness.Cli.ConnectFailure = new OperationCanceledException(new CancellationToken(canceled: true));
 
         await Assert.ThrowsAsync<OperationCanceledException>(
@@ -258,6 +274,7 @@ public async Task ConnectFastFailureAfterPlacement_PersistsTheParkedClient()
         var client = new SandboxClientWindow((nint)0x5678, 8765, 638_900_000_100_000_000);
         var windows = new PlacedWindowController(client);
         using var harness = new BackendHarness(windows);
+        harness.MarkInstanceOwnedButNeverBootstrapped();
         harness.Cli.ConnectFailure = ExecutionTargetException.Create(
             ExecutionTargetErrorCodes.NoInteractiveSession,
             "The Windows Sandbox client could not connect.");
@@ -804,11 +821,18 @@ public void SetRunning(params string[] ids)
         public Task<IReadOnlyList<string>> ListAsync(CancellationToken cancellationToken) =>
             Task.FromResult<IReadOnlyList<string>>([.. _running]);
 
-        public Task<string> StartAsync(string instanceId, string? configuration, CancellationToken cancellationToken)
+        /// <summary>The ID Windows gives the Sandbox a launched window opens.</summary>
+        public string LaunchedId { get; set; } = "sandbox-launched";
+
+        public Task<SandboxConnectAttempt> LaunchAsync(
+            Action<SandboxConnectAttempt> onLaunched,
+            CancellationToken cancellationToken)
         {
-            Operations.Add("start");
-            _running.Add(instanceId);
-            return Task.FromResult(instanceId);
+            Operations.Add("launch");
+            _running.Add(LaunchedId);
+            var attempt = SandboxConnectAttempt.ForLauncher(4343, 1_000_000);
+            onLaunched(attempt);
+            return Task.FromResult(attempt);
         }
 
         public Task<bool> IsResolvableAsync(string id, CancellationToken cancellationToken) =>