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

enricobuehler commented 2026-08-22 18:42:34 +00:00

Owner

Copy Link

Copy Source

Two commits from one field report ("controller still dead on GameStream clients; also, every paired device is called CN=NVidia Gamestream Client").

539ac2f2 — name a paired Moonlight device

The names are 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. A user with a phone, a TV and a Switch saw three identical rows, with only a fingerprint prefix to tell them apart — worst exactly when choosing which one to unpair.

• client-labels.json, a sidecar to paired.json rather than a field in it. paired.json is a bare Vec<Vec<u8>> of DERs and decides who may connect; giving it a shape would be a migration on the trust store, and a cosmetic label must never be able to lock anyone out. Same atomic temp+rename as save_paired.
• PATCH /api/v1/clients/{fingerprint} sets/clears, GET /clients grows label. Whitespace-only clears rather than storing a blank name; only an already-paired fingerprint may be named; unpairing forgets the label.
• Scrubbing reuses native_pairing::sanitize_device_name instead of growing a second one. Not cosmetic here: the label is the only thing distinguishing two paired devices, so an unscrubbed bidi override 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 route takes the DELETE's auth lanes (neither plugin token nor paired cert may reach it), not the roster GET's read permission — pinned by the lane test.
• Console: a pencil on Moonlight rows opens the existing promptText, seeded with the current label (not the CN=… fallback, or every rename would start by deleting boilerplate). Native rows keep their pairing-supplied name.

f5a75d9e — a devtest probe, and a hypothesis it killed

UhidManager::heartbeat's own doc says a UMDF pad reads a multi-second input silence as an unplug; the native plane calls it every tick and SessionPads::pump_rumble does not. That looked decisive, because the planes differ exactly where it would show: our own client re-sends every pad's snapshot every 100 ms (input_task.rs refresh tick), while moonlight-common-c sends a controller packet only on change.

--idle-after N / --resume-after M test precisely that. Measured on .173: it does not reproduce — the pad sat through 58 s of total silence with its devnode at Status=OK and its promoted HID\PUNKTFUNK&IG_00 child intact. So the one-line "add a heartbeat" fix this was going to justify is not warranted, and is not in this PR. The probe is, so the next person doesn't re-derive the same dead end.

Recorded in the same commit because it is real, just not this bug: two live processes wanting pad index 0 collide exactly as PadCreateFault::IndexOwnedElsewhere describes, and dfcffcdd put both input planes on that one mailbox name (Global\pfds-boot-0) where before GameStream used Global\pfxusb-boot-0 and they could never collide. A clean release-then-retake does not collide (0 s/1 s/3 s gaps all fine), so an ordinary reconnect is not the trigger.

On the original gamepad report

dfcffcdd works. Verified on .173 with 0.31.2 across four GameStream sessions, three after an explicit exit→reconnect — the exact sequence the reporter says kills it — building the pad cleanly every time (controller arrival (Xbox Wireless/Windows) → virtual Xbox pad created (Windows UMDF HID) → driver attached … late=false). No creation failed, no index-owned-elsewhere, no dropped input.

So the reporter's host is not doing what ours does, and one grep of their host.log for controller arrival says which:

line	meaning
controller arrival (Xbox Wireless/Windows)	on the fix, HID pad — healthy
controller arrival (Xbox 360/Windows)	XUSB companion — host predates v0.31.1, or PUNKTFUNK_XBOX_BACKEND=xusb is set
virtual … creation failed … fault="index-owned-elsewhere"	a second live process holds pad 0

Also ruled out on the box, by measurement rather than reasoning: the driver package (pf_gamepad.inf 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, read as GRANT_ALL).

Verification

• cargo test -p punktfunk-host mgmt:: on .173 → 58 passed, including the new client_label_round_trips_scrubs_and_is_forgotten_on_unpair and both guardrails that caught this mid-flight (route-lane classification, OpenAPI freshness).
• cargo clippy -p punktfunk-host --all-targets -- -D warnings run on Windows — windows-host.yml never runs on pull_request, and the devtest.rs change is cfg(target_os = "windows"), so a green PR here would have proven nothing about it.
• Web tsc --noEmit clean; bun run build clean; i18n check passes for en + de.

Two pre-existing things in the diff, verified as such rather than assumed

• sdk/src/gen/punktfunk.ts is bigger than the feature. Regenerating from the unchanged committed spec already yields a ~700-line diff — the checked-in copy had drifted from its own pinned generator, and 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 first: the only differences are PATCH /clients/{fingerprint}, the RenameClient schema and PairedClient.label — no OS-driven drift.

Not fixed here, but found

mgmt::tests::display_monitors_answers_even_with_no_compositor fails on Windows at HEAD too: it gets 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 — a real question, so it is left for someone to answer rather than papered over.

Two commits from one field report ("controller still dead on GameStream clients; also, every paired device is called `CN=NVidia Gamestream Client`"). ## `539ac2f2` — name a paired Moonlight device The names are 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. A user with a phone, a TV and a Switch saw three identical rows, with only a fingerprint prefix to tell them apart — worst exactly when choosing which one to unpair. - `client-labels.json`, a **sidecar** to `paired.json` rather than a field in it. `paired.json` is a bare `Vec<Vec<u8>>` of DERs and decides who may connect; giving it a shape would be a migration on the trust store, and a cosmetic label must never be able to lock anyone out. Same atomic temp+rename as `save_paired`. - `PATCH /api/v1/clients/{fingerprint}` sets/clears, `GET /clients` grows `label`. Whitespace-only clears rather than storing a blank name; only an already-paired fingerprint may be named; unpairing forgets the label. - Scrubbing reuses `native_pairing::sanitize_device_name` instead of growing a second one. Not cosmetic here: the label is the only thing distinguishing two paired devices, so an unscrubbed bidi override 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 route takes the **DELETE's** auth lanes (neither plugin token nor paired cert may reach it), not the roster GET's read permission — pinned by the lane test. - Console: a pencil on Moonlight rows opens the existing `promptText`, seeded with the current label (not the `CN=…` fallback, or every rename would start by deleting boilerplate). Native rows keep their pairing-supplied name. ## `f5a75d9e` — a devtest probe, and a hypothesis it killed `UhidManager::heartbeat`'s own doc says a UMDF pad reads a multi-second input silence as an unplug; the native plane calls it every tick and `SessionPads::pump_rumble` does not. That looked decisive, because the planes differ exactly where it would show: our own client re-sends every pad's snapshot every 100 ms (`input_task.rs` refresh tick), while moonlight-common-c sends a controller packet **only on change**. `--idle-after N` / `--resume-after M` test precisely that. **Measured on `.173`: it does not reproduce** — the pad sat through 58 s of total silence with its devnode at `Status=OK` and its promoted `HID\PUNKTFUNK&IG_00` child intact. So the one-line "add a heartbeat" fix this was going to justify is **not** warranted, and is not in this PR. The probe is, so the next person doesn't re-derive the same dead end. Recorded in the same commit because it *is* real, just not this bug: two live processes wanting pad index 0 collide exactly as `PadCreateFault::IndexOwnedElsewhere` describes, and `dfcffcdd` put both input planes on that one mailbox name (`Global\pfds-boot-0`) where before GameStream used `Global\pfxusb-boot-0` and they could never collide. A clean release-then-retake does *not* collide (0 s/1 s/3 s gaps all fine), so an ordinary reconnect is not the trigger. ## On the original gamepad report `dfcffcdd` **works**. Verified on `.173` with 0.31.2 across four GameStream sessions, three after an explicit exit→reconnect — the exact sequence the reporter says kills it — building the pad cleanly every time (`controller arrival (Xbox Wireless/Windows)` → `virtual Xbox pad created (Windows UMDF HID)` → `driver attached … late=false`). No `creation failed`, no `index-owned-elsewhere`, no dropped input. So the reporter's host is not doing what ours does, and one grep of their `host.log` for `controller arrival` says which: | line | meaning | | --- | --- | | `controller arrival (Xbox Wireless/Windows)` | on the fix, HID pad — healthy | | `controller arrival (Xbox 360/Windows)` | XUSB companion — host predates v0.31.1, or `PUNKTFUNK_XBOX_BACKEND=xusb` is set | | `virtual … creation failed … fault="index-owned-elsewhere"` | a second live process holds pad 0 | Also ruled out on the box, by measurement rather than reasoning: the driver package (`pf_gamepad.inf` 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, read as `GRANT_ALL`). ## Verification - `cargo test -p punktfunk-host mgmt::` on `.173` → **58 passed**, including the new `client_label_round_trips_scrubs_and_is_forgotten_on_unpair` and both guardrails that caught this mid-flight (route-lane classification, OpenAPI freshness). - `cargo clippy -p punktfunk-host --all-targets -- -D warnings` run **on Windows** — `windows-host.yml` never runs on `pull_request`, and the `devtest.rs` change is `cfg(target_os = "windows")`, so a green PR here would have proven nothing about it. - Web `tsc --noEmit` clean; `bun run build` clean; i18n check passes for en + de. ## Two pre-existing things in the diff, verified as such rather than assumed - **`sdk/src/gen/punktfunk.ts` is bigger than the feature.** Regenerating from the *unchanged committed spec* already yields a ~700-line diff — the checked-in copy had drifted from its own pinned generator, and 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 first: the only differences are `PATCH /clients/{fingerprint}`, the `RenameClient` schema and `PairedClient.label` — no OS-driven drift. ## Not fixed here, but found `mgmt::tests::display_monitors_answers_even_with_no_compositor` fails on Windows **at HEAD too**: it gets `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 — a real question, so it is left for someone to answer rather than papered over.

enricobuehler added 2 commits 2026-08-22 18:42:38 +00:00

test(devtest): drive a Windows HID pad through silence and back, to test what a Moonlight client actually does ... f5a75d9edc

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.

feat(host,web): name a paired Moonlight device, because its certificate never will ... 539ac2f2a5

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.

enricobuehler added 1 commit 2026-08-22 18:57:15 +00:00

test(mgmt): one config-dir override for the file, not one per test ... 4a32c8fb36

`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.

enricobuehler merged commit 2b91339cb8 into main 2026-08-22 19:22:03 +00:00

enricobuehler referenced this issue from a commit 2026-08-22 22:54:41 +00:00

release: 0.31.3 — version bump, notes, CHANGELOG, Play notes

enricobuehler referenced this issue from a commit 2026-08-23 10:01:50 +00:00

release: 0.31.3 — version bump, notes, CHANGELOG, Play notes

enricobuehler referenced this pull request 2026-08-23 10:02:22 +00:00

0.31.3 — the launch that dropped, the refresh a TV never output, and the probe that choked the link #386

enricobuehler referenced this issue from a commit 2026-08-23 10:13:24 +00:00

chore(sdk): cut 0.1.6 — the rename route's types cannot reach a plugin until they ship

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

FernandoMX FlipperDan HitFrostbite bluelightspecial enricobuehler (Enrico Bühler) kszlim luxus nannku neoney

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#374