A paired Moonlight device can be given a name, and the pad-silence theory is measured and dropped #374

Merged

enricobuehler merged 3 commits from worktree-gamestream-pad-heartbeat into main 2026-08-22 19:22:03 +00:00

Conversation 0 Commits 3 Files Changed 14

+1089 -96

3 Commits

Author	SHA1	Message	Date
enricobuehler	4a32c8fb36	test(mgmt): one config-dir override for the file, not one per test ... `ci / rust` failed the unsafe-hygiene gate: mgmt/tests.rs went to 6 process-global-API mentions against a baseline of 3. The new rename test had copy-pasted the existing `EnvGuard` + CONFIG_DIR_TEST_LOCK + tempdir dance, which is exactly the duplication gate C exists to catch — its advice is to fix the call site rather than raise the baseline. So there is now ONE `ConfigDirOverride` both tests use. It also makes the pairing harder to get wrong than the copies were: the lock is a FIELD rather than a separate `_serial` binding a test could forget, and since `Drop::drop` runs before any field drops, the environment is restored while the guard still holds the lock. Back to 3 mentions, and `sh scripts/ci/check-unsafe-hygiene.sh` reports all three gates clean. Note the last one is a PROSE mention: the grep counts comments too (deliberately — "keep it dumb and stable"), so the doc comment had to stop naming the function it warns about. Not re-run on .173: the box went off-network mid-change. It does not need to be — this is `mgmt/tests.rs`, which is not Windows-gated, so Linux CI compiles and runs it. The Windows-only verification (clippy over the `cfg(target_os = "windows")` devtest change) was already done and that file is untouched here.	2026-08-22 20:57:05 +02:00
enricobuehler	539ac2f2a5	feat(host,web): name a paired Moonlight device, because its certificate never will ... Reported from the field: "is there a possibility of renaming the moonlight paired devices? as they're all named CN=NVidia Gamestream Client". They are, and it is not a display bug — every moonlight-common-c client self-signs with that same fixed subject, so the certificate carries no device identity at all. Until now the console listed that string for every Moonlight row, which means a user with a phone, a TV and a Switch saw three identical rows and had nothing but a fingerprint prefix to tell them apart — most sharply when deciding which one to unpair. The name is an operator-supplied label, stored host-side keyed by fingerprint: * `client-labels.json`, a SIDECAR to `paired.json` rather than a field inside it. `paired.json` is a bare `Vec<Vec<u8>>` of DERs, so giving it a shape would be a migration on the one file that decides who may connect — and a label is not part of that trust decision, so a corrupt or missing label file must never be able to lock anyone out. Same atomic temp-file + rename as `save_paired`. * `PATCH /api/v1/clients/{fingerprint}` sets or clears it; `GET /clients` grows a `label`. A whitespace-only body clears rather than storing a blank name, and only an already-paired fingerprint may be named (a label for an unknown one would be invisible and never cleaned up). Unpairing forgets the label, so the file cannot grow without bound and a re-pairing of the same certificate starts unnamed. * Scrubbing reuses `native_pairing::sanitize_device_name` rather than growing a second one: it already strips C0/C1 controls and Unicode bidi overrides and caps at 64. That is not cosmetic here — the label is the ONLY thing distinguishing two paired devices in the console, so an unscrubbed one could dress a stranger's device up as the operator's TV and be spared an unpair on that basis. For the same reason the new route takes the plugin/cert lanes of the DELETE beside it (neither may reach it), not the roster GET's read permission; the lane test now pins that. * Console: a pencil on Moonlight rows opens the existing `promptText` dialog seeded with the current label (not the `CN=…` fallback, or every rename would start by deleting boilerplate). Native rows keep their pairing-supplied name and get no pencil. Test: one round trip through the API — name it, see it in the list, watch the bidi override and the whitespace collapse get scrubbed, clear it two ways, reject a malformed and an unpaired fingerprint, and assert the unpair forgot it on disk. VERIFIED on .173 (the Windows box, since punktfunk-host does not build on macOS): `cargo test -p punktfunk-host mgmt::` → 58 passed, including the new `client_label_round_trips_scrubs_and_is_forgotten_on_unpair` and both guardrails that caught this work in progress (`every_route_is_classified_for_the_plugin_and_cert_lanes` and `openapi_document_is_complete_and_checked_in`). Web `tsc --noEmit` clean. Two notes on the diff, both PRE-EXISTING and verified as such rather than assumed: * `sdk/src/gen/punktfunk.ts` is bigger than this feature. Regenerating it from the UNCHANGED committed spec already produces a ~700-line diff, i.e. the checked-in copy had drifted from its own pinned generator — nothing in CI regenerates or verifies it. This lands the clean regeneration rather than hand-patching generated code. * `api/openapi.json` was regenerated on Windows, not CI's Linux. Checked structurally before committing: the only differences are `PATCH /clients/{fingerprint}`, the `RenameClient` schema and `PairedClient.label` — no OS-driven drift. Unrelated and NOT touched: `mgmt::tests::display_monitors_answers_even_with_no_compositor` fails on Windows, at HEAD as well. It answers `compositor="windows", monitors=[], error=null`, and the test's escape hatches only cover gamescope, an absent compositor or an error. Either the test needs a Windows arm or Windows display enumeration is returning nothing it should — that is a real question, so it is left for someone to answer rather than papered over here.	2026-08-22 19:31:43 +02:00
enricobuehler	f5a75d9edc	test(devtest): drive a Windows HID pad through silence and back, to test what a Moonlight client actually does ... Chasing "gamepad still dead on GameStream clients after dfcffcdd" (Artemis on Android, Moonlight on a Switch; both report only mouse/touch working). dfcffcdd moved this plane from the XUSB companion to the UMDF HID Xbox pad and was verified by `cargo check` + `clippy` only, so nothing about it had ever run. The suspicion this flag was built to test: `UhidManager` has a `heartbeat` whose own doc says a UMDF pad "treats a multi-second input silence as an unplugged controller", the native plane calls it every tick, and `SessionPads::pump_rumble` does not. That asymmetry looked decisive because the two planes differ in exactly the way that would expose it: punktfunk's own client re-sends every live pad's snapshot every 100 ms unconditionally (`input_task.rs` refresh tick), so a native pad is never silent, while moonlight-common-c sends a controller packet only on CHANGE — an untouched pad emits nothing at all. `--idle-after N` stops the state frames while still pumping; `--resume-after M` starts them again, because enumeration surviving a silence proves nothing on its own (a pad can stay listed and deliver no input) — what matters is whether a report written after the silence still lands. MEASURED on .173 (Win11 26200), and it does NOT reproduce: with `--xboxhid --idle-after 12 --seconds 75`, the pad sat through 58 s of total input silence with `SWD\PUNKTFUNK\PF_XBOX_0` at Status=OK and its promoted `HID\PUNKTFUNK&IG_00` child still present the whole time. So the heartbeat gap is NOT the field bug, and the one-line "add a heartbeat to the GameStream arm" fix this was going to justify is not warranted — which is the point of landing the probe rather than the guess. Also measured with the same binary, and worth recording because it IS real: * two LIVE processes wanting pad index 0 collide exactly as `PadCreateFault:: IndexOwnedElsewhere` describes (`Global\pfds-boot-0`, ACCESS_DENIED because the mailbox DACL is SYSTEM+LocalService). dfcffcdd put BOTH input planes on that one name — before it, GameStream used `Global\pfxusb-boot-0` and the two could never collide — so the hazard is new, even if it is not what the reporter hit. * a clean release-then-retake does NOT collide: back-to-back runs at 0 s, 1 s and 3 s gaps all created their pad, so an ordinary client reconnect is not the trigger. Ruled out on the same box while here: the driver package (`pf_gamepad.inf` 08/18 declares all three Xbox hwids and the `xinputhid` promotion), stale drivers in the field (the Windows updater is a full Inno Setup run that re-runs `driver install --gamepad`), and access grants (a Moonlight fingerprint has no grants record, which `control.rs` reads as GRANT_ALL). Still open, and it needs a live session: .173 runs `PUNKTFUNK_HOST_CMD=serve`, i.e. GameStream is switched OFF, so this box has never exercised the plane dfcffcdd changed. That is how a compile-only fix reached users unexercised, and it is the first thing to change before the next attempt.	2026-08-22 18:58:57 +02:00