fix(streaming): refresh the process identity when Apollo restarts

src/MultiSeat.Service/Streaming/ApolloManager.cs

@@ -176,7 +176,12 @@ public void KillForReconnect(SeatInfo seat)
         }
 
         // Reset restart count — a sleep reconnect is not a crash
-        _instances[seat.Id] = instance with { ProcessId = 0, RestartCount = 0 };
+        // Clear the identity along with the PID. Leaving it behind describes a process that has
+        // just been killed, and the record would then claim an identity it does not have. It
+        // fails closed rather than dangerously — a stale identity matches nothing — but a record
+        // that contradicts itself is exactly what made the restart path wrong.
+        _instances[seat.Id] = instance with { ProcessId = 0, RestartCount = 0, Identity = null };
+        seat.ApolloIdentity = null;
     }
 
     /// <summary>
@@ -198,8 +203,18 @@ public void Stop(SeatInfo seat)
             return;
         }
 
-        // The instance record is gone — the normal state after a service restart. The seat itself
-        // still carries the identity, so this stays a verified kill rather than a hopeful one.
+        // Second source for the identity, when the instance record has none.
+        //
+        // ⚠️ CORRECTION. This branch was originally justified by "after a service restart
+        // _instances is empty while the seat is still alive". That cannot happen: seats are held
+        // in memory only, with no persistence and no restore, so _seats and _instances are
+        // populated together and lost together. A restarted service has no SeatInfo to call this
+        // with in the first place.
+        //
+        // It is kept because it is cheap, correct, and the honest fallback if either store ever
+        // gains independent lifetime — not because the scenario above occurs today. The two are
+        // now written together at every site (start, restart, reconnect-kill), so they cannot
+        // disagree.
         if (seat.ApolloIdentity is { } seatIdentity)
         {
             var outcome = TryKillIdentifiedProcess(
@@ -250,16 +265,40 @@ public async Task<int> RestartAsync(SeatInfo seat, CancellationToken ct)
 
         if (pid > 0)
         {
+            // ⛔ The identity MUST be re-read here. Carrying `prev`'s forward alongside a new
+            // ProcessId produces a record that contradicts itself, and both readers then fail in
+            // dangerous directions:
+            //
+            //   IsAlive  compares the new ProcessId against the OLD identity's, so a perfectly
+            //            healthy restarted Apollo reports DEAD — and SessionHealthCheck restarts
+            //            it again, forever, until MaxRestartAttempts.
+            //   Stop     kills using the OLD identity, finds that PID long gone, reports
+            //            AlreadyGone, and never touches the Apollo that is actually running —
+            //            leaking it on every teardown that follows a restart.
+            //
+            // Found by runtime-testing a real provision/teardown cycle; no unit test caught it.
+            var restartedAt = GetProcessStartTime(pid);
+            if (restartedAt is null)
+            {
+                _logger.LogWarning(
+                    "Seat {Id}: Apollo restarted (PID {Pid}) but its start time could not be " +
+                    "read — no PID-reuse protection for this instance", seat.Id, pid);
+            }
+
+            ProcessIdentity? identity = restartedAt is { } t ? new ProcessIdentity(pid, t) : null;
+
             _instances[seat.Id] = prev with
             {
                 ProcessId = pid,
                 StartedAt = DateTimeOffset.UtcNow,
                 RestartCount = prev.RestartCount + 1,
                 SessionId = seat.SessionId,
-                AccountName = seat.AccountName
+                AccountName = seat.AccountName,
+                Identity = identity
             };
 
             seat.ApolloProcessId = pid;
+            seat.ApolloIdentity = identity;
             _logger.LogInformation(
                 "Seat {Id}: Apollo restarted (PID {Pid})", seat.Id, pid);
         }

src/MultiSeat.Shared/Models/SeatInfo.cs

@@ -38,15 +38,24 @@ public sealed class SeatInfo
     /// <summary>
     /// The identity — PID plus the OS-reported start time — of the Apollo this seat launched.
     ///
-    /// ⭐ This is what makes a kill safe when <c>ApolloManager</c>'s in-memory instance record is
-    /// gone, which is exactly the state after a service restart. Without it the only survivor is
-    /// <see cref="ApolloProcessId"/>, a bare number Windows is free to have handed to something
-    /// else in the meantime, and terminating on that alone can kill an unrelated process tree.
-    /// PR B narrowed that path to a process-name check; carrying the identity here closes it.
+    /// It is the seat's own record of which process it owns, so a client can see it and a kill
+    /// has a second source to verify against. <see cref="ApolloProcessId"/> alone is a bare
+    /// number Windows is free to have handed to something else, and terminating on that can kill
+    /// an unrelated process tree.
     ///
-    /// Null when the start time could not be read at launch. ⛔ Never populate it with a
-    /// substitute timestamp: an identity carrying a made-up time can compare equal to a recycled
-    /// PID by coincidence, which is worse than having no identity at all.
+    /// ⚠️ This was first justified as covering "the instance record is gone after a service
+    /// restart, but the seat survives". That is NOT true — seats are in-memory only, with no
+    /// persistence and no restore, so the seat and the instance record are lost together. The
+    /// field is genuinely useful; that particular argument for it was wrong.
+    ///
+    /// ⛔ It must be rewritten wherever <see cref="ApolloProcessId"/> is, and cleared wherever
+    /// that is cleared. A restart that advanced the PID while leaving this pointing at the dead
+    /// process made <c>IsAlive</c> report a healthy Apollo as dead and made teardown leak the
+    /// live one.
+    ///
+    /// Null when the start time could not be read. ⛔ Never populate it with a substitute
+    /// timestamp: an identity carrying a made-up time can compare equal to a recycled PID by
+    /// coincidence, which is worse than having no identity at all.
     /// </summary>
     public ProcessIdentity? ApolloIdentity { get; set; }
 

src/MultiSeat.Tests/Streaming/ProcessIdentityTests.cs

@@ -247,6 +247,64 @@ public void Stop_WithAStaleIdentityOnTheSeat_LeavesTheProcessRunning()
         }
     }
 
+    // ── the record must never contradict itself ─────────────────────────────────
+
+    [Fact]
+    public void AnInstanceWhoseIdentityNamesADifferentPid_ReportsDead()
+    {
+        // This is the shape RestartAsync used to produce: a NEW ProcessId carried alongside the
+        // PREVIOUS identity. Both readers then fail, in opposite and dangerous directions —
+        // IsAlive calls a healthy Apollo dead (restart loop), and Stop kills the old PID and
+        // leaks the live one. The test documents WHY the two fields must be written together.
+        using var victim = StartVictim();
+        try
+        {
+            var realStart = ApolloManager.GetProcessStartTime(victim.Id);
+            Assert.NotNull(realStart);
+
+            var contradictory = new ApolloInstance(
+                SeatId: Guid.NewGuid(),
+                ProcessId: victim.Id,                                  // the live process
+                ConfigPath: "x", SessionId: 2, AccountName: "GuestTest",
+                StartedAt: DateTimeOffset.UtcNow, RestartCount: 1,
+                Identity: new ProcessIdentity(victim.Id + 1, realStart!.Value));   // a different one
+
+            Assert.False(contradictory.IsAlive);   // healthy process, reported dead
+
+            // The same record with a consistent identity reports the truth.
+            var consistent = contradictory with
+            {
+                Identity = new ProcessIdentity(victim.Id, realStart.Value)
+            };
+            Assert.True(consistent.IsAlive);
+        }
+        finally
+        {
+            if (!victim.HasExited) victim.Kill(entireProcessTree: true);
+        }
+    }
+
+    [Fact]
+    public void AnInstanceWithNoIdentity_FallsBackToPidExistence()
+    {
+        // Start time unreadable at launch. Reporting dead here would make SessionHealthCheck
+        // restart a seat whose Apollo is running fine, so the fallback must say alive.
+        using var victim = StartVictim();
+        try
+        {
+            var noIdentity = new ApolloInstance(
+                SeatId: Guid.NewGuid(), ProcessId: victim.Id,
+                ConfigPath: "x", SessionId: 2, AccountName: "GuestTest",
+                StartedAt: DateTimeOffset.UtcNow, RestartCount: 0, Identity: null);
+
+            Assert.True(noIdentity.IsAlive);
+        }
+        finally
+        {
+            if (!victim.HasExited) victim.Kill(entireProcessTree: true);
+        }
+    }
+
     [Fact]
     public void TryKillIdentifiedProcess_NeverThrows()
     {