The management port is movable, and the client no longer needs mDNS to find it #230

Merged

enricobuehler merged 4 commits from worktree-mgmt-port-single-source into main 2026-08-14 18:25:14 +00:00

Conversation 0 Commits 4 Files Changed 43

+936 -59

4 Commits

Author	SHA1	Message	Date
enricobuehler	35f940a3bb	fix(presenter): name the Connected callback type — widening it tripped clippy::type_complexity ... Adding the mgmt port beside the fingerprint pushed the inline `Option<Box<dyn FnMut([u8; 32], u16)>>` over `clippy::type_complexity`, which CI denies. A named `ConnectedFn` is what the lint asks for, and it gives the two positional arguments somewhere to be documented. My local gates ran `cargo check`, not `clippy -D warnings`, which is exactly why this reached CI instead of dying locally. Re-verified with `cargo clippy --all-targets -- -D warnings` across all eight crates: exit 0, pf-presenter confirmed genuinely linted, no type_complexity remaining.	2026-08-14 20:09:05 +02:00
enricobuehler	3cc8fa7ee0	feat(core): the host tells the client where its library is, so mDNS is no longer required ... ABI 19 -> 20. Wire protocol unchanged (still 2). Persisting the mgmt port (fe2bfeca) made a moved port survive mDNS going away, but mDNS was still the only SOURCE: a host that had never been seen on it — VPN-only, a routed subnet, or simply added by address on a network where multicast has never worked — had nothing to learn from and fell back to 47990. The `Welcome` now carries the port, so the client learns it over the connection it has already authenticated and discovery stops being involved at all. `Welcome.mgmt_port`, a trailing u16 after the cipher block, following the same additive discipline as the eight fields before it (compositor, gamepad, bitrate_kbps, bit_depth, color, chroma_format, audio_channels, codec): an older peer stops earlier and gets a documented default, in both directions, so WIRE_VERSION does not move. ⚠ THE TRAP, and why emitting the port forces the `cipher` placeholder: `cipher` is emitted only when non-default, so appending the port to an AES Welcome would land its LOW BYTE at offset 68 — exactly where every shipped 0.28.x client reads `cipher`, whose decode is deliberately fail-closed on an unknown id. 47991 is 0xBB57, so byte 68 would read 0x57 = 87, and EVERY current client would fail the handshake against a host that had merely moved its mgmt port. `encode` therefore writes an explicit cipher byte whenever a port rides along (the placeholder discipline `Hello::encode` already uses); a current client reads AES, a pre-cipher client stops before 68 regardless. The test pins the byte, both offsets (69 AES / 101 ChaCha), and that a host advertising no port still emits exactly 68 bytes — this field costs the common case nothing. Host: `mgmt::effective_port()` reads the same resolved bind `publish_endpoint` writes, so the wire, the endpoint file and the mDNS TXT cannot disagree — one lookup, not a fourth place to compute a port. `0` on the standalone punktfunk1-host binary, which has no management API: advertising 47990 from a host that is not serving it would be worse than saying nothing. Clients persist it on connect, feeding the store plumbing fe2bfeca already built: * Rust — `on_connected` grew the port alongside the fingerprint, plus `learn_mgmt_port_by_fp` (keyed by fingerprint alone, the identity a just-connected client is certain of). * Apple — `PunktfunkConnection.hostMgmtPort` + `updateMgmtPort` at the existing markConnected site. * Android — a new `nativeHostMgmtPort` JNI call, persisted where the session is constructed. Verified: Linux (pf-lxcheck2, amd64) `cargo check --all-targets` clean across punktfunk-core, pf-host-config, punktfunk-host, pf-client-core, pf-presenter, punktfunk-cli, punktfunk-client-linux and punktfunk-client-session, each confirmed genuinely compiled (counting `Compiling` as well as `Checking` — cargo prints the former for bin-only packages, which is what made an earlier gate look vacuous when it was not). punktfunk-core quic tests 76/76. Android: :kit+:app Kotlin, ParseRecordTest 12/12, and cargoNdkClippy clean for aarch64-linux-android. Apple: xcframework rebuilt at ABI 20, `swift build` complete. cargo fmt --all --check clean. NOT verified: the Windows client (192.168.1.133 unreachable).	2026-08-14 19:44:19 +02:00
enricobuehler	99eb679c07	feat(clients): a moved mgmt port now outlives the advert that announced it ... Moving the mgmt port off 47990 (the fix for sharing a box with a Sunshine fork, whose web UI owns that port) only ever worked for as long as mDNS did. The real port lived in the advert and nowhere else: every client read it live and threw it away, so on a VPN, a routed subnet, or any multicast-dead network the library silently fell back to a port nothing was listening on. `KnownHost` gains `mgmt_port: Option<u16>` + `effective_mgmt_port()` + `learn_mgmt_port()`, exactly the shape `mac` and `os` already use ("learned from the advert while online, persisted so it survives the host going to sleep") — except this one is load-bearing rather than cosmetic, so `upsert` states the preserve rule explicitly instead of relying on the does-not-mention-it accident that `clipboard_sync` survives by, and `upsert_trusted` carries it across a re-key. Wired through all four client families, each of which was wrong in its own way: * CLI / Windows / Linux reached for `DEFAULT_MGMT_PORT` at the call site — the constant is the FALLBACK, not the answer. Windows also needed the port on `Target`, which the library screen has instead of a `KnownHost`. * The session console read `advert.and_then(mgmt_port)` with NO saved fallback, two lines above an `os` that gets the three-rung treatment right. It now matches, and learns on every tick. * Linux's `mgmt_port_for` consulted live adverts only; it now falls back to the store. * Android never carried the port at all — its native discovery record stopped at 8 fields. Added `mgmt` as the 9th (the record's own documented "new fields append, never reorder" rule), then through `DiscoveredHost` -> `KnownHost` -> `LibraryScreen`. * Apple LOOKED done and was not: `StoredHost.mgmtPort` and `effectiveMgmtPort` have existed all along, but nothing anywhere wrote the field and the `mgmt` TXT was never parsed — so it was permanently nil and every Apple client resolved to 47990 regardless. That is worse than the honest omissions above, because it reads as finished. Now parsed, carried on `DiscoveredHost`, and written by `HostStore.updateMgmtPort` at the same site that learns MACs and the OS chain. Also `PUNKTFUNK_NATIVE_PORT` in host.env, finishing the pair with PUNKTFUNK_MGMT_BIND: `--native-port` was likewise CLI-only and died on a package upgrade. A bad value is a startup ERROR rather than the silent fall back to 9777 that `PUNKTFUNK_DATA_PORT` still does — the failure that reads as "I moved the port and the client still can't reach me". The client side of the native port already worked (`KnownHost.port` is persisted, `--connect HOST:PORT` names it). Adding the field broke three `KnownHost` literals in tests, which is the `Default` impl's stated purpose working ("adding a field here can't silently produce records that lack it"). All three now carry 47991 — deliberately NOT the default, so the assertions cannot pass vacuously against a hardcode. New coverage: forward-compat decode of a store predating the field, the resolver fallback, re-key carry-forward, and on Android the 9th-field parse plus 0/non-numeric/out-of-range all reading as unknown. What this does NOT fix: a host that moved its mgmt port and has NEVER been seen over mDNS. Nothing tells the client where to look, and the honest fix is for the host to announce it in-band — the `Welcome` message has an established "append a trailing field, older peer decodes to the default" pattern for exactly this, at the cost of a C ABI accessor and a bump. Left for a separate change. Verified: Linux (punktfunk-rust-ci/pf-lxcheck2, amd64) `cargo check --all-targets` clean for pf-host-config, punktfunk-host, pf-client-core, punktfunk-cli, punktfunk-client-linux and punktfunk-client-session — the last confirmed non-vacuous by planting a compile_error! and watching the gate fail (cargo prints "Compiling", not "Checking", for bin-only packages, so the usual marker grep lies about it). Android: :kit + :app compileDebugKotlin clean, ParseRecordTest 12/12 with both new cases named in the XML. Apple: xcframework built, `swift build` complete, SharedFoundationTests pass. cargo fmt --all --check clean. NOT verified: the Windows client (192.168.1.133 unreachable).	2026-08-14 19:44:19 +02:00
enricobuehler	bb78117504	feat(host): moving the management port off 47990 now survives, and the console follows ... 47990 is the management API's port and also Sunshine's (and Apollo's, and Vibeshine's) web UI port. With the GameStream planes off it is the ONLY port the two still share, so moving it is the whole of what "run both on one box" needs — except moving it was barely possible: * `--mgmt-bind` was the sole route, and it lives in a unit file / service registration that a package upgrade rewrites. There was no `host.env` key, so the change did not survive. * The literal 47990 appeared in SIX places — mgmt::DEFAULT_PORT, the Windows service's console launch, scripts/punktfunk-web.service, the NixOS module, web/web-run.cmd, and the console's own default. Nothing downstream could learn a different port, so moving the listener silently left the console proxying to a port nothing was listening on. Now there is one source of truth. `PUNKTFUNK_MGMT_BIND` joins `host.env` (the `--gamestream` / PUNKTFUNK_GAMESTREAM shape: either source works, the CLI flag wins), and `serve` publishes the port it ACTUALLY bound to ~/.config/punktfunk/mgmt-endpoint, in the same KEY=VALUE form mgmt-token already uses so it is sourceable as a systemd EnvironmentFile and readable by the Windows service's existing read_env_file_value. Every consumer derives from that; the 47990 literals survive only as the fallback that keeps an OLD host working with a NEW console. The two unit files drop their hardcoded `Environment=PUNKTFUNK_MGMT_URL=` rather than layering a default beneath the file: whether Environment= or EnvironmentFile= wins is a directive-ordering question, and the hand-written unit and the Nix-generated one do not order the same way. No default, no precedence puzzle — the server's own built-in fallback covers a host that never wrote the file. Two robustness details worth naming, because both fail in the same direction: * mgmt-endpoint is written write-then-rename. A torn read would set PUNKTFUNK_MGMT_URL to EMPTY, which is worse than a missing file — a built-in default only rescues an *unset* variable. * mgmtUrl() now treats blank as unset, which `??` alone does not. The publish happens in parse_serve next to the token persistence, so both files appear together; the console's unit gates on mgmt-token, and its Restart=always picks up a lost race anyway. What this does NOT change: a lost 47990 bind is still fatal to the whole host (the bind sits in tokio::try_join! with the native plane), and running two Moonlight-compatible hosts at once is still unsupported — on Windows the exclusive display topology is a second, independent conflict. Both are documented rather than altered. Verified on Linux in punktfunk-rust-ci (amd64): cargo check --all-targets clean for punktfunk-host and pf-host-config with the "Checking punktfunk-host" marker confirmed present (a first run exited 0 having compiled nothing — the warm shared target dir judged it fresh), 40/40 mgmt tests pass including the new one pinning the published line against both parsers that consume it. Console: tsc --noEmit clean, bun test server/ 9/9. cargo fmt --all --check clean.	2026-08-14 19:44:19 +02:00