diff --git a/crates/pf-vdisplay/src/lib.rs b/crates/pf-vdisplay/src/lib.rs index 03da7a88..543bb1e9 100644 --- a/crates/pf-vdisplay/src/lib.rs +++ b/crates/pf-vdisplay/src/lib.rs @@ -130,6 +130,22 @@ impl Compositor { } } + /// Does this backend need a compositor that is ALREADY RUNNING for this uid? + /// + /// Every desktop backend attaches to a live session — it asks Mutter/KWin/sway/Hyprland to mint + /// a virtual output over their IPC, so with nothing running there is no one to ask and `create` + /// can only fail (on GNOME: `RemoteDesktop.CreateSession: + /// org.freedesktop.DBus.Error.ServiceUnknown`). [`Compositor::Gamescope`] is the exception: it + /// stands its own session up from nothing (bare headless spawn / managed takeover), which is + /// exactly why a headless box pins to it. + /// + /// Callers use this to tell "the session is up" from "the session is a corpse" BEFORE marching a + /// client into a doomed bring-up — the state a compositor crash leaves behind (gnome-shell + /// SIGSEGV → GDM greeter, whose auto-login is once-per-boot, so it never returns on its own). + pub fn needs_live_session(self) -> bool { + !matches!(self, Compositor::Gamescope) + } + /// Human label for UIs. pub fn label(self) -> &'static str { match self { diff --git a/crates/punktfunk-host/src/native/compositor.rs b/crates/punktfunk-host/src/native/compositor.rs index 7b4d9a3f..fe4cdf99 100644 --- a/crates/punktfunk-host/src/native/compositor.rs +++ b/crates/punktfunk-host/src/native/compositor.rs @@ -33,6 +33,52 @@ fn pick_compositor( } } +/// Is this connect pinned at a compositor that is not actually running? +/// +/// Pure (the I/O shell passes in the observed liveness) so the interaction is unit-tested, because +/// it is invisible from the outside: an operator pin puts its backend into +/// [`crate::vdisplay::available`] unconditionally AND skips `apply_session_env`'s +/// `XDG_CURRENT_DESKTOP` scrub, so [`pick_compositor`] hands back a compositor that may be a corpse +/// and its `None` (recover) arm can never fire. [`Compositor::Gamescope`] is exempt — it stands its +/// own session up, which is the whole reason a headless box pins it. +#[cfg(not(target_os = "windows"))] +fn pinned_at_a_dead_session( + overridden: bool, + chosen: crate::vdisplay::Compositor, + live: crate::vdisplay::ActiveKind, +) -> bool { + overridden && chosen.needs_live_session() && live == crate::vdisplay::ActiveKind::None +} + +/// The handshake error for "no graphical session is live for this uid" — the state a compositor +/// crash leaves behind (gnome-shell SIGSEGV → GDM greeter, whose auto-login is once-per-boot, so the +/// box would otherwise need a walk-up or a reboot). +/// +/// Fires the operator's recovery hook (debounced) on the way out when one is configured, so the +/// client's retry a few seconds later lands in a recovered desktop. `pinned` names the +/// `PUNKTFUNK_COMPOSITOR` value when the pin is what got us here, so the message can say which knob +/// to change rather than the generic advice to *set* the knob that caused it. +#[cfg(not(target_os = "windows"))] +fn no_live_session(pinned: Option<&str>) -> anyhow::Error { + if crate::vdisplay::try_recover_session() { + return anyhow::anyhow!( + "no live graphical session for this uid — host session recovery launched \ + (PUNKTFUNK_RECOVER_SESSION_CMD); retry in a few seconds" + ); + } + match pinned { + Some(pin) => anyhow::anyhow!( + "PUNKTFUNK_COMPOSITOR={pin} pins this host to a backend that can only attach to an \ + already-running compositor, and no graphical session is live for this uid — start a \ + session, pin `gamescope` (it stands its own up), or set PUNKTFUNK_RECOVER_SESSION_CMD" + ), + None => anyhow::anyhow!( + "no usable compositor (no live graphical session for this uid; set \ + PUNKTFUNK_COMPOSITOR or start a desktop/gaming session)" + ), + } +} + /// Resolve the client's compositor preference to a concrete backend (the I/O shell around /// [`pick_compositor`]): enumerate what's available, auto-detect the default, pick, and log /// whether the explicit request was honored or fell back. Runs blocking probes — call off the @@ -61,13 +107,21 @@ pub(super) fn resolve_compositor( // Explicit operator override (legacy / CI / forcing a backend for a test) wins and is assumed // to come with a hand-set env — don't retarget the process env in that case. let overridden = pf_host_config::config().compositor.is_some(); + // Liveness is read on BOTH paths. The auto path retargets the process env at the live + // session (below); the PINNED path needs it too, because a pin names a BACKEND, not a + // running session — and a pin whose compositor has died used to be indistinguishable from a + // healthy one here (it skips `apply_session_env`'s `XDG_CURRENT_DESKTOP` scrub and lands + // itself in `available()`, so `pick_compositor` could never return `None`). That combination + // marched every client through 8 doomed `create` retries and left the operator's + // `PUNKTFUNK_RECOVER_SESSION_CMD` unreachable — see the `needs_live_session` gate below. + let active = crate::vdisplay::detect_active_session(); let detected = if overridden { crate::vdisplay::detect().ok() } else { // Auto: detect the LIVE session (Gaming vs Desktop) and retarget the process env at it so // every backend (video capture + input) this connect opens against the active session — // this is the state machine that lets one host follow a Bazzite box across Gaming↔Desktop. - let active = crate::vdisplay::detect_active_session(); + // // A4: if the compositor instance changed since the last connect (an idle-time Game↔Desktop // switch), bump the epoch + invalidate the old backend's kept displays so this connect never // reuses a node id from the dead instance. @@ -84,14 +138,36 @@ pub(super) fn resolve_compositor( // under `game_session=dedicated` (gamescope confirmed available) forces its OWN headless // gamescope spawn at the client's mode, overriding the detected desktop/game-mode backend. The // env was already retargeted above (for XDG_RUNTIME_DIR / the PipeWire daemon); we just pin the - // backend + input to the spawn sub-mode. Skipped under an explicit operator compositor pin. - if dedicated_launch && !overridden { - let route = crate::vdisplay::apply_input_env(Compositor::Gamescope, true); - tracing::info!( - ?route, - "dedicated game session — routing to a headless gamescope spawn at the client mode" - ); - return Ok((Compositor::Gamescope, route)); + // backend + input to the spawn sub-mode. An explicit operator compositor pin still outranks + // it — but says so out loud (below), because a silent veto is indistinguishable from the + // feature being broken. + if dedicated_launch { + if overridden { + // The pin still wins (it is the operator's explicit, hand-configured knob), but it + // must NEVER win silently: the console goes on displaying `game_session=dedicated` + // while every launch lands in the pinned session instead, and nothing in the log + // connects the two. That cost a full triage on a box whose `PUNKTFUNK_COMPOSITOR` + // was a forgotten validation leftover — the setting had never once taken effect and + // the only evidence was the ABSENCE of the info! line below. + tracing::warn!( + pin = pf_host_config::config() + .compositor + .as_deref() + .unwrap_or("-"), + "game_session=dedicated asked for this launch's OWN headless gamescope, but \ + PUNKTFUNK_COMPOSITOR pins this host to a backend — the operator pin wins and \ + the game launches into the pinned session instead. Unset PUNKTFUNK_COMPOSITOR \ + to get dedicated game sessions." + ); + } else { + let route = crate::vdisplay::apply_input_env(Compositor::Gamescope, true); + tracing::info!( + ?route, + "dedicated game session — routing to a headless gamescope spawn at the client \ + mode" + ); + return Ok((Compositor::Gamescope, route)); + } } let available = crate::vdisplay::available(); let chosen = match pick_compositor(pref, &available, detected) { @@ -112,23 +188,18 @@ pub(super) fn resolve_compositor( ); Compositor::Gamescope } - None => { - // The state a compositor crash leaves behind (gnome-shell - // SIGSEGV → GDM greeter, whose auto-login is once-per-boot). If the operator - // configured a recovery hook, fire it (debounced) and tell the client to retry: - // its next knock lands in the recovered desktop. - if crate::vdisplay::try_recover_session() { - anyhow::bail!( - "no live graphical session for this uid — host session recovery launched \ - (PUNKTFUNK_RECOVER_SESSION_CMD); retry in a few seconds" - ); - } - anyhow::bail!( - "no usable compositor (no live graphical session for this uid; set \ - PUNKTFUNK_COMPOSITOR or start a desktop/gaming session)" - ); - } + None => return Err(no_live_session(None)), }; + // Same dead-session exit, reached the other way: a pin puts its backend in `available()` + // unconditionally, so `pick_compositor` above can hand back a compositor that is not + // actually running and the `None` arm never fires. Check the backend's own requirement + // against observed liveness instead of trusting the pin. Gamescope is exempt — it stands + // its own session up, which is the whole point of pinning it on a headless box. + if pinned_at_a_dead_session(overridden, chosen, active.kind) { + return Err(no_live_session( + pf_host_config::config().compositor.as_deref(), + )); + } // Point input at the same backend and resolve the gamescope sub-mode (managed where the // session infra exists, attach to a foreign gamescope, else per-session bare spawn). The // route travels back to the caller as a VALUE and is carried on the backend instance — an @@ -170,6 +241,44 @@ mod tests { use super::pick_compositor; use punktfunk_core::config::CompositorPref; + /// A pin at a compositor that ISN'T RUNNING must take the recovery exit rather than march the + /// client into a bring-up that can only fail. + /// + /// The regression this pins down: `PUNKTFUNK_COMPOSITOR=mutter` on a box whose gnome-shell had + /// segfaulted. The pin put Mutter in `available()` and suppressed the `XDG_CURRENT_DESKTOP` + /// scrub, so every connect "resolved" happily and then spent 8 retries on + /// `RemoteDesktop.CreateSession: ServiceUnknown` — while the operator's + /// `PUNKTFUNK_RECOVER_SESSION_CMD` sat unreachable behind a `None` arm that could never fire. + #[cfg(not(target_os = "windows"))] + #[test] + fn a_pin_at_a_dead_session_recovers_instead_of_retrying() { + use super::pinned_at_a_dead_session as dead; + use crate::vdisplay::{ActiveKind, Compositor::*}; + // The bug: pinned to a desktop backend with nothing live for this uid. + assert!(dead(true, Mutter, ActiveKind::None)); + assert!(dead(true, Kwin, ActiveKind::None)); + assert!(dead(true, Wlroots, ActiveKind::None)); + assert!(dead(true, Hyprland, ActiveKind::None)); + // Pinned but the session IS up — the ordinary case, must not bail. + assert!(!dead(true, Mutter, ActiveKind::DesktopGnome)); + // Gamescope stands its own session up from nothing: pinning it on a headless box is a + // SUPPORTED setup, not a dead session. (This is the .21 no-login workaround — never break it.) + assert!(!dead(true, Gamescope, ActiveKind::None)); + // Unpinned is untouched: the auto path already reaches `pick_compositor`'s `None` arm via + // `compositor_for_kind(ActiveKind::None)`, and it owns the managed-takeover case. + assert!(!dead(false, Mutter, ActiveKind::None)); + } + + /// gamescope is the ONLY backend that can serve a connect with no session already running. + #[test] + fn only_gamescope_survives_a_dead_session() { + use crate::vdisplay::Compositor::*; + assert!(!Gamescope.needs_live_session()); + for c in [Mutter, Kwin, Wlroots, Hyprland] { + assert!(c.needs_live_session(), "{c:?} needs a live compositor"); + } + } + #[test] fn compositor_resolution_precedence() { use crate::vdisplay::Compositor::*; diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 524f2765..a49977ad 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -1553,6 +1553,46 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option