Merge pull request 'Uninstalling the Windows host left every audio device it minted behind forever — and the installer script documented that as a decision' (#145) from worktree-win-audio-uninstall-cleanup into main

Reviewed-on: #145

crates/punktfunk-host/src/audio.rs

@@ -199,6 +199,11 @@ pub(crate) mod audio_probe;
 #[cfg(target_os = "windows")]
 #[path = "audio/windows/minted.rs"]
 pub(crate) mod minted;
+// The uninstall sweep over every audio devnode the two providers above (and the probe) mint —
+// pub(crate) for `driver uninstall --audio`, the installer's [UninstallRun] leg.
+#[cfg(target_os = "windows")]
+#[path = "audio/windows/devnode_cleanup.rs"]
+pub(crate) mod devnode_cleanup;
 #[cfg(target_os = "windows")]
 #[path = "audio/windows/wasapi_cap.rs"]
 mod wasapi_cap;

crates/punktfunk-host/src/audio/windows/audio_control.rs

@@ -406,6 +406,34 @@ fn recover_orphaned_default() {
     });
 }
 
+/// [`recover_orphaned_default`]'s uninstall-time twin: same "put the operator's device back if
+/// the default is still parked on ours" rule, minus the `Once` gate (the uninstaller is a fresh
+/// process that runs it exactly once) — and it always drops the marker file, because there is no
+/// next host run to consume it.
+///
+/// Why the uninstaller needs this at all: the devnode sweep that follows deletes the endpoint the
+/// default may still point at. Windows would then re-pick something on its own, but it re-picks by
+/// its OWN ranking, not the device the operator had before we parked it. Restoring first means
+/// uninstalling gives the box back exactly the default it came with.
+///
+/// Returns whether a device was actually put back — the caller only logs it.
+pub(crate) fn unpark_default_for_uninstall() -> bool {
+    let path = park_marker_path();
+    let Ok(s) = std::fs::read_to_string(&path) else {
+        return false;
+    };
+    let _ = std::fs::remove_file(&path);
+    let mut lines = s.lines();
+    let (Some(prev), Some(set)) = (lines.next(), lines.next()) else {
+        return false;
+    };
+    // A default the operator changed by hand since the park wins, exactly as on the recovery path.
+    if default_render_id().as_deref() != Some(set) {
+        return false;
+    }
+    set_default_endpoint(prev).is_ok()
+}
+
 /// Make `id` the default playback device for the duration of the desktop-audio capture,
 /// remembering the operator's current default (in memory + the crash marker) the FIRST time so
 /// [`restore_default_playback`] can put it back. Nothing is remembered when `id` already is the

crates/punktfunk-host/src/audio/windows/audio_probe.rs

@@ -42,8 +42,10 @@ use windows::Win32::System::Registry::{
 };
 
 /// Marker value in a probe devnode's `Device Parameters` key — how `cleanup` finds what this
-/// devtest minted (and nothing else).
-const PROBE_MARKER: &str = "PunktfunkAudioProbe";
+/// devtest minted (and nothing else). pub(crate): the uninstall sweep
+/// ([`devnode_cleanup`](super::devnode_cleanup)) sweeps this family too, so a devtest run on an
+/// operator's box cannot outlive the product.
+pub(crate) const PROBE_MARKER: &str = "PunktfunkAudioProbe";
 /// DeviceDesc for probe devnodes (visible in Device Manager until the INF install renames it).
 const PROBE_DESC: &str = "Punktfunk Audio Probe";
 /// How long to wait for audiosrv to register a minted endpoint.

crates/punktfunk-host/src/audio/windows/devnode_cleanup.rs

@@ -0,0 +1,217 @@
+//! Uninstall-time removal of every audio device punktfunk minted on this box — the
+//! `punktfunk-host driver uninstall --audio` leg the installer's Inno `[UninstallRun]` calls.
+//!
+//! The field report this exists for: uninstalling punktfunk left "Punktfunk Speakers",
+//! "Punktfunk Microphone" and the per-pad "Wireless Controller" endpoints sitting in Windows'
+//! Sound settings forever. They are not files and no uninstaller deletes them by walking a
+//! payload list — they are DEVNODES this host created at runtime, and they persist exactly
+//! because they are designed to ([`minted`](super::minted) and
+//! [`pad_endpoint`](super::pad_endpoint) both re-resolve their devnodes across host restarts
+//! rather than re-minting them). Persistent across restarts must not mean permanent.
+//!
+//! What gets swept: every MEDIA-class devnode carrying one of the three durable owner markers
+//! this product writes into `Device Parameters`, whatever minted it —
+//!
+//! * [`pad_endpoint::PAD_INDEX_VALUE`](super::pad_endpoint::PAD_INDEX_VALUE) — the per-pad
+//!   DualSense speaker endpoints,
+//! * [`minted::ROLE_MARKER`](super::minted::ROLE_MARKER) — the Speakers/Microphone substrate,
+//! * [`audio_probe::PROBE_MARKER`](super::audio_probe::PROBE_MARKER) — devtest leftovers, so a
+//!   probe run on an operator's box cannot outlive the product either.
+//!
+//! Marker-matched, never name-matched: our devnodes are instances of VALVE's streaming-audio
+//! drivers and are name-identical to Steam's own (the same reason the wiring plan works by
+//! recorded id). Steam's devnodes, its driver packages, and a VB-CABLE from the era when we
+//! bundled one all carry no marker and are therefore untouchable here — uninstalling punktfunk
+//! removes what punktfunk created, and nothing else.
+
+// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it.
+#![deny(clippy::undocumented_unsafe_blocks)]
+
+use super::{audio_control, audio_probe, minted, pad_endpoint as pe};
+use anyhow::Result;
+use windows::Win32::Devices::DeviceAndDriverInstallation::SetupDiEnumDeviceInfo;
+
+/// The `Device Parameters` REG_DWORD each punktfunk-minted devnode family stamps on itself. The
+/// VALUE is what differs per family; presence of the NAME is "this one is ours", which is all a
+/// sweep needs.
+const OWNER_MARKERS: [&str; 3] = [
+    pe::PAD_INDEX_VALUE,
+    minted::ROLE_MARKER,
+    audio_probe::PROBE_MARKER,
+];
+
+/// What one sweep removed. `endpoint_records` is counted separately from `devnodes` because the
+/// registry half is best-effort by design — see [`delete_endpoint_record`].
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub(crate) struct Removed {
+    pub devnodes: usize,
+    pub devnode_failures: usize,
+    pub endpoint_records: usize,
+}
+
+/// Restore the default playback device if we left it parked, then remove every audio devnode
+/// this product minted, newest registry record and all.
+///
+/// Best-effort throughout, like the rest of the (un)install path: a devnode that refuses to go
+/// is counted and reported, never fatal — a non-zero exit here would abort the whole uninstaller
+/// over a virtual speaker.
+pub(crate) fn purge() -> Result<Removed> {
+    // FIRST, before the sweep deletes the endpoint the default may still point at. A host that
+    // died mid-stream leaves the box's default playback parked on our loopback sink; Windows
+    // would re-pick on its own once the device vanishes, but by its own ranking rather than by
+    // what the operator had. Putting it back is the difference between "the box works again"
+    // and "the box works again, on the device it started with".
+    if audio_control::unpark_default_for_uninstall() {
+        println!("restored the default playback device this host had parked");
+    }
+
+    let mut out = Removed::default();
+    for inst in owned_devnodes()? {
+        // Resolve the endpoint records BEFORE the devnode goes. An endpoint's MMDevices key is
+        // tied to us only through its `{1}.<instance id>` devnode link — once the devnode is
+        // removed, nothing left in the store says the record was ever ours, and a sweep that
+        // guessed by NAME is exactly the mistake this module refuses to make.
+        let records: Vec<(&str, String)> = [
+            (pe::MMDEV_RENDER_PATH, pe::find_endpoint_for_devnode(&inst)),
+            (
+                pe::MMDEV_CAPTURE_PATH,
+                pe::find_capture_endpoint_for_devnode(&inst),
+            ),
+        ]
+        .into_iter()
+        .filter_map(|(path, found)| Some((path, found.ok().flatten()?)))
+        .collect();
+
+        if !remove_devnode(&inst) {
+            out.devnode_failures += 1;
+            // The device is still there, so its record still belongs to a live endpoint.
+            continue;
+        }
+        out.devnodes += 1;
+        for (path, endpoint) in records {
+            if delete_endpoint_record(path, &endpoint) {
+                out.endpoint_records += 1;
+            }
+        }
+    }
+    Ok(out)
+}
+
+/// Every MEDIA-class devnode carrying one of [`OWNER_MARKERS`]. Enumerated WITHOUT `DIGCF_PRESENT`
+/// (that is what [`pe::media_class_devs`] gives us), so a phantom left by a crashed host is swept
+/// too — the same "ghost in Device Manager forever" complaint the pad and vdisplay legs fixed.
+fn owned_devnodes() -> Result<Vec<String>> {
+    let set = pe::media_class_devs()?;
+    let mut out = Vec::new();
+    for i in 0.. {
+        let mut did = pe::devinfo_data();
+        // SAFETY: live set; `did` is a live out-param with cbSize set.
+        if unsafe { SetupDiEnumDeviceInfo(set.0, i, &mut did) }.is_err() {
+            break; // ERROR_NO_MORE_ITEMS
+        }
+        let Some(inst) = pe::instance_id(&set, &did) else {
+            continue;
+        };
+        if !is_removable_instance(&inst) {
+            continue;
+        }
+        if OWNER_MARKERS
+            .iter()
+            .any(|m| pe::read_devparam_dword(&set, &did, m).is_some())
+        {
+            out.push(inst);
+        }
+    }
+    Ok(out)
+}
+
+/// A devnode this sweep is allowed to remove: ROOT-enumerated, i.e. software-created.
+///
+/// Every devnode we mint comes from `SetupDiCreateDeviceInfoW(… DICD_GENERATE_ID)` on the MEDIA
+/// class, which always yields `ROOT\MEDIA\NNNN`. Nothing else can be ours — so if a marker name
+/// we own ever collides with a value some vendor writes under a REAL sound card's `Device
+/// Parameters`, this guard is what stops an uninstall from taking the user's hardware with it.
+fn is_removable_instance(instance_id: &str) -> bool {
+    instance_id.to_ascii_uppercase().starts_with("ROOT\\")
+}
+
+/// `pnputil /remove-device` — the same teardown `audio-probe cleanup` and the driver legs use.
+/// Called by absolute path: an uninstaller must not depend on the invoking shell's `%PATH%`.
+fn remove_devnode(instance_id: &str) -> bool {
+    let windir = std::env::var("WINDIR").unwrap_or_else(|_| r"C:\Windows".into());
+    match std::process::Command::new(format!(r"{windir}\System32\pnputil.exe"))
+        .args(["/remove-device", instance_id])
+        .output()
+    {
+        Ok(o) if o.status.success() => {
+            println!("removed audio devnode {instance_id}");
+            true
+        }
+        Ok(o) => {
+            eprintln!(
+                "warning: pnputil could not remove {instance_id} (status {:?}): {}",
+                o.status.code(),
+                String::from_utf8_lossy(&o.stderr).trim()
+            );
+            false
+        }
+        Err(e) => {
+            eprintln!("warning: could not run pnputil for {instance_id}: {e}");
+            false
+        }
+    }
+}
+
+/// Delete one endpoint's MMDevices record — the `{guid}` subkey holding its name, its stamped
+/// formats and its per-endpoint volume/settings.
+///
+/// BEST-EFFORT ON PURPOSE, and quiet when it fails. These keys are owned by SYSTEM and grant
+/// Administrators read only (the same ACL that forces the stamping path through
+/// `grant_system_full_control`), while the uninstaller runs elevated but as a USER — so on a
+/// stock box this is denied and the record stays. What stays is inert: with the devnode gone the
+/// endpoint is NOTPRESENT, which Sound settings surface only behind "Show Disconnected Devices",
+/// and nothing re-animates it without a devnode to link to. Buying that last cosmetic scrap would
+/// mean an uninstaller seizing ownership of SYSTEM-owned registry keys — a worse thing to ship
+/// than the leftover. The DEVICE, which is what the field report was about, is gone either way.
+fn delete_endpoint_record(reg_path: &str, endpoint_id: &str) -> bool {
+    use winreg::enums::{HKEY_LOCAL_MACHINE, KEY_ALL_ACCESS};
+    use winreg::RegKey;
+
+    let Ok(guid) = pe::endpoint_guid_part(endpoint_id) else {
+        return false;
+    };
+    let Ok(store) =
+        RegKey::predef(HKEY_LOCAL_MACHINE).open_subkey_with_flags(reg_path, KEY_ALL_ACCESS)
+    else {
+        return false;
+    };
+    store.delete_subkey_all(guid).is_ok()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn only_root_enumerated_devnodes_are_ours() {
+        assert!(is_removable_instance(r"ROOT\MEDIA\0003"));
+        // PnP casing is not guaranteed.
+        assert!(is_removable_instance(r"root\media\0004"));
+        // A real sound card, however it got a marker-shaped value written under it.
+        assert!(!is_removable_instance(
+            r"HDAUDIO\FUNC_01&VEN_10EC&DEV_0900\4&1c4a4e5&0&0001"
+        ));
+        assert!(!is_removable_instance(r"USB\VID_046D&PID_0A38\ABCDEF"));
+        // Not a prefix match on the string "ROOT" appearing anywhere.
+        assert!(!is_removable_instance(r"SWD\ROOT\MEDIA\0003"));
+    }
+
+    #[test]
+    fn every_minted_family_is_swept() {
+        // The sweep is only as complete as this list — a new minted-devnode family that forgets
+        // to register here would ship the same leak again.
+        assert!(OWNER_MARKERS.contains(&"PunktfunkPadIndex"));
+        assert!(OWNER_MARKERS.contains(&"PunktfunkAudioRole"));
+        assert!(OWNER_MARKERS.contains(&"PunktfunkAudioProbe"));
+    }
+}

crates/punktfunk-host/src/audio/windows/minted.rs

@@ -32,8 +32,9 @@ use std::sync::{Arc, Mutex, OnceLock};
 use std::thread;
 use std::time::{Duration, Instant};
 
-/// Durable role marker in a minted devnode's `Device Parameters` key.
-const ROLE_MARKER: &str = "PunktfunkAudioRole";
+/// Durable role marker in a minted devnode's `Device Parameters` key. pub(crate): the uninstall
+/// sweep ([`devnode_cleanup`](super::devnode_cleanup)) matches devnodes on it.
+pub(crate) const ROLE_MARKER: &str = "PunktfunkAudioRole";
 /// How long to wait for audiosrv to register a freshly minted endpoint.
 const ENDPOINT_WAIT: Duration = Duration::from_secs(15);
 /// Minimum spacing between provisioning retries once the startup attempt failed

crates/punktfunk-host/src/audio/windows/pad_endpoint.rs

@@ -87,13 +87,15 @@ const DEVNODE_DESC: &str = "Punktfunk Pad Audio";
 /// The multi-instancing Steam Remote Play render driver we ride on.
 const SSS_HWID: &str = "ROOT\\SteamStreamingSpeakers";
 /// Registry value under the devnode's `Device Parameters` key persisting which pad slot the
-/// devnode serves (REG_DWORD).
-const PAD_INDEX_VALUE: &str = "PunktfunkPadIndex";
+/// devnode serves (REG_DWORD). pub(crate): the uninstall sweep
+/// ([`devnode_cleanup`](super::devnode_cleanup)) matches devnodes on it.
+pub(crate) const PAD_INDEX_VALUE: &str = "PunktfunkPadIndex";
 /// The endpoint store for render endpoints (each subkey = one endpoint GUID).
-const MMDEV_RENDER_PATH: &str = r"SOFTWARE\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Render";
+pub(crate) const MMDEV_RENDER_PATH: &str =
+    r"SOFTWARE\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Render";
 /// The capture-direction sibling of [`MMDEV_RENDER_PATH`] — where a paired device's microphone
 /// half registers (the `audio-probe` devtest's S3 lookup).
-const MMDEV_CAPTURE_PATH: &str =
+pub(crate) const MMDEV_CAPTURE_PATH: &str =
     r"SOFTWARE\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Capture";
 /// WASAPI endpoint-id prefix for render endpoints (`{0.0.0.00000000}.{guid}`).
 const ENDPOINT_ID_PREFIX: &str = "{0.0.0.00000000}.";
@@ -355,8 +357,9 @@ fn reg_registry_value(v: &StampValue) -> winreg::RegValue<'static> {
 }
 
 /// The per-endpoint GUID portion of a WASAPI endpoint id (`{0.0.0.00000000}.{guid}` →
-/// `{guid}`) — the endpoint's MMDevices registry key name.
-fn endpoint_guid_part(endpoint_id: &str) -> Result<&str> {
+/// `{guid}`) — the endpoint's MMDevices registry key name. pub(crate): the uninstall sweep
+/// deletes those keys by name.
+pub(crate) fn endpoint_guid_part(endpoint_id: &str) -> Result<&str> {
     endpoint_id
         .rfind('{')
         .map(|i| &endpoint_id[i..])

crates/punktfunk-host/src/windows/install.rs

@@ -46,14 +46,14 @@ fn run_capture(cmd: &str, args: &[&str]) -> String {
         .unwrap_or_default()
 }
 
-// ── `driver install [--gamepad] --dir <stage>` / `driver uninstall [--gamepad]` ────────────────
+// ── `driver install [--gamepad] --dir <stage>` / `driver uninstall [--gamepad|--audio]` ────────
 pub fn driver_main(args: &[String]) -> Result<()> {
     match args.first().map(String::as_str) {
         Some("install") => driver_install(&args[1..]),
         Some("uninstall") => driver_uninstall(&args[1..]),
         _ => bail!(
             "usage: punktfunk-host driver install --dir <stage> [--gamepad]\n\
-             \x20      punktfunk-host driver uninstall [--gamepad]"
+             \x20      punktfunk-host driver uninstall [--gamepad|--audio]"
         ),
     }
 }
@@ -409,16 +409,25 @@ fn remove_pad_devnodes() {
     }
 }
 
-// ── `driver uninstall [--gamepad]` ──────────────────────────────────────────────────────────────
+// ── `driver uninstall [--gamepad|--audio]` ──────────────────────────────────────────────────────
 // The uninstaller's cleanup counterpart (Inno [UninstallRun]) — the field report was that our
-// virtual-device drivers survived an uninstall. Removes the pf-vdisplay device node(s) + driver
-// package, or (--gamepad) the pf-gamepad/pf-xusb driver packages (their devnodes are per-session
-// SwDeviceCreate'd and are already gone once the service stopped). Locale-safe by construction: we
-// never parse pnputil's localized LABELS — devices are matched on the un-localized VALUE side
-// (instance IDs / device IDs), and driver packages are found by scanning %WINDIR%\INF\oem*.inf
-// CONTENT for our driver names, then passed to pnputil by file name.
+// virtual devices survived an uninstall. Removes the pf-vdisplay device node(s) + driver package,
+// or (--gamepad) the pf-gamepad/pf-xusb driver packages (their devnodes are per-session
+// SwDeviceCreate'd and are already gone once the service stopped), or (--audio) the audio devnodes
+// the HOST mints at runtime — the same complaint one layer up, since those are created by the
+// running host rather than by any driver payload the installer laid down. Locale-safe by
+// construction: we never parse pnputil's localized LABELS — devices are matched on the
+// un-localized VALUE side (instance IDs / device IDs / registry markers), and driver packages are
+// found by scanning %WINDIR%\INF\oem*.inf CONTENT for our driver names, then passed to pnputil by
+// file name.
 
 fn driver_uninstall(args: &[String]) -> Result<()> {
+    // The audio leg touches no driver package and no certificate — it removes devnodes the host
+    // minted on Valve's drivers — so it returns before the cert purge below rather than making
+    // that purge run a third time per uninstall.
+    if flag_present(args, "--audio") {
+        return uninstall_audio_devices();
+    }
     let gamepad = flag_present(args, "--gamepad");
     let (what, res) = if gamepad {
         ("gamepad", uninstall_gamepad())
@@ -437,6 +446,38 @@ fn driver_uninstall(args: &[String]) -> Result<()> {
     Ok(())
 }
 
+/// Remove the "Punktfunk Speakers"/"Punktfunk Microphone" endpoints and the per-pad DualSense
+/// speaker endpoints the running host minted — the audio half of the surviving-virtual-device
+/// complaint. Must run AFTER `service uninstall`: a live host re-mints them on its next wiring
+/// pass, which would make this sweep look like it did nothing.
+///
+/// Never removes Steam's streaming-audio DRIVERS. Ours are extra devnodes riding on drivers that
+/// belong to Steam and that the user's own Remote Play still needs; the sweep is marker-matched
+/// (see `audio::devnode_cleanup`) precisely so it can tell the two apart.
+fn uninstall_audio_devices() -> Result<()> {
+    match crate::audio::devnode_cleanup::purge() {
+        Ok(r) if r.devnodes == 0 && r.devnode_failures == 0 => {
+            println!("no punktfunk audio devices to remove")
+        }
+        Ok(r) => {
+            println!(
+                "removed {} punktfunk audio device(s), {} endpoint record(s)",
+                r.devnodes, r.endpoint_records
+            );
+            if r.devnode_failures > 0 {
+                eprintln!(
+                    "warning: {} punktfunk audio device(s) could not be removed — they can be \
+                     deleted from Device Manager (View ▸ Show hidden devices)",
+                    r.devnode_failures
+                );
+            }
+        }
+        // Best-effort like every other leg: an enumeration that fails must not fail the uninstall.
+        Err(e) => eprintln!("warning: audio device cleanup: {e:#}"),
+    }
+    Ok(())
+}
+
 fn uninstall_pf_vdisplay() -> Result<()> {
     // 1. Remove the ROOT device node(s) the installer created via nefconc (leaving them would keep
     //    a ghost "punktfunk virtual display" in Device Manager forever — the exact complaint).

packaging/windows/punktfunk-host.iss

@@ -340,10 +340,17 @@ Filename: "{app}\punktfunk-host.exe"; Parameters: "service uninstall"; Flags: ru
 ; install laid down); `driver uninstall` is best-effort and no-ops when nothing is installed.
 ; A VB-CABLE from an OLDER punktfunk install (bundled until the audio-substrate change) is
 ; deliberately NOT removed: it is a third-party shared component the user may use elsewhere.
-; The host's own minted audio devnodes ("Punktfunk Speakers/Microphone") are likewise left in
-; place - they are plain instances of Steam's streaming drivers, inert without the host.
 Filename: "{app}\punktfunk-host.exe"; Parameters: "driver uninstall"; Flags: runhidden waituntilterminated; RunOnceId: "PunktfunkVdisplayDriverUninstall"
 Filename: "{app}\punktfunk-host.exe"; Parameters: "driver uninstall --gamepad"; Flags: runhidden waituntilterminated; RunOnceId: "PunktfunkGamepadDriverUninstall"
+; ...and the audio devices the RUNNING HOST mints ("Punktfunk Speakers", "Punktfunk Microphone",
+; the per-pad "Wireless Controller" endpoints). These have no installer payload behind them - the
+; host creates them at runtime and re-resolves them across restarts by design - so nothing else in
+; this uninstall would ever touch them, and the field report was that they sat in Sound settings
+; forever after an uninstall. Marker-matched, so Steam's own streaming-audio devices and drivers
+; (which our instances ride on, and which Remote Play still needs) are left alone. Runs after the
+; two driver legs, and well after `service uninstall`: a live host re-mints them on its next
+; wiring pass.
+Filename: "{app}\punktfunk-host.exe"; Parameters: "driver uninstall --audio"; Flags: runhidden waituntilterminated; RunOnceId: "PunktfunkAudioDeviceUninstall"
 #ifdef WithWeb
 ; Remove the console's firewall rule + any LEGACY PunktfunkWeb task and stray listener (the
 ; service-supervised console itself died with `service uninstall` above, via its kill-on-close job;