diff --git a/src/MultiSeat.Service/Monitoring/SessionHealthCheck.cs b/src/MultiSeat.Service/Monitoring/SessionHealthCheck.cs index 66a6215..2a496ec 100644 --- a/src/MultiSeat.Service/Monitoring/SessionHealthCheck.cs +++ b/src/MultiSeat.Service/Monitoring/SessionHealthCheck.cs @@ -296,11 +296,16 @@ private async Task CheckSeatAsync(SeatInfo seat, CancellationToken ct) // In the future, we could track the game PID for auto-restart. } - // ── Launch-on-connect: tail Apollo's log for client connect/disconnect ── - // and launch (or kill) the configured per-seat apps on the edges. No-op when - // MultiSeat:LaunchOnConnect is empty. Cheap: reads only the bytes appended - // since the previous tick. Does not change seat state here. - _onConnectApps.ProcessSeat(seat, ct); + // ── Client connect/disconnect: tail Apollo's log for the edges ────────── + // Moves the seat between Ready and Streaming, and launches (or kills) the configured + // per-seat apps. Cheap: reads only the bytes appended since the previous tick. + // + // ⚠️ This DOES change seat state now. It used to be skipped entirely unless + // MultiSeat:LaunchOnConnect was configured, which is empty by default — so on a normal + // host nothing ever noticed a client connecting and a streaming seat reported Ready + // forever (#43). The app launching is still gated; the observation is not. + if (_onConnectApps.ProcessSeat(seat, ct)) + return true; // Ready <-> Streaming changed — worth broadcasting // ── Follow the client's requested resolution ────────────── // Apollo cannot apply it itself inside an RDP seat, so resize by reconnecting the diff --git a/src/MultiSeat.Service/Streaming/OnConnectAppLauncher.cs b/src/MultiSeat.Service/Streaming/OnConnectAppLauncher.cs index ada7408..f2135e7 100644 --- a/src/MultiSeat.Service/Streaming/OnConnectAppLauncher.cs +++ b/src/MultiSeat.Service/Streaming/OnConnectAppLauncher.cs @@ -58,16 +58,27 @@ public OnConnectAppLauncher( } /// - /// Inspect a seat's Apollo log for new connect/disconnect events and act on edges. - /// Cheap and safe to call every health-check tick. No-op when the feature is off. + /// Inspect a seat's Apollo log for new connect/disconnect events and act on edges: move the + /// seat between Ready and Streaming, and launch or kill the configured apps. + /// + /// Returns true when the seat's status changed, so the caller can broadcast it. + /// + /// ⭐ Detection is NOT gated on MultiSeat:LaunchOnConnect. It used to be — this method opened + /// with if (_options.LaunchOnConnect.Length == 0) return;, which disabled the whole + /// watcher including the connect/disconnect reading. Since that option is empty by default, + /// nothing on a default host ever observed a client connecting, and a seat streaming happily + /// still reported Ready: the dashboard showed it idle, and anything deciding whether a seat + /// was safe to tear down got the wrong answer. See issue #43. + /// + /// Launching apps stays gated; only the observation is unconditional. Cheap either way — it + /// reads just the bytes appended since the previous tick. /// - public void ProcessSeat(SeatInfo seat, CancellationToken ct) + public bool ProcessSeat(SeatInfo seat, CancellationToken ct) { - if (_options.LaunchOnConnect.Length == 0) return; // feature disabled - if (seat.SessionId < 0) return; + if (seat.SessionId < 0) return false; var logPath = _apollo.GetLogPath(seat.AccountName, _options.ApolloConfigDir); - if (!File.Exists(logPath)) return; + if (!File.Exists(logPath)) return false; var state = _states.GetOrAdd(seat.Id, _ => SeedState(logPath)); @@ -87,18 +98,54 @@ public void ProcessSeat(SeatInfo seat, CancellationToken ct) } bool? connectedNow = ReadLatestState(logPath, state); - if (connectedNow is null) return; // no new connect/disconnect lines since last tick + if (connectedNow is null) return false; // no new connect/disconnect lines since last tick lock (state.Gate) { - if (connectedNow.Value == state.Connected) return; // no edge + if (connectedNow.Value == state.Connected) return false; // no edge state.Connected = connectedNow.Value; - if (connectedNow.Value) - OnConnect(seat, state, ct); - else - OnDisconnect(seat, state); + // Status first, and independently of the app feature. A client is attached or it is + // not; whether anyone configured an app to launch has nothing to do with it. + var statusChanged = ApplyStreamingStatus(seat, connectedNow.Value); + + if (_options.LaunchOnConnect.Length > 0) + { + if (connectedNow.Value) + OnConnect(seat, state, ct); + else + OnDisconnect(seat, state); + } + + return statusChanged; + } + } + + /// + /// Move the seat between Ready and Streaming to match whether a client is attached. + /// + /// ⚠️ Only those two statuses are touched. A seat that is Provisioning, TearingDown or Error + /// is mid-something that matters more than a client edge, and stamping Streaming over it would + /// both lose that information and trip the transition table. + /// + private bool ApplyStreamingStatus(SeatInfo seat, bool clientConnected) + { + var target = clientConnected ? SeatStatus.Streaming : SeatStatus.Ready; + + if (seat.Status == target) return false; + if (seat.Status is not (SeatStatus.Ready or SeatStatus.Streaming)) + { + _logger.LogDebug( + "Seat {Id}: client {Edge} while {Status} — leaving the status alone", + seat.Id, clientConnected ? "connected" : "disconnected", seat.Status); + return false; } + + seat.TransitionTo(target, _logger); + _logger.LogInformation( + "Seat {Id}: client {Edge} — now {Status}", + seat.Id, clientConnected ? "connected" : "disconnected", target); + return true; } /// Drop tracked state for a seat that has been torn down. diff --git a/src/MultiSeat.Tests/Streaming/OnConnectAppLauncherTests.cs b/src/MultiSeat.Tests/Streaming/OnConnectAppLauncherTests.cs index c7a3394..597667f 100644 --- a/src/MultiSeat.Tests/Streaming/OnConnectAppLauncherTests.cs +++ b/src/MultiSeat.Tests/Streaming/OnConnectAppLauncherTests.cs @@ -221,4 +221,133 @@ public void SeedingAnEmptyLog_StartsDisconnected() } finally { File.Delete(path); } } + // ── #43: a streaming seat must not report Ready ────────────────────── + // + // ProcessSeat used to open with `if (_options.LaunchOnConnect.Length == 0) return;`, which + // switched off the whole watcher — detection included — and that option is empty by default. + // So on a normal host nothing ever saw a client connect, and a seat streaming happily still + // reported Ready: the dashboard showed it idle, and anything deciding whether a seat was safe + // to tear down got the wrong answer. + // + // These drive the REAL ProcessSeat with LaunchOnConnect EMPTY. That is the whole point: the + // tests above only exercise the static log helpers, which is why none of them noticed. + + private sealed class NoopLogger : Microsoft.Extensions.Logging.ILogger + { + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(Microsoft.Extensions.Logging.LogLevel l) => false; + public void Log(Microsoft.Extensions.Logging.LogLevel l, + Microsoft.Extensions.Logging.EventId e, TState s, Exception? ex, + Func f) { } + } + + private static (OnConnectAppLauncher Launcher, string LogPath, string Root) NewWatcher() + { + var root = Path.Combine(Path.GetTempPath(), $"ms-onconnect-{Guid.NewGuid():N}"); + var seatDir = Path.Combine(root, "GuestTest"); + Directory.CreateDirectory(seatDir); + var logPath = Path.Combine(seatDir, "apollo.log"); + File.WriteAllText(logPath, string.Empty, + new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); + + var options = Microsoft.Extensions.Options.Options.Create( + new MultiSeat.Service.Configuration.MultiSeatOptions + { + ApolloConfigDir = root, + LaunchOnConnect = Array.Empty() + }); + + var apollo = new ApolloManager(new NoopLogger(), options, + configBuilder: null!, processInjector: null!); + var launcher = new OnConnectAppLauncher(new NoopLogger(), options, + apollo, injector: null!); + return (launcher, logPath, root); + } + + private static MultiSeat.Shared.Models.SeatInfo NewSeat( + MultiSeat.Shared.Models.SeatStatus status = MultiSeat.Shared.Models.SeatStatus.Ready) => new() + { + Id = Guid.NewGuid(), + AccountName = "GuestTest", + SessionId = 2, + Status = status + }; + + [Fact] + public void ClientConnect_MovesSeatToStreaming_EvenWithNoAppsConfigured() + { + var (launcher, logPath, root) = NewWatcher(); + try + { + var seat = NewSeat(); + launcher.ProcessSeat(seat, CancellationToken.None); // seed at current end + + Append(logPath, Line(Connect)); + var changed = launcher.ProcessSeat(seat, CancellationToken.None); + + Assert.True(changed); + Assert.Equal(MultiSeat.Shared.Models.SeatStatus.Streaming, seat.Status); + } + finally { Directory.Delete(root, true); } + } + + [Fact] + public void ClientDisconnect_MovesSeatBackToReady() + { + var (launcher, logPath, root) = NewWatcher(); + try + { + var seat = NewSeat(); + launcher.ProcessSeat(seat, CancellationToken.None); + Append(logPath, Line(Connect)); + launcher.ProcessSeat(seat, CancellationToken.None); + Assert.Equal(MultiSeat.Shared.Models.SeatStatus.Streaming, seat.Status); + + Append(logPath, Line(Disconnect)); + var changed = launcher.ProcessSeat(seat, CancellationToken.None); + + Assert.True(changed); + Assert.Equal(MultiSeat.Shared.Models.SeatStatus.Ready, seat.Status); + } + finally { Directory.Delete(root, true); } + } + + [Fact] + public void NoEdge_ReportsNoChange() + { + // A quiet log must not be reported as a change every tick, or the health check would + // broadcast a seat update forever. + var (launcher, logPath, root) = NewWatcher(); + try + { + var seat = NewSeat(); + launcher.ProcessSeat(seat, CancellationToken.None); + Append(logPath, "[2026-08-29 10:26:20.078]: Info: Client dynamicRange: 0\n"); + + Assert.False(launcher.ProcessSeat(seat, CancellationToken.None)); + Assert.Equal(MultiSeat.Shared.Models.SeatStatus.Ready, seat.Status); + } + finally { Directory.Delete(root, true); } + } + + [Theory] + [InlineData(MultiSeat.Shared.Models.SeatStatus.Provisioning)] + [InlineData(MultiSeat.Shared.Models.SeatStatus.TearingDown)] + [InlineData(MultiSeat.Shared.Models.SeatStatus.Error)] + public void ASeatMidLifecycle_IsLeftAlone(MultiSeat.Shared.Models.SeatStatus status) + { + // A client edge is less important than what the seat is already doing, and stamping + // Streaming over TearingDown would both lose that and trip the transition table. + var (launcher, logPath, root) = NewWatcher(); + try + { + var seat = NewSeat(status); + launcher.ProcessSeat(seat, CancellationToken.None); + Append(logPath, Line(Connect)); + + Assert.False(launcher.ProcessSeat(seat, CancellationToken.None)); + Assert.Equal(status, seat.Status); + } + finally { Directory.Delete(root, true); } + } }