From 1fc184516a8611a1be16cbf2a55d264ed3b796c7 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 10 Aug 2026 14:11:01 +0200 Subject: [PATCH 01/11] =?UTF-8?q?feat(apple):=20the=20browse=20route=20is?= =?UTF-8?q?=20real=20=E2=80=94=20a=20Shortcut=20or=20a=20widget=20jumps=20?= =?UTF-8?q?straight=20into=20a=20host's=20library?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reserved punktfunk://browse/ route now routes on Apple: it drives the same libraryTarget every internal surface writes, so the link lands in whichever presentation the current mode owns — the gamepad console's in-place library screen, the touch cover, the macOS sheet, or tvOS's cover. Connect's posture minus the connect: a pin conflict refuses, a live session is never preempted, an unsaved host gets a notice (the library rides the paired mTLS identity, so there is nothing to browse before the host is saved). browse ignores launch=/profile= — nothing streams until a title is picked, and that connect resolves its own profile. On top of the route, the two new front doors: - OpenLibraryIntent ("Open Game Library") beside Connect/Wake/End in Shortcuts/Siri/Spotlight, host-parameterized like the others and round-tripping through the URL — one router, no second path. - A configurable library widget (kind "PunktfunkLibrary", AppIntentConfiguration over HostEntity — the configuration the HostEntity doc comment anticipated): pick a host, tap into its library. Unconfigured it follows the most recent host; a configured host that was removed shows the empty state rather than silently following a different host. Same .never timeline + HostStore push as the hosts widget, now reloading both kinds. DeepLink.browse(host:) is the one emitter both doors share, covered by a round-trip test beside connect's; the parse side was already in the grammar and the vector file. Docs updated (clients, game-library, profiles-and-links). --- .../PunktfunkWidgets/LibraryWidget.swift | 202 ++++++++++++++++++ .../PunktfunkWidgetBundle.swift | 1 + .../Sources/PunktfunkClient/ContentView.swift | 50 ++++- .../Intents/SessionShortcuts.swift | 34 ++- .../PunktfunkClient/Stores/HostStore.swift | 9 +- .../Sources/PunktfunkShared/DeepLink.swift | 7 + .../SharedFoundationTests.swift | 12 ++ docs-site/content/docs/clients.md | 6 +- docs-site/content/docs/game-library.md | 5 +- docs-site/content/docs/profiles-and-links.md | 6 +- 10 files changed, 317 insertions(+), 15 deletions(-) create mode 100644 clients/apple/PunktfunkWidgets/LibraryWidget.swift diff --git a/clients/apple/PunktfunkWidgets/LibraryWidget.swift b/clients/apple/PunktfunkWidgets/LibraryWidget.swift new file mode 100644 index 00000000..066746fa --- /dev/null +++ b/clients/apple/PunktfunkWidgets/LibraryWidget.swift @@ -0,0 +1,202 @@ +// Configurable Home-Screen / Lock-Screen library widget (kind "PunktfunkLibrary"). The user picks +// a saved host in the widget's configuration (long-press → Edit Widget — the picker is +// `HostEntity`'s query over the shared App-Group store, running in this extension process); a tap +// deep-links into that host's game library via `punktfunk://browse/` — the app's onOpenURL +// routes it to the same library presentation every internal surface drives. No session starts +// until a title is picked there. +// +// Unconfigured, it follows the most recently connected host (the same order the hosts widget +// leads with). A configured host that no longer exists shows the empty state rather than silently +// following a different host — a widget that says "Studio" must never open someone else's library. +// +// Timeline is a single `.never` entry — the app pushes reloads on store changes (HostStore → +// WidgetCenter.reloadTimelines), exactly like the hosts widget. + +import AppIntents +import SwiftUI +import WidgetKit + +import PunktfunkShared + +// MARK: - Configuration intent + +/// The widget's per-instance configuration. Executes in the EXTENSION process — which is why +/// `HostEntity` and its query live in PunktfunkShared, not the app. +struct LibraryWidgetConfigIntent: WidgetConfigurationIntent { + static let title: LocalizedStringResource = "Choose Host" + static let description = IntentDescription("Pick whose game library this widget opens.") + + @Parameter(title: "Host", description: "Leave empty to follow your most recent host.") + var host: HostEntity? +} + +// MARK: - Timeline + +struct LibraryEntry: TimelineEntry { + let date: Date + /// The resolved target: the configured host if it still exists, the most recent one when + /// unconfigured, nil when there's nothing to open (empty store, or a removed configured host). + let host: StoredHost? +} + +struct LibraryProvider: AppIntentTimelineProvider { + func placeholder(in context: Context) -> LibraryEntry { + LibraryEntry(date: .now, host: nil) + } + + func snapshot(for configuration: LibraryWidgetConfigIntent, in context: Context) async + -> LibraryEntry { + LibraryEntry(date: .now, host: Self.resolve(configuration.host)) + } + + func timeline(for configuration: LibraryWidgetConfigIntent, in context: Context) async + -> Timeline { + // Single entry, never auto-refresh: the app reloads this timeline on every store change. + Timeline(entries: [LibraryEntry(date: .now, host: Self.resolve(configuration.host))], + policy: .never) + } + + /// The configured host by id — nil (NOT a fallback) when it's gone; most-recent when nothing + /// was configured. + static func resolve(_ configured: HostEntity?) -> StoredHost? { + let hosts = HostsProvider.loadHosts() // shared-suite JSON, most-recent first + guard let configured else { return hosts.first } + return hosts.first { $0.id == configured.id } + } +} + +// MARK: - Widget + +struct LibraryWidget: Widget { + var body: some WidgetConfiguration { + AppIntentConfiguration( + kind: "PunktfunkLibrary", intent: LibraryWidgetConfigIntent.self, + provider: LibraryProvider() + ) { entry in + LibraryWidgetView(entry: entry) + .containerBackground(.fill.tertiary, for: .widget) + } + .configurationDisplayName("Game Library") + .description("Jump straight into a host's game library.") + .supportedFamilies([.systemSmall, .accessoryCircular, .accessoryRectangular]) + } +} + +// MARK: - Views + +/// Deep link that opens a stored host's library. +private func browseURL(_ host: StoredHost) -> URL { + DeepLink.browse(host: host.id).url +} + +struct LibraryWidgetView: View { + @Environment(\.widgetFamily) private var family + let entry: LibraryEntry + + var body: some View { + switch family { + case .accessoryCircular: + CircularLibraryView(host: entry.host) + case .accessoryRectangular: + RectangularLibraryView(host: entry.host) + default: // systemSmall + fallback + SmallLibraryView(host: entry.host) + } + } +} + +private struct SmallLibraryView: View { + let host: StoredHost? + var body: some View { + if let host { + VStack(alignment: .leading, spacing: 6) { + Image(systemName: "square.grid.2x2.fill") + .font(.title2) + .foregroundStyle(Color.brand) + Spacer(minLength: 0) + Text(host.displayName) + .font(.headline) + .lineLimit(2) + Text("Game Library") + .font(.caption2) + .foregroundStyle(.secondary) + } + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + .widgetURL(browseURL(host)) + } else { + EmptyLibraryView() + } + } +} + +private struct CircularLibraryView: View { + let host: StoredHost? + var body: some View { + ZStack { + AccessoryWidgetBackground() + Image(systemName: "square.grid.2x2.fill") + } + .widgetURL(host.map(browseURL)) + } +} + +private struct RectangularLibraryView: View { + let host: StoredHost? + var body: some View { + HStack { + Image(systemName: "square.grid.2x2.fill") + VStack(alignment: .leading) { + Text(host?.displayName ?? "Punktfunk") + .lineLimit(1) + Text("Library") + .font(.caption2) + .foregroundStyle(.secondary) + } + } + .widgetURL(host.map(browseURL)) + } +} + +private struct EmptyLibraryView: View { + var body: some View { + VStack(spacing: 6) { + Image(systemName: "square.grid.2x2") + .font(.title2) + .foregroundStyle(.secondary) + Text("Open Punktfunk to pick a host.") + .font(.caption) + .multilineTextAlignment(.center) + .foregroundStyle(.secondary) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + } +} + +// MARK: - Previews (Xcode canvas) +// +// Same pattern as the hosts widget: `#Preview(as:widget:timeline:)` feeds sample entries directly, +// so the canvas works without a paired device or saved hosts. The small preview's second entry +// shows the empty state one timeline click away. + +private let previewHost = StoredHost( + name: "Studio", address: "192.168.1.20", + lastConnected: .now.addingTimeInterval(-40 * 60)) + +#Preview("Small", as: .systemSmall) { + LibraryWidget() +} timeline: { + LibraryEntry(date: .now, host: previewHost) + LibraryEntry(date: .now, host: nil) +} + +#Preview("Lock Screen circular", as: .accessoryCircular) { + LibraryWidget() +} timeline: { + LibraryEntry(date: .now, host: previewHost) +} + +#Preview("Lock Screen rectangular", as: .accessoryRectangular) { + LibraryWidget() +} timeline: { + LibraryEntry(date: .now, host: previewHost) +} diff --git a/clients/apple/PunktfunkWidgets/PunktfunkWidgetBundle.swift b/clients/apple/PunktfunkWidgets/PunktfunkWidgetBundle.swift index 4ee4e8d6..dff6ee22 100644 --- a/clients/apple/PunktfunkWidgets/PunktfunkWidgetBundle.swift +++ b/clients/apple/PunktfunkWidgets/PunktfunkWidgetBundle.swift @@ -15,6 +15,7 @@ import WidgetKit struct PunktfunkWidgetBundle: WidgetBundle { var body: some Widget { HostsWidget() + LibraryWidget() PunktfunkSessionLiveActivity() } } diff --git a/clients/apple/Sources/PunktfunkClient/ContentView.swift b/clients/apple/Sources/PunktfunkClient/ContentView.swift index c469c2a8..d2e78529 100644 --- a/clients/apple/Sources/PunktfunkClient/ContentView.swift +++ b/clients/apple/Sources/PunktfunkClient/ContentView.swift @@ -487,10 +487,20 @@ struct ContentView: View { ?? "That link is malformed and was ignored." return } - guard link.route == .connect else { - // `wake` and `browse` are reserved in the grammar and parse today; this build routes - // neither, and saying so beats silently connecting instead. - deepLinkNotice = "Punktfunk links can't do “\(link.route.rawValue)” yet." + switch link.route { + case .connect: + break + case .browse: + // The reserved library route, now real: open the host's game library without starting + // a session. `launch=`/`profile=` are meaningless on a browse (nothing streams until a + // title is picked, and that connect resolves its own profile) — ignored, not refused, + // per the unknown-parameter rule. + openLibrary(from: link) + return + case .wake: + // Still reserved: saying so beats silently connecting instead. (Shortcuts users have + // the Wake Host intent, which never round-trips through a URL.) + deepLinkNotice = "Punktfunk links can't do “wake” yet." return } // Resolve the one-off profile BEFORE anything happens: an unknown or ambiguous reference @@ -544,6 +554,38 @@ struct ContentView: View { } } + /// `punktfunk://browse/` — jump into a host's game library. Drives the SAME + /// `libraryTarget` every internal surface writes, so the link lands in whichever presentation + /// the current mode owns: the gamepad console's in-place library screen, the touch cover, the + /// macOS sheet, or tvOS's cover. Connect's posture minus the connect itself: a pin conflict + /// refuses, a live session is never preempted, and an unsaved host can't be browsed — the + /// library fetch rides the paired mTLS identity, so there is nothing to show before the host + /// is saved (the notice says what to do instead). + private func openLibrary(from link: DeepLink) { + switch link.resolveHost(in: store.hosts) { + case .known(let host): + guard !link.pinConflict(with: host) else { + deepLinkNotice = "That link's fingerprint doesn't match the identity saved for " + + "\(host.displayName). It's out of date, or it isn't pointing where it says." + return + } + guard model.phase == .idle else { + let current = model.activeHost?.displayName ?? "a host" + deepLinkNotice = "Already streaming \(current). End that session first." + return + } + libraryTarget = host + case .unknown(let address, _, let name, _): + deepLinkNotice = "\(name ?? address) isn't saved on this device yet. " + + "Add it with the + button first — a library can only be browsed on a saved host." + case .ambiguous: + deepLinkNotice = "More than one saved host is called “\(link.hostRef)”. " + + "Rename one, or link to it by its address." + case .unresolvable: + deepLinkNotice = "That host isn't saved on this device." + } + } + private var home: some View { // The full-screen connect takeover rides over BOTH home UIs (and the pre-connect window is // still `home`, so it covers the whole dial → wake → online → connect sequence): instant diff --git a/clients/apple/Sources/PunktfunkClient/Intents/SessionShortcuts.swift b/clients/apple/Sources/PunktfunkClient/Intents/SessionShortcuts.swift index 7c23c643..c0ea204f 100644 --- a/clients/apple/Sources/PunktfunkClient/Intents/SessionShortcuts.swift +++ b/clients/apple/Sources/PunktfunkClient/Intents/SessionShortcuts.swift @@ -1,7 +1,8 @@ // Siri / Shortcuts / Spotlight surface (design §M4, extended by client-deep-links.md §6). // Deliberately thin: every action already has an internal entry point — the deep-link router -// (connect / connect-and-launch / connect-with-a-profile), the in-process end-session hook, and -// the existing Wake-on-LAN path — so these intents only wrap them. +// (connect / connect-and-launch / connect-with-a-profile, and the `browse` route into a host's +// library), the in-process end-session hook, and the existing Wake-on-LAN path — so these +// intents only wrap them. // // Connect and Wake compile on macOS and tvOS too: AppIntents is genuinely available there // (macOS 13+ / tvOS 16+), and "Stream Desktop with Work" from Spotlight on a Mac is part of the @@ -51,6 +52,28 @@ struct ConnectToHostIntent: AppIntent { } } +/// Jump straight into a host's game library — no session. Foregrounds the app and routes the +/// `browse` route through the same `.onOpenURL` path a widget tap uses, which drives the one +/// `libraryTarget` every surface shares — so the shortcut lands in whichever library presentation +/// the current mode owns: the gamepad console's library screen when the gamepad UI is active, the +/// touch/desktop library otherwise. A session starts only when a title is picked there. +struct OpenLibraryIntent: AppIntent { + static let title: LocalizedStringResource = "Open Game Library" + static let description = IntentDescription( + "Open a host's game library in Punktfunk, without starting a stream.") + static let openAppWhenRun = true + + @Parameter(title: "Host") var host: HostEntity + + func perform() async throws -> some IntentResult { + let url = DeepLink.browse(host: host.id).url + await MainActor.run { + NotificationCenter.default.post(name: .punktfunkOpenDeepLink, object: url) + } + return .result() + } +} + /// Wake a sleeping host (magic packet). No `openAppWhenRun` — usable in automations ("when I get /// home, wake the tower") without foregrounding the app. struct WakeHostIntent: AppIntent { @@ -97,6 +120,13 @@ struct PunktfunkShortcuts: AppShortcutsProvider { "Stream \(\.$host) with \(.applicationName)", ], shortTitle: "Connect", systemImageName: "play.tv.fill") + AppShortcut( + intent: OpenLibraryIntent(), + phrases: [ + "Open \(\.$host) library in \(.applicationName)", + "Show \(\.$host) games in \(.applicationName)", + ], + shortTitle: "Game Library", systemImageName: "square.grid.2x2.fill") AppShortcut( intent: WakeHostIntent(), phrases: [ diff --git a/clients/apple/Sources/PunktfunkClient/Stores/HostStore.swift b/clients/apple/Sources/PunktfunkClient/Stores/HostStore.swift index 2c47eeaa..2c5f754e 100644 --- a/clients/apple/Sources/PunktfunkClient/Stores/HostStore.swift +++ b/clients/apple/Sources/PunktfunkClient/Stores/HostStore.swift @@ -200,15 +200,16 @@ final class HostStore: ObservableObject { if let data = try? JSONEncoder().encode(hosts) { defaults.set(data, forKey: Self.key) } - reloadHostsWidget() // the widget reads this store; any change refreshes its timeline + reloadHostsWidget() // the widgets read this store; any change refreshes their timelines } - /// Ask WidgetKit to rebuild the hosts widget's timeline after any store change (add/remove/pin/ - /// last-connected). iOS-only and a no-op where WidgetKit is absent; the widget uses - /// `.never`-refresh entries and relies on this push. + /// Ask WidgetKit to rebuild the launcher widgets' timelines after any store change (add/remove/ + /// pin/last-connected). iOS-only and a no-op where WidgetKit is absent; both widgets use + /// `.never`-refresh entries and rely on this push. private func reloadHostsWidget() { #if canImport(WidgetKit) && os(iOS) WidgetCenter.shared.reloadTimelines(ofKind: "PunktfunkHosts") + WidgetCenter.shared.reloadTimelines(ofKind: "PunktfunkLibrary") #endif } } diff --git a/clients/apple/Sources/PunktfunkShared/DeepLink.swift b/clients/apple/Sources/PunktfunkShared/DeepLink.swift index 99fc1744..0037f9df 100644 --- a/clients/apple/Sources/PunktfunkShared/DeepLink.swift +++ b/clients/apple/Sources/PunktfunkShared/DeepLink.swift @@ -191,6 +191,13 @@ public struct DeepLink: Equatable, Sendable { profile: (profile?.isEmpty ?? true) ? nil : profile) } + /// A library link for a saved host — the shape the library widget and the Open Library intent + /// emit. Opens the host's game library without starting a session; a session begins only when + /// the user picks a title there, through the normal connect path. + public static func browse(host: UUID) -> DeepLink { + DeepLink(route: .browse, hostRef: host.uuidString) + } + /// The self-emitted form for a saved host: id first (address-independent), with the address /// and pin alongside so the link degrades to a confirmation sheet instead of a dead click when /// the record is gone ("Copy link", and any shortcut written from a card). diff --git a/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift b/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift index 8926b995..3f79ec54 100644 --- a/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift +++ b/clients/apple/Tests/PunktfunkKitTests/SharedFoundationTests.swift @@ -178,6 +178,18 @@ final class SharedFoundationTests: XCTestCase { XCTAssertEqual(try DeepLink(url: profiled.url).profile, "a1b2c3d4e5f6") } + /// The library widget's and the Open Library intent's emitter — the reserved `browse` route + /// with a bare UUID path. Same backward-compatibility stakes as connect: a Home-Screen widget + /// keeps sending yesterday's URL. + func testDeepLinkBrowseRoundTrips() throws { + let id = UUID(uuidString: "11111111-2222-4333-8444-555555555555")! + let link = DeepLink.browse(host: id) + XCTAssertEqual(link.route, .browse) + XCTAssertEqual( + link.urlString, "punktfunk://browse/11111111-2222-4333-8444-555555555555") + XCTAssertEqual(try DeepLink(url: link.url), link) + } + /// Self-emitted links ("Copy link", a shortcut) carry all three references, so they survive /// both a re-addressed host and a wiped store. func testDeepLinkForHostCarriesIDAddressAndPin() throws { diff --git a/docs-site/content/docs/clients.md b/docs-site/content/docs/clients.md index 7bc527a3..631a462f 100644 --- a/docs-site/content/docs/clients.md +++ b/docs-site/content/docs/clients.md @@ -28,8 +28,10 @@ protocol — the lowest-latency, most resilient path, with the full feature set: launch one straight into the stream. - A live **stats overlay** (resolution, fps, bitrate, latency) and a built-in **network speed test** to pick a bitrate for your link. -- **Widgets, Live Activities and Shortcuts** — a hosts widget for the home screen, a Live Activity - while a session runs, and App Intents so Siri and the Shortcuts app can start a stream. +- **Widgets, Live Activities and Shortcuts** — a hosts widget and a game-library widget for the + home screen (the library one opens a host you pick straight into its library), a Live Activity + while a session runs, and App Intents so Siri and the Shortcuts app can start a stream or jump + into a host's game library. Open the app, pick your host, [pair](/docs/pairing) once, and stream. It builds from the `clients/apple` directory in the repo (Swift / VideoToolbox / Metal). diff --git a/docs-site/content/docs/game-library.md b/docs-site/content/docs/game-library.md index 631880a2..f10d7dd1 100644 --- a/docs-site/content/docs/game-library.md +++ b/docs-site/content/docs/game-library.md @@ -142,7 +142,10 @@ and runs what it already knows about the title, so a client can never hand the h See [Moonlight](/docs/moonlight). - **A link** — a [`punktfunk://` link](/docs/profiles-and-links) carries the id in a `launch=` parameter, so a desktop shortcut, a browser bookmark or a home-automation rule starts the stream - with the title already launching: `punktfunk://connect/couch-pc?launch=steam:570`. + with the title already launching: `punktfunk://connect/couch-pc?launch=steam:570`. On the Apple + apps, `punktfunk://browse/couch-pc` opens the library itself instead — that route backs their + home-screen library widget and the **Open Game Library** shortcut, so a tap lands you in a + picked host's library with nothing streaming yet. - **The command line** — the client's own [`punktfunk`](/docs/host-cli#punktfunk-on-the-client-machine) command, which ships with the Linux and Windows clients. `punktfunk library ` prints `id`, `store` and `title` as tab-separated lines, then a count (`--json` for tools); diff --git a/docs-site/content/docs/profiles-and-links.md b/docs-site/content/docs/profiles-and-links.md index 8beac305..b6203d46 100644 --- a/docs-site/content/docs/profiles-and-links.md +++ b/docs-site/content/docs/profiles-and-links.md @@ -124,8 +124,10 @@ dropped, unknown parameters are ignored, an empty value means "not given", and i appears twice the first one wins. `pf://` parses as an input alias, but nothing emits it and no app registers it with the operating system, so write `punktfunk://`. -`connect` is the only route any client acts on today; `wake` and `browse` parse, but every client -answers them with a notice. Values are capped (2048 for the whole URL, 128 for the host reference +`connect` works everywhere. `browse` — the same grammar with no parameters acted on — opens the +host's game library instead of streaming, on the Apple apps today (it backs their library widget +and the Open Game Library shortcut); every other client answers it with a notice, as they all do +for `wake`. Values are capped (2048 for the whole URL, 128 for the host reference and `launch`, 64 for `profile` and `name`), and `launch` must be printable ASCII with no spaces, quotes, backslashes, `$` or backticks. From 657e82cd290e6389e052c3a1d40324ca101d3638 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 10 Aug 2026 17:16:29 +0200 Subject: [PATCH 02/11] fix(gamescope): a takeover's mask no longer bars the box's own way back into Game Mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A managed takeover runtime-masks the box's `gamescope-session-plus@*` unit so its session supervisor cannot restart it underneath our Steam. The only unmask ran in `do_restore_tv_session`, on client disconnect — so for the whole stream the mask stayed on, and it silently barred the door the user was most likely to walk through next. `mask_unit`'s own doc said the mask "blocks nothing" on images whose sddm helper execs the session script directly. That is half right, and the half it gets wrong is this bug: on f43 bazzite-deck the script's last act is systemctl --user --wait start gamescope-session-plus@${CLIENT}.service (verified on the .41 VM). What the mask fails to stop is the RELOGIN LOOP — sddm keeps trying regardless, which is why stopping the DM is the real defense. What it very much does stop is the unit, and with it every entry into game mode, including the user's own deliberate "Return to Gaming Mode" after a mid-stream switch to the desktop. Steam then sits on its "Switch to Desktop…" modal forever. `--runtime` lives in tmpfs, so a reboot cleared it — hence "it works right after a reboot" — and a plain `unmask` does not (measured: still `masked-runtime`). So the mask's sound lifetime is shorter than the takeover's: it ends the moment the box stops being ours. The mid-stream session watcher already detects exactly that, so it now lifts the mask on a confirmed switch to a desktop session — ahead of the `compositor_for_kind` arm, because a switch we cannot follow still has to unbar the return. `Gaming` and `None` deliberately do not lift: a takeover's own managed session reads as `Gaming` and one momentarily down between relaunches reads as `None`, and lifting on either would void the mask for the whole stream, in exactly the SDDM-storm window it exists for. Fixes a second, worse leak on the way: `honor_session_select_switch` consumed `STOPPED_AUTOLOGIN` — the only record of what carries a mask — without unmasking, so under a DM-stop takeover the disconnect restore found an empty list and lifted nothing. That mask outlived not just the stream but the boot. It is also what let that path's own step 1 work at all, since the DM's autologin heads back into game mode through precisely this unit. The lift is idempotent, keeps the restart list intact (the disconnect restore still owes those units a `start`), and every hand-back path now routes through it. Verified on Linux: `switch_ends_mask_window` decision table, plus an ignored end-to-end test driving real `systemctl --user` (masked → survives Gaming/None → lifted by a desktop switch → restart list intact → idempotent). Proven non-vacuous by planting "Gaming also lifts", which fails it on the during-stream assert. --- crates/pf-vdisplay/src/lib.rs | 2 +- .../src/vdisplay/linux/gamescope.rs | 197 ++++++++++++++++-- crates/pf-vdisplay/src/vdisplay/routing.rs | 16 ++ crates/punktfunk-host/src/native/stream.rs | 7 + 4 files changed, 202 insertions(+), 20 deletions(-) diff --git a/crates/pf-vdisplay/src/lib.rs b/crates/pf-vdisplay/src/lib.rs index 1d3b20ff..a4b1fbab 100644 --- a/crates/pf-vdisplay/src/lib.rs +++ b/crates/pf-vdisplay/src/lib.rs @@ -88,7 +88,7 @@ pub use session::{session_epoch, try_recover_session}; pub(crate) mod routing; pub use routing::{ apply_input_env, managed_session_available, preflight_takeover_privilege, - resolve_gamescope_route, restore_managed_session, restore_takeover_now, + release_autologin_mask, resolve_gamescope_route, restore_managed_session, restore_takeover_now, restore_takeover_on_startup, start_restore_worker, wants_dedicated_game_session, GamescopeRoute, }; diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs index 6dcba18f..6fe74c0f 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs @@ -91,6 +91,13 @@ static STOPPED_AUTOLOGIN: std::sync::Mutex> = std::sync::Mutex::new( /// never gets DRM master — live-proven on the Nobara repro VM 2026-07-24). static STOPPED_DM: std::sync::Mutex> = std::sync::Mutex::new(None); +/// Whether this takeover runtime-masked the [`STOPPED_AUTOLOGIN`] units ([`mask_unit`]) — i.e. +/// whether there is a mask left to lift. Process memory, like the rest of the takeover mechanics. +/// [`restore_takeover_on_startup`] sets it for a stranded takeover it adopts: unmasking a unit we +/// never masked is a no-op, while missing one that IS masked leaves the box unable to enter its +/// own Game Mode until reboot. +static AUTOLOGIN_MASKED: std::sync::Mutex = std::sync::Mutex::new(false); + /// mtime of the `steamos-session-select` sentinel as of the takeover — the baseline the in-stream /// "Switch to Desktop" detector compares against. Steam's session-select script writes /// `~/.config/steamos-session-select` unconditionally in its USER pass, before any of its @@ -252,6 +259,12 @@ pub fn restore_takeover_on_startup() { stopped_dm = ?state.stopped_dm, "gamescope: found a stranded takeover from a previous host instance — scheduling TV restore" ); + // Assume the adopted takeover carries our runtime mask whenever it stopped units: whether it did + // is not persisted (it follows from the DM flavor, which can only have stayed the same), and the + // two errors are not symmetric — unmasking a unit we never masked is a no-op, while skipping one + // that IS masked bars the box from its own game mode until reboot. + *AUTOLOGIN_MASKED.lock().unwrap_or_else(|e| e.into_inner()) = + !state.stopped_autologin.is_empty(); *STOPPED_AUTOLOGIN.lock().unwrap_or_else(|e| e.into_inner()) = state.stopped_autologin; *STEAMOS_TOOK_OVER.lock().unwrap_or_else(|e| e.into_inner()) = state.steamos; *STOPPED_DM.lock().unwrap_or_else(|e| e.into_inner()) = state.stopped_dm; @@ -1547,16 +1560,23 @@ fn kill_unit(unit: &str) { /// 2026-07-07). `--runtime` keeps the mask in tmpfs so a reboot clears it even if the host dies /// without restoring (the same semantics as the persisted takeover file). /// -/// ⚠ The mask only covers the UNIT path — it is NOT what stops the relogin loop itself. On images -/// whose SDDM session helper execs the session script directly (`/etc/sddm/wayland-session -/// gamescope-session-plus steam`, f43 bazzite-deck — live-diagnosed on the .41 VM 2026-07-31) the -/// relogin never touches the unit, so the mask blocks nothing: SDDM relogins ~3×/s, each a full -/// `bash --login` session start that fails against the managed instance — 328 forks/s, load 6+, -/// 1481 logind sessions in 8 minutes, the journal flooded past its own rotation. The stream itself +/// ⚠ The mask stops the UNIT from starting — it does NOT stop the relogin loop that keeps trying. +/// On images whose SDDM session helper execs the session script directly (`/etc/sddm/wayland-session +/// gamescope-session-plus steam`, f43 bazzite-deck — live-diagnosed on the .41 VM 2026-07-31) SDDM +/// relogins ~3×/s regardless, each a full `bash --login` session start — 328 forks/s, load 6+, 1481 +/// logind sessions in 8 minutes, the journal flooded past its own rotation. The stream itself /// survives, but the storm starves the game and the encoder ("atrocious, unplayable 240fps"). The -/// real defense is stopping the DM ([`dm_plan`]); the mask stays as belt-and-braces for the window -/// before the stop lands, for images that DO route the relogin through the unit, and as the -/// degraded takeover when the stop is impossible. +/// real defense against the storm is stopping the DM ([`dm_plan`]); the mask stays as belt-and-braces +/// for the window before the stop lands, and as the degraded takeover when the stop is impossible. +/// +/// ⚠⚠ The mask DOES bite on that image, which is easy to miss and was the 2026-08-10 field bug: the +/// session script's last act is `systemctl --user --wait start gamescope-session-plus@$1.service`, so +/// a masked unit makes every entry into game mode — including the user's own deliberate "Return to +/// Gaming Mode" — fail instantly, with Steam left sitting on its "Switch to Desktop…" modal forever. +/// The mask is therefore only sound while our managed session actually holds the box: the moment the +/// box leaves it (a mid-stream switch to a desktop session), [`lift_autologin_mask`] must lift it, or +/// the way back is barred until reboot (`--runtime` lives in tmpfs — which is exactly why "it works +/// again after a reboot"). fn mask_unit(unit: &str) { let _ = Command::new("systemctl") .args(["--user", "mask", "--runtime", unit]) @@ -1571,6 +1591,58 @@ fn unmask_unit(unit: &str) { .status(); } +/// Lift the takeover's runtime mask ([`mask_unit`]) on the box's own autologin units, so the box can +/// enter its own game mode again. Idempotent and cheap once lifted (the flag short-circuits), so +/// every path that hands the box back may call it unconditionally. +/// +/// Deliberately does NOT consume [`STOPPED_AUTOLOGIN`]: the mask's lifetime is shorter than the +/// takeover's. Lifting it mid-stream only says "the box may start its own gaming session again"; the +/// restore still owes that list a `start` if the box is still ours at disconnect +/// ([`do_restore_tv_session`]). +fn lift_autologin_mask() { + let mut masked = AUTOLOGIN_MASKED.lock().unwrap_or_else(|e| e.into_inner()); + if !*masked { + return; + } + *masked = false; + let units = STOPPED_AUTOLOGIN.lock().unwrap_or_else(|e| e.into_inner()); + for unit in units.iter() { + unmask_unit(unit); + } + tracing::info!( + units = ?*units, + "gamescope: lifted the takeover's runtime mask — the box can enter its own game mode again" + ); +} + +/// Does a mid-stream session switch TO `kind` end the window in which the takeover's mask is sound? +/// Only a **desktop** session does: it means the box left our managed game session for one of its +/// own, so nothing is left for the mask to defend and the user's next move — "Return to Gaming +/// Mode" — needs the unit ([`mask_unit`]). +/// +/// [`ActiveKind::Gaming`] must not lift by itself (a takeover's own managed session reads as Gaming, +/// and lifting there would void the mask for the whole stream, in exactly the storm window it exists +/// for), and neither must [`ActiveKind::None`] — a managed session momentarily down between +/// relaunches reads as `None`, and that is mid-takeover, not the end of one. +fn switch_ends_mask_window(kind: super::ActiveKind) -> bool { + use super::ActiveKind; + matches!( + kind, + ActiveKind::DesktopKde + | ActiveKind::DesktopGnome + | ActiveKind::DesktopWlroots + | ActiveKind::DesktopHyprland + ) +} + +/// The host's mid-stream session watcher calls this on every switch it confirms; see +/// [`switch_ends_mask_window`] for which ones actually lift the mask. +pub fn release_autologin_mask(switched_to: super::ActiveKind) { + if switch_ends_mask_window(switched_to) { + lift_autologin_mask(); + } +} + /// The unit name of the display manager driving this box's graphical logins, from the /// `display-manager.service` alias symlink (the Fedora/Arch/openSUSE convention every /// gamescope-session distro follows). `None` when no DM is installed (a box that boots straight @@ -2118,7 +2190,13 @@ fn honor_session_select_switch(dm: String) { "gamescope: in-stream session-select detected — restoring the display manager and \ switching the box to the desktop session" ); - // Consume the takeover state up front: from here on the box is the DM's again. + // Consume the takeover state up front: from here on the box is the DM's again. The mask goes + // FIRST and while the unit list still exists — this path discards that list, and it is the only + // record of what carries a mask. Without this the mask outlived not just the stream but the + // boot: the disconnect restore (the only other unmask) would find an empty list and lift + // nothing. It is also what lets step 1 below work at all, since the DM's autologin heads back + // into game mode through exactly this unit. + lift_autologin_mask(); std::mem::take(&mut *STOPPED_AUTOLOGIN.lock().unwrap_or_else(|e| e.into_inner())); clear_takeover(); *MANAGED_SESSION.lock().unwrap_or_else(|e| e.into_inner()) = None; @@ -2327,6 +2405,11 @@ fn stop_autologin_sessions() -> Result<()> { } let units: Vec = listed.into_iter().map(|(u, _)| u).collect(); let mut stopped = Vec::new(); + if plan.mask { + // Record that a mask is outstanding BEFORE laying it: every hand-back path lifts it off this + // flag, and one that ran between the mask and an unrecorded flag would leave it on forever. + *AUTOLOGIN_MASKED.lock().unwrap_or_else(|e| e.into_inner()) = true; + } for unit in units { if plan.mask { mask_unit(&unit); // belt-and-braces under a stopped DM; the whole defense otherwise @@ -2623,6 +2706,12 @@ fn do_restore_tv_session() { return; } } + // Unmask BEFORE taking the list (it reads that list) and unconditionally — before the + // desktop-active early return below, and before the restart loop: a unit left masked would break + // the user's own return to gaming mode until reboot. Usually already lifted by then (the + // mid-stream switch that ends the mask's window does it — [`release_autologin_mask`]), in which + // case this is a no-op. + lift_autologin_mask(); let units = std::mem::take(&mut *STOPPED_AUTOLOGIN.lock().unwrap_or_else(|e| e.into_inner())); let dm = std::mem::take(&mut *STOPPED_DM.lock().unwrap_or_else(|e| e.into_inner())); if units.is_empty() && dm.is_none() { @@ -2645,11 +2734,6 @@ fn do_restore_tv_session() { } clear_takeover(); // A3: takeover consumed — drop the persisted crash-restore marker stop_session(SESSION_UNIT); // our gamescope/Steam session, so Steam is free for the autologin - // Unmask UNCONDITIONALLY (before the desktop-active early return below): a unit left masked - // would break the user's own return to gaming mode until reboot. - for unit in &units { - unmask_unit(unit); - } *MANAGED_SESSION.lock().unwrap_or_else(|e| e.into_inner()) = None; // Only bring the gaming autologin BACK if the box is still meant to be in gaming mode. If the // user switched to a desktop session (KDE/GNOME/wlroots/Hyprland) in the meantime, don't yank @@ -3955,10 +4039,11 @@ mod tests { use super::{ cgroup_is_punktfunk_owned, cgroup_under_user_manager, connected_connector_under, display_manager_unit_under, dm_plan, dm_survives_masked_unit, game_hz, hdr_args, - is_steam_launch, missing_flags, mode_mismatch, nested_wrapper_script, plan_bind, - script_hardcodes_gamescope, sentinel_advanced, shape_dedicated_command, - xwayland_refusal_marker, BindOff, BindPlan, DmHelperError, SessionBind, - DISTRO_GAMESCOPE_PATH, X11_SOCKET_DIR, + is_steam_launch, mask_unit, missing_flags, mode_mismatch, nested_wrapper_script, plan_bind, + release_autologin_mask, script_hardcodes_gamescope, sentinel_advanced, + shape_dedicated_command, switch_ends_mask_window, unmask_unit, xwayland_refusal_marker, + BindOff, BindPlan, DmHelperError, SessionBind, AUTOLOGIN_MASKED, DISTRO_GAMESCOPE_PATH, + STOPPED_AUTOLOGIN, X11_SOCKET_DIR, }; /// The HDR spawn flags are what make a nested game render HDR at all — and their absence is @@ -4127,6 +4212,80 @@ mod tests { assert!(!p.skip && p.mask && !p.stop_dm); } + #[test] + fn only_a_desktop_switch_ends_the_mask_window() { + use crate::ActiveKind; + // The user switched the box to a desktop session mid-stream: our managed game session is + // over, so the mask defends nothing — and the "Return to Gaming Mode" that follows has to + // be able to start the unit (the distro session script starts exactly it). + for kind in [ + ActiveKind::DesktopKde, + ActiveKind::DesktopGnome, + ActiveKind::DesktopWlroots, + ActiveKind::DesktopHyprland, + ] { + assert!(switch_ends_mask_window(kind), "{kind:?}"); + } + // A takeover's own managed session reads as Gaming, so lifting here would void the mask for + // the whole stream — in exactly the SDDM-relogin window it exists for. (Coming BACK to + // gaming needs no lift either: it evidently already started.) + assert!(!switch_ends_mask_window(ActiveKind::Gaming)); + // A managed session momentarily down between relaunches reads as None. That is + // mid-takeover, not the end of one. + assert!(!switch_ends_mask_window(ActiveKind::None)); + } + + /// The same rule as above, but end-to-end against REAL systemd — that the decision is actually + /// wired to the mask, that a lift leaves the restart list intact, and that `--runtime` is what + /// comes off (a plain `unmask` does NOT clear a runtime mask, which is the whole reason the + /// stranded mask survived every non-reboot remedy in the field). + /// + /// Ignored by default: it needs a live `systemd --user` manager. On a Linux box with a session: + /// `cargo test -p pf-vdisplay -- --ignored the_mask_comes_off`. Uses a unit name nothing owns — + /// `mask` works on a non-existent unit (it is just a symlink to `/dev/null`), so this never goes + /// near the box's real gaming session. + #[test] + #[ignore = "needs a live systemd --user manager (run explicitly on a Linux box with a session)"] + fn the_mask_comes_off_only_when_the_box_takes_itself_back() { + const PROBE: &str = "punktfunk-mask-probe@lifetime-test.service"; + let is_enabled = || { + let out = std::process::Command::new("systemctl") + .args(["--user", "is-enabled", PROBE]) + .output() + .expect("systemctl --user"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + unmask_unit(PROBE); // a previous failed run must not decide this one + + // Lay the takeover's mask exactly as `stop_autologin_sessions` does. + *STOPPED_AUTOLOGIN.lock().unwrap() = vec![PROBE.to_string()]; + *AUTOLOGIN_MASKED.lock().unwrap() = true; + mask_unit(PROBE); + assert_eq!(is_enabled(), "masked-runtime"); + + // Mid-stream, with the box still ours: the mask is doing its job and must stay. `Gaming` is + // what our own managed session reads as, and `None` is one momentarily down between + // relaunches — lifting on either would void the mask for the whole stream. + release_autologin_mask(crate::ActiveKind::Gaming); + release_autologin_mask(crate::ActiveKind::None); + assert_eq!(is_enabled(), "masked-runtime"); + + // The user switched the box to its own desktop mid-stream: the window is over, and the way + // back into game mode has to be clear before they ask for it. + release_autologin_mask(crate::ActiveKind::DesktopKde); + assert_ne!(is_enabled(), "masked-runtime"); + // The restart list SURVIVES the lift: the mask's lifetime is shorter than the takeover's, + // and the disconnect restore still owes these units a `start`. + assert_eq!(STOPPED_AUTOLOGIN.lock().unwrap().as_slice(), [PROBE]); + // Idempotent — the watcher calls it on every switch it confirms. + release_autologin_mask(crate::ActiveKind::DesktopGnome); + assert_ne!(is_enabled(), "masked-runtime"); + + unmask_unit(PROBE); + STOPPED_AUTOLOGIN.lock().unwrap().clear(); + *AUTOLOGIN_MASKED.lock().unwrap() = false; + } + #[test] fn connector_status_scan() { let base = std::env::temp_dir().join(format!("pf-drm-scan-{}", std::process::id())); diff --git a/crates/pf-vdisplay/src/vdisplay/routing.rs b/crates/pf-vdisplay/src/vdisplay/routing.rs index 2fca39be..5d3494b1 100644 --- a/crates/pf-vdisplay/src/vdisplay/routing.rs +++ b/crates/pf-vdisplay/src/vdisplay/routing.rs @@ -396,6 +396,22 @@ pub fn restore_takeover_now() { #[cfg(not(target_os = "linux"))] pub fn restore_takeover_now() {} +/// Tell the takeover that the box switched to `switched_to` mid-stream. A managed takeover +/// runtime-masks the box's own autologin gaming unit so its session supervisor cannot restart it +/// underneath us — but that mask is only sound while our managed session actually holds the box. +/// Once the user has switched the box to a desktop session mid-stream, the mask defends nothing and +/// bars the way back: the distro's session script starts that very unit, so "Return to Gaming Mode" +/// fails instantly and Steam sits on its "Switch to Desktop…" modal until a reboot clears the mask. +/// Call from the mid-stream session watcher on every switch it confirms; no-op when no takeover +/// masked anything, and when the switch does not end the mask's window. +#[cfg(target_os = "linux")] +pub fn release_autologin_mask(switched_to: crate::ActiveKind) { + gamescope::release_autologin_mask(switched_to); +} + +#[cfg(not(target_os = "linux"))] +pub fn release_autologin_mask(_switched_to: crate::ActiveKind) {} + #[cfg(all(test, target_os = "linux"))] mod tests { use super::*; diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 6eae5b76..31bd0c1e 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -1144,6 +1144,13 @@ fn session_watcher_loop(tx: std::sync::mpsc::Sender, stop: Arc= DEBOUNCE => { + // Before anything about capture: a managed takeover runtime-masks the box's own + // autologin gaming unit, and a switch to a desktop session ends the window where + // that mask is sound. Left on, it silently bars the way back — the user's "Return + // to Gaming Mode" cannot start a masked unit, so Steam sits on its "Switch to + // Desktop…" modal until a reboot clears it. Ahead of the `compositor_for_kind` + // arm below on purpose: a switch we cannot follow still has to unbar the return. + vdisplay::release_autologin_mask(cur); match vdisplay::compositor_for_kind(cur) { Some(comp) => { tracing::info!(from = ?current, to = ?cur, compositor = comp.id(), From 002702bcec1cd2fb88b862a3ee80d72b94638c2c Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 10 Aug 2026 18:38:14 +0200 Subject: [PATCH 03/11] =?UTF-8?q?fix(pf-vdisplay):=20NixOS=20sessions=20we?= =?UTF-8?q?re=20undetectable=20=E2=80=94=20comm=20is=20the=20WRAPPER's=20n?= =?UTF-8?q?ame?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session probe decided "is a desktop live?" by reading /proc//comm for every process of our uid and exact-matching it against "kwin_wayland" / "gamescope" / "gnome-shell" / "Hyprland". comm is the kernel's name for the executed FILE, truncated to 15 bytes — not argv[0]. nixpkgs wraps essentially every graphical binary: wrapProgram moves the real ELF aside to `.-wrapped` and installs a wrapper under the original name, which then `exec -a "$0"`s the hidden file. So on NixOS the kernel reports `.kwin_wayland-w` (15 bytes of `.kwin_wayland-wrapped`) while ps/pgrep -a show a perfectly ordinary `kwin_wayland`, because they read argv. Measured against a live kernel: `.kwin_wayland-w`, `.kwin_wayland_w` (KWin's own kwin_wayland_wrapper), `.gamescope-wrap`, all 15 bytes. Nothing downstream could recover from that one string comparison: - detect_active_session returned ActiveKind::None on a *running* KDE desktop; - wayland_display is only resolved for a detected kind, so the connect log reported wayland="-" even though WAYLAND_DISPLAY was correct; - pick_compositor's Auto arm returns the DETECTED backend, so a live, fully working KWin sitting in available() was never chosen — every connect died "no usable compositor"; - and PUNKTFUNK_COMPOSITOR could not rescue it: pinned_at_a_dead_session consults the same probe, turning the miss into a hard error instead. No environment variable reached the comparison — the XDG_CURRENT_DESKTOP fallback in detect() is only on the pinned path. Capture itself was never at fault: a decoy process merely NAMED kwin_wayland satisfied the probe and the stream came up against the real KWin. Resolve the name through /proc//exe (the full, untruncated file name) and strip the nixpkgs decoration. Both the leading `.` and the trailing `-wrapped` are required before anything is stripped, so KWin's own real `kwin_wayland_wrapper` binary keeps its name rather than collapsing into `kwin_wayland` and handing the probe the parent's PID. The comm fast path is kept for every ordinary distro — one read, no readlink, and no name that matched before can stop matching. Also applied to foreign_gamescope_running, which had the same defect: nixpkgs wraps gamescope too, so the attach-vs-spawn ladder saw no foreign session. Tests are fixture-driven rather than spawn-driven on purpose: a stand-in has to be a real ELF that tolerates being renamed, and /bin/sleep is not one — modern coreutils is a multi-call binary that dispatches on the executable's own name, so a copy called `.kwin_wayland-wrapped` exits instantly and /proc//exe is gone before it can be read. That failure looks exactly like this resolver being broken; it cost one debugging round here and the same trap is already recorded in punktfunk-host's /proc matcher. --- .../src/vdisplay/linux/gamescope.rs | 6 +- crates/pf-vdisplay/src/vdisplay/proc.rs | 258 ++++++++++++++++++ crates/pf-vdisplay/src/vdisplay/session.rs | 11 +- 3 files changed, 269 insertions(+), 6 deletions(-) diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs index 6dcba18f..b7a06251 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs @@ -645,10 +645,12 @@ pub fn foreign_gamescope_running() -> bool { if md.uid() != uid { continue; } - let Ok(comm) = std::fs::read_to_string(e.path().join("comm")) else { + // Resolved, not a raw `comm` read: nixpkgs wraps gamescope too, so on NixOS the kernel + // reports `.gamescope-wrap` and this probe saw no foreign session at all. + let Some(comm) = crate::proc::match_name(&e.path()) else { continue; }; - if !matches!(comm.trim(), "gamescope" | "gamescope-wl") { + if !matches!(comm.as_str(), "gamescope" | "gamescope-wl") { continue; } if !descends_from(pid, our_pid) { diff --git a/crates/pf-vdisplay/src/vdisplay/proc.rs b/crates/pf-vdisplay/src/vdisplay/proc.rs index 0c38b91b..a2971875 100644 --- a/crates/pf-vdisplay/src/vdisplay/proc.rs +++ b/crates/pf-vdisplay/src/vdisplay/proc.rs @@ -111,6 +111,74 @@ pub(crate) fn current_uid() -> u32 { unsafe { libc::getuid() } } +/// The longest `/proc//comm` the kernel will report: `TASK_COMM_LEN` is 16 *including* the +/// NUL, so a name of exactly this many bytes may be a truncation of a longer one. +#[cfg(target_os = "linux")] +const COMM_MAX: usize = 15; + +/// The executable name to identify a process by, with nixpkgs wrapper decoration undone. +/// +/// `comm` is the kernel's name for the **executed file**, truncated to [`COMM_MAX`] bytes — it is +/// not `argv[0]` and not the command line. nixpkgs wraps essentially every graphical binary: +/// `wrapProgram` moves the real ELF aside to `.-wrapped` and installs a shell wrapper under +/// the original name, and that wrapper `exec -a "$0"`s the hidden file. So `ps`/`pgrep -a` show a +/// perfectly ordinary `kwin_wayland` (they read argv) while the kernel reports `.kwin_wayland-w` +/// — 15 bytes of `.kwin_wayland-wrapped`, which can never equal `kwin_wayland`. +/// +/// That is not a KDE-only detail. On NixOS `kwin_wayland`, `gamescope`, `gnome-shell` and +/// `Hyprland` are all wrapped, so an exact `comm` comparison made [`super::session`]'s probe +/// answer [`crate::ActiveKind::None`] on a visibly running desktop — and because the probe is the +/// *only* input to that decision, no environment variable could reach it: `WAYLAND_DISPLAY` was +/// correct, capture worked the moment detection was satisfied, and a `PUNKTFUNK_COMPOSITOR` pin +/// turned the miss into a hard error via `pinned_at_a_dead_session`. (sway survives by accident — +/// nixpkgs' wrapper execs a real binary that is itself still called `sway`.) +/// +/// The `comm` fast path is kept for every ordinary distro: one read, no readlink. Only a name that +/// *could* be decorated or truncated — it starts with `.`, or it is exactly [`COMM_MAX`] bytes — +/// is re-resolved through `/proc//exe`, which carries the full, untruncated file name. +/// +/// `pid_path` is a `/proc/` directory. `None` when the process vanished mid-scan. +#[cfg(target_os = "linux")] +pub(crate) fn match_name(pid_path: &std::path::Path) -> Option { + let comm = std::fs::read_to_string(pid_path.join("comm")).ok()?; + let comm = comm.trim(); + // An undecorated name short enough to be complete is already the answer. + if !comm.starts_with('.') && comm.len() < COMM_MAX { + return Some(comm.to_string()); + } + // Reading our OWN uid's `/proc//exe` needs no privilege (every caller filters on uid + // first), but it is still absent for a kernel thread and for a process exiting under us — + // in which case the truncated `comm` is the best that exists. + match std::fs::read_link(pid_path.join("exe")) + .ok() + .as_deref() + .and_then(|p| p.file_name()) + .and_then(|n| n.to_str()) + { + Some(full) => Some(undecorate(full).to_string()), + None => Some(comm.to_string()), + } +} + +/// Strip nixpkgs `wrapProgram` decoration: `.-wrapped`, plus the `_` suffixes make-wrapper +/// appends when that hidden name is already taken (a doubly-wrapped app — Qt *and* GApps). +/// +/// **Both** halves are required, and that is the load-bearing part rather than pedantry: KWin +/// ships its own real binary called `kwin_wayland_wrapper` (the session's parent process), so a +/// rule that merely stripped a `wrapper`-ish suffix would rewrite it into `kwin_wayland` and hand +/// the session probe the wrong PID. Demanding the leading `.` as well keeps it — and any genuine +/// `foo-wrapped` — under its real name. +#[cfg(target_os = "linux")] +fn undecorate(name: &str) -> &str { + let Some(rest) = name.strip_prefix('.') else { + return name; + }; + match rest.trim_end_matches('_').strip_suffix("-wrapped") { + Some(real) if !real.is_empty() => real, + _ => name, + } +} + /// Ending the *tree* the helper started, not just the process we spawned. /// /// [`std::process::Child::kill`] is one `TerminateProcess` / one `SIGKILL`: it ends exactly the @@ -280,6 +348,196 @@ mod tests { } } +/// The `comm`-vs-real-name resolution ([`match_name`]). Linux-only, because the trap it exists for +/// is a Linux kernel detail (`comm` names the executed FILE, truncated to 15 bytes) crossed with a +/// nixpkgs packaging convention. +/// +/// Driven against **fixture** `/proc/` directories rather than spawned processes, for the same +/// reason the `/proc` matcher in `punktfunk-host` learned the hard way: a stand-in has to be a real +/// ELF that tolerates being *renamed*, and `/bin/sleep` is not one. Modern coreutils (uutils on +/// Ubuntu 25.10+, busybox elsewhere) is a MULTI-CALL binary — copied to `.kwin_wayland-wrapped` it +/// prints "unknown program" and exits before `/proc` can be read, and restoring `argv[0]` does not +/// save it. That reads exactly like this resolver being broken. The truncation the fixtures encode +/// is not guessed: the strings below were measured from a live kernel (`.kwin_wayland-w`, +/// `.kwin_wayland_w`, `.gamescope-wrap` — all 15 bytes) against binaries installed and exec'd the +/// way nixpkgs does it. +#[cfg(all(test, target_os = "linux"))] +mod name_tests { + use super::*; + use std::path::{Path, PathBuf}; + + /// A fake `/proc/` directory: a `comm` file and, optionally, the `exe` symlink. Removed on + /// drop. + struct FakePid { + dir: PathBuf, + } + + impl FakePid { + /// `comm` is written exactly as the kernel would report it — i.e. already truncated. + fn new(tag: &str, comm: &str, exe: Option<&str>) -> FakePid { + let dir = std::env::temp_dir().join(format!("pf-vd-name-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("fixture dir"); + std::fs::write(dir.join("comm"), format!("{comm}\n")).expect("comm"); + if let Some(exe) = exe { + // The target need not exist: `read_link` reports the link's contents, and a real + // `/proc//exe` routinely points at a path that has since been replaced. + std::os::unix::fs::symlink( + format!("/nix/store/eeee-kwin-6.5.0/bin/{exe}"), + dir.join("exe"), + ) + .expect("exe symlink"); + } + FakePid { dir } + } + fn path(&self) -> &Path { + &self.dir + } + } + + impl Drop for FakePid { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } + } + + /// The decoration table. The `kwin_wayland_wrapper` rows are the ones that earn their keep: it + /// is a REAL KWin binary (the session's parent process), so the rule must leave it under its own + /// name in both its plain and its wrapped form rather than collapsing either into + /// `kwin_wayland` and handing the session probe the wrong PID. + #[test] + fn undecorate_strips_only_a_real_nixpkgs_wrapper() { + for (raw, want) in [ + (".kwin_wayland-wrapped", "kwin_wayland"), + (".gamescope-wrapped", "gamescope"), + (".gnome-shell-wrapped", "gnome-shell"), + (".Hyprland-wrapped", "Hyprland"), + // make-wrapper appends `_`s when the hidden name is already taken (a Qt + GApps + // double-wrap), so the underscores come off before the suffix does. + (".kwin_wayland-wrapped_", "kwin_wayland"), + (".kwin_wayland-wrapped__", "kwin_wayland"), + // Not decoration — every one of these keeps its exact name. + ("kwin_wayland", "kwin_wayland"), + ("kwin_wayland_wrapper", "kwin_wayland_wrapper"), + (".kwin_wayland_wrapper-wrapped", "kwin_wayland_wrapper"), + ("foo-wrapped", "foo-wrapped"), + (".hidden", ".hidden"), + (".-wrapped", ".-wrapped"), + ] { + assert_eq!(undecorate(raw), want, "undecorate({raw:?})"); + } + } + + /// The whole bug. Every compositor the session probe matches on is wrapped by nixpkgs, so the + /// kernel reports a truncated, decorated `comm` that can never equal the name being compared — + /// which is why `detect_active_session` answered `ActiveKind::None` on a *running* KDE desktop + /// and every connect died "no usable compositor". + #[test] + fn a_nixpkgs_wrapped_compositor_resolves_to_its_real_name() { + for (tag, comm, exe, want) in [ + ( + "kwin", + ".kwin_wayland-w", + ".kwin_wayland-wrapped", + "kwin_wayland", + ), + ( + "gamescope", + ".gamescope-wrap", + ".gamescope-wrapped", + "gamescope", + ), + ( + "gnome", + ".gnome-shell-wr", + ".gnome-shell-wrapped", + "gnome-shell", + ), + ("hypr", ".Hyprland-wrapp", ".Hyprland-wrapped", "Hyprland"), + ] { + let p = FakePid::new(tag, comm, Some(exe)); + assert_eq!( + match_name(p.path()).as_deref(), + Some(want), + "a nixpkgs-wrapped {want} must resolve to the name the session probe matches" + ); + } + } + + /// KWin's own `kwin_wayland_wrapper` is a real binary that runs *alongside* `kwin_wayland`, and + /// its wrapped `comm` (`.kwin_wayland_w`) differs from the compositor's by a single byte. It + /// must NOT resolve to `kwin_wayland`: the probe would then match the parent process and carry + /// its PID as the compositor identity, which drives restart detection. + #[test] + fn kwins_own_wrapper_binary_does_not_masquerade_as_the_compositor() { + let p = FakePid::new( + "kwrap", + ".kwin_wayland_w", + Some(".kwin_wayland_wrapper-wrapped"), + ); + assert_eq!( + match_name(p.path()).as_deref(), + Some("kwin_wayland_wrapper") + ); + } + + /// The other half of the 15-byte limit, with no nix involved: a long name is truncated too, and + /// has to be recovered from `exe` rather than matched short. + #[test] + fn a_long_name_is_recovered_untruncated() { + let p = FakePid::new( + "long", + "a-very-long-com", + Some("a-very-long-compositor-name"), + ); + assert_eq!( + match_name(p.path()).as_deref(), + Some("a-very-long-compositor-name") + ); + } + + /// The fast path answers without consulting `exe` at all — which is what keeps this probe at one + /// read per process on every ordinary distro, and what lets it answer for a process whose `exe` + /// is unreadable in the first place. + #[test] + fn an_ordinary_short_name_never_needs_the_exe_link() { + let p = FakePid::new("plain", "kwin_wayland", None); + assert_eq!(match_name(p.path()).as_deref(), Some("kwin_wayland")); + } + + /// A decorated-or-truncated name whose `exe` cannot be read (a kernel thread, or a process + /// exiting under the scan) degrades to the truncated `comm` instead of failing the whole entry. + #[test] + fn an_unreadable_exe_falls_back_to_comm() { + let p = FakePid::new("noexe", ".kwin_wayland-w", None); + assert_eq!(match_name(p.path()).as_deref(), Some(".kwin_wayland-w")); + } + + /// A pid directory that does not exist yields `None`, not a bogus name — the scans `continue`. + #[test] + fn a_vanished_process_yields_none() { + assert_eq!(match_name(Path::new("/proc/0")), None); + } + + /// The one thing a fixture cannot establish: that reading `/proc//exe` is actually + /// *permitted* for a process of our own uid, which the whole resolver depends on. Checked + /// against the only such process guaranteed to be running — this one. + #[test] + fn our_own_exe_link_is_readable() { + let me = Path::new("/proc/self"); + let exe = std::fs::read_link(me.join("exe")) + .expect("/proc/self/exe must be readable for our own uid"); + let name = exe.file_name().and_then(|n| n.to_str()).expect("exe name"); + let got = match_name(me).expect("our own name"); + // Whichever rung answered, it must agree with the real binary: the fast path returns the + // (short, undecorated) comm, which is a prefix of it; the exe path returns it outright. + assert!( + name.starts_with(got.as_str()) || got == name, + "resolved {got:?} disagrees with our real binary {name:?}" + ); + } +} + /// The same two cases through `cmd /c`, so the budget logic is covered on the platform whose /// process model differs most (job objects, no `SIGKILL`). `ping -n` is the standard Windows /// no-extra-tooling sleep. diff --git a/crates/pf-vdisplay/src/vdisplay/session.rs b/crates/pf-vdisplay/src/vdisplay/session.rs index d95e6cc9..e99b6f88 100644 --- a/crates/pf-vdisplay/src/vdisplay/session.rs +++ b/crates/pf-vdisplay/src/vdisplay/session.rs @@ -311,8 +311,11 @@ pub fn detect_active_session() -> ActiveSession { let dbus = default_bus(&env, &xdg_runtime_dir); // Process probe: the running graphical compositor of THIS uid decides the kind. Priority lets - // a real desktop (kwin/gnome/sway) win over a leftover gamescope child. comm names mirror the - // `pkill -x` discipline (exact, ≤15 chars so untruncated). + // a real desktop (kwin/gnome/sway) win over a leftover gamescope child. Names are matched + // exactly, `pkill -x` style — but resolved through [`crate::proc::match_name`], NOT a raw + // `comm` read: on NixOS every one of these binaries is a nixpkgs wrapper whose real ELF is + // `.-wrapped`, so a raw `comm` says `.kwin_wayland-w` and this whole probe answered + // `None` on a running KDE desktop. let mut kind = ActiveKind::None; let mut best = 0u8; // The winning compositor's PID — kept so a same-kind compositor RESTART (a new PID) bumps the @@ -332,10 +335,10 @@ pub fn detect_active_session() -> ActiveSession { if md.uid() != uid { continue; } - let Ok(comm) = std::fs::read_to_string(pid_path.join("comm")) else { + let Some(comm) = crate::proc::match_name(&pid_path) else { continue; }; - let (k, prio) = match comm.trim() { + let (k, prio) = match comm.as_str() { "gamescope" | "gamescope-wl" => (ActiveKind::Gaming, 1), "kwin_wayland" => (ActiveKind::DesktopKde, 4), "gnome-shell" => (ActiveKind::DesktopGnome, 4), From 159bbdbfc246246244fd32af49114bdb6df2698c Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 10 Aug 2026 19:27:14 +0200 Subject: [PATCH 04/11] fix(nix): port three NixOS-module divergences from the shipped systemd units MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sweep of the Nix packaging against the units the deb/rpm actually install found three decisions that were made, documented and deliberate everywhere else, and simply not carried into packaging/nix/nixos-module.nix. punktfunk-web — StartLimitIntervalSec=0. The unit's EnvironmentFile for the mgmt token is mandatory ON PURPOSE, so the console genuinely fails until the host's first `serve` writes it. systemd's default rate limit (5 starts / 10 s) against RestartSec=2 then gives up permanently after ~10 s — which on an appliance is exactly the window before the host is ready, so a console enabled before the host's first run stayed dead until someone restarted it by hand. scripts/punktfunk-web.service has carried the override since that defect was found; the Nix module omitted it while its own comment went on promising "Restart retries until the host has created it". punktfunk-web — Restart=always, not on-failure. A console that exits 0 has still stopped serving, and on-failure leaves it down. Matches the shipped unit and web-run.cmd on Windows, both of which relaunch bun on ANY exit. An explicit `systemctl --user stop` is unaffected. punktfunk-scripting — the sandbox was missing entirely. The shipped unit confines the runner with NoNewPrivileges, ProtectSystem= strict, ReadWritePaths=%h /tmp and an AF_UNIX/AF_INET/AF_INET6 address-family restriction, plus PrivateTmp=no (a field report: a private /tmp hides /tmp/vhclient and /tmp/.X11-unix, so a plugin launches its vendor binary and then cannot reach the daemon behind it). The NixOS unit had none of it — so the one unit here that executes arbitrary operator TypeScript by design ran strictly LESS confined on NixOS than on every other channel. Verified by evaluating the module against the pinned nixpkgs and rendering the units: assertions clean, cap_sys_nice=ep on the encode-worker wrapper, firewall 47984/47989/47990/47992/47993/48010, and each unit carrying exactly the directives above. That evaluation is NOT something CI does — measured: `nix flake check` passes a nixosModule containing a nonexistent option, a nonexistent pkgs attribute and a nonexistent lib function, printing "checking NixOS module ... all checks passed!" while never evaluating it against nixpkgs. nix.yml's header claims that leg covers the module. It does not; tracked separately. --- packaging/nix/nixos-module.nix | 47 +++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/packaging/nix/nixos-module.nix b/packaging/nix/nixos-module.nix index f7dfef8f..e66b72cf 100644 --- a/packaging/nix/nixos-module.nix +++ b/packaging/nix/nixos-module.nix @@ -508,6 +508,15 @@ in ]; wants = [ "punktfunk-web-init.service" ]; wantedBy = optional cfg.web.autoStart "default.target"; + # Retry INDEFINITELY while the host is still writing the mgmt token + identity cert. The + # EnvironmentFile below is mandatory on purpose, so the unit genuinely fails until those + # exist — and systemd's default rate limit (5 starts / 10 s) against `RestartSec = 2` gives + # up permanently after ~10 s, which on an appliance is exactly the window before the host's + # first `serve` completes. A console enabled before the host's first run then stayed dead + # until someone restarted it by hand. The shipped unit (scripts/punktfunk-web.service) has + # carried this since that defect was found; it was missed in the port, while the comment + # below went on promising the behaviour it removes. + unitConfig.StartLimitIntervalSec = 0; environment = { PUNKTFUNK_MGMT_URL = "https://127.0.0.1:47990"; PORT = "47992"; @@ -525,7 +534,11 @@ in "-%h/.config/punktfunk/web-password" ]; ExecStart = "${cfg.web.package}/bin/punktfunk-web-server"; - Restart = "on-failure"; + # `always`, not `on-failure`: a console that exits 0 has still stopped serving, and + # `on-failure` would leave it down. An explicit `systemctl --user stop` is still honoured + # (Restart= never fights that). Matches scripts/punktfunk-web.service and the Windows + # web-run.cmd, both of which relaunch bun on ANY exit. + Restart = "always"; RestartSec = 2; }; }; @@ -554,6 +567,38 @@ in KillMode = "mixed"; KillSignal = "SIGTERM"; TimeoutStopSec = 30; + + # Sandbox — the same confinement scripts/punktfunk-scripting.service gives the deb/rpm + # installs. The runner `import()`s the operator's own `.ts` files, so this is the one unit + # here that executes arbitrary code by design; without these it ran strictly LESS confined + # on NixOS than on every other channel. Read-only outside $HOME, no setuid re-escalation, + # and only the address families automation actually uses (loopback mgmt API, LAN/IPv6 + # webhooks, unix sockets). + NoNewPrivileges = true; + # PrivateTmp deliberately OFF (field report 2026-08-03, the VirtualHere plugin). A + # plugin's whole job is integrating with things already running on this box, and on Linux + # those talk over /tmp: VirtualHere's client IPC is the FIFO pair /tmp/vhclient + + # /tmp/vhclient_response, X11 is /tmp/.X11-unix. A private /tmp hides all of it — the + # plugin launches the vendor binary fine and then cannot reach the daemon behind it, + # which presents as an error no amount of config fixes. + PrivateTmp = false; + ProtectSystem = "strict"; + # ReadWritePaths puts back the write bit ProtectSystem=strict takes away: plugin state and + # ~/.config/punktfunk under $HOME, plus the /tmp above. A plugin that must write OUTSIDE + # $HOME (a game library on another mount) gets it with + # systemctl --user edit punktfunk-scripting → [Service] ReadWritePaths=/mnt/games + # ⚠ ProtectSystem is a MOUNT-NAMESPACE option, and for a *user* unit that needs + # unprivileged user namespaces. On a kernel/config that restricts those it fails the unit + # rather than degrading — drop it via the same drop-in if this box is one of them. + ReadWritePaths = [ + "%h" + "/tmp" + ]; + RestrictAddressFamilies = [ + "AF_UNIX" + "AF_INET" + "AF_INET6" + ]; }; }; }) From e93947969f4b3ac3d80fc79905d16252990c81d2 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Mon, 10 Aug 2026 19:55:13 +0200 Subject: [PATCH 05/11] =?UTF-8?q?fix(console):=20the=20hide=20button=20was?= =?UTF-8?q?=20invisible=20AND=20clickable=20=E2=80=94=20a=20corner=20nobod?= =?UTF-8?q?y=20could=20see=20dropped=20games?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hiding a library title shipped in 0.26.0 and was, in practice, unusable: the operator reported there was simply no button, then found it by CLICKING the empty top-right corner of a poster. Both halves of that are the same defect. The card's control row was `opacity-0` until `group-hover`, and `opacity-0` paints nothing while still HIT-TESTING. So the corner of every tile in the grid was a live hide button with no visual presence at all: a stray click there removed that title from every play surface — the client grid, the native clients, the GameStream app list — with nothing on screen having suggested a control was under the cursor. What read as "the button finally appeared" was the hide taking effect, since `hidden` is the one state that drops the `opacity-0`. The feature announced itself by firing. And the reveal rested on hover ALONE. `:hover` never fires on a touch screen, so on a tablet the hide control was unreachable by construction and discoverable only by the blind click above. The original commit spotted this hazard for UN-hide — it kept those controls always-visible so nobody could be stranded in the hidden state — but left the hide side hover-gated, which is the same trap one step earlier. So opacity and `pointer-events` now move together, always: whatever cannot be seen cannot be clicked. `pointer-coarse:` shows the row outright wherever the device has no hover to give, rather than making touch a second-class path. Keyboard reach is unchanged — `pointer-events: none` does not block focus, so tabbing in still trips `focus-within`, which now restores interactivity along with opacity. The eye icon also gains a `title`. On a scanned entry it is the ONLY control on the card, with no edit/delete beside it to read as a toolbar, and an unlabelled eye-with-slash is not a promise that a game is about to leave the library. Verified in the built CSS rather than by eye, because a variant that does not compile fails exactly like the bug being fixed: `@media(pointer:coarse)` emits both `pointer-coarse:opacity-100` and `pointer-coarse:pointer-events-auto`, and it lands at the END of the sheet — media queries add no specificity, so this tie against the base `.pointer-events-none` / `.opacity-0` is won on source order, not by accident. The `group-hover:` and `focus-within:` forms compile to `:is(:where(.group):hover *)` and `:focus-within`, carrying a pseudo-class each, so they win on specificity outright. Console: tsc clean, production build clean, biome clean on the touched file, i18n 633 messages across en+de. --- web/src/sections/Library/GameCard.tsx | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/web/src/sections/Library/GameCard.tsx b/web/src/sections/Library/GameCard.tsx index fdb5d723..ebfcf2e4 100644 --- a/web/src/sections/Library/GameCard.tsx +++ b/web/src/sections/Library/GameCard.tsx @@ -122,10 +122,22 @@ export const GameCard: FC = ({ {/* A hidden card keeps its controls VISIBLE rather than hover-revealed. Hover-to-reveal is fine for an ordinary tile, but the un-hide button is the only way out of the hidden state — requiring a hover to discover it would strand anyone on a touch - screen, which is exactly where the console's pointer work landed. */} + screen, which is exactly where the console's pointer work landed. + Two rules that used to be one, because `opacity-0` alone got BOTH of them wrong: + 1. Invisible must also mean UNCLICKABLE. An `opacity-0` element paints nothing and + still hit-tests, so the top-right corner of every poster was a live hide button + nobody could see — a stray click there dropped that title from every play + surface with no visible cause. Opacity and `pointer-events` move together now. + 2. The reveal cannot rest on hover ALONE. `:hover` never fires on a touch screen, + so the control was unreachable there by construction and discoverable only by + blind-clicking the corner. `pointer-coarse` shows it outright wherever the + device has no hover to give. Keyboard reach is unaffected: `pointer-events: + none` does not block focus, so tabbing in still trips `focus-within`. */}