fix(web-server) [BRNS-DESK-021]: prune ws subscribers on close instead of writing to closed sockets

[sebastian-ssvlabs]

Summary

• tokio_tungstenite logged Sending after closing is not allowed ~99x per session at DEBUG during ordinary navigation and reloads. Send failure is now the deregistration signal: the broadcaster hands each connection a Subscriber registration, forwarders skip the fan-out for a pruned subscriber, and a failed send prunes it and exits.
• Found a second, larger cause while reading it: the per-connection tasks outlived the connection. tokio::select! only drops the losing JoinHandles, and dropping a JoinHandle detaches the task rather than aborting it — so a_forward, b_forward, cmd_loop, and heartbeat_task kept their broadcast receivers and their Arc clone of the split sink alive for the rest of the process. Teardown now prunes, then aborts all four.
• The 300s heartbeat was the worst of these: a detached heartbeat never returns, so it pinged a closed socket every 5 minutes forever, once per connection ever opened. It now returns on ping failure.
• Deregistration is logged once per connection ([broadcaster] subscriber gone, pruned (N live)), never per failed send.
• Closes BRNS-DESK-021 (P3, effort S, desktop / web-server).

The lifecycle race

The race cannot be eliminated — a client close is asynchronous to a broadcast already fanning out — so it is handled once instead of being logged per attempt:

1. Subscriber is registered at connection setup, shared by the forwarder tasks, and deregisters on prune() or on Drop. Both paths decrement exactly once, so an entry cannot leak whichever way the connection dies.
2. Both forwarders check is_live() before touching a message, which also drops the wasted serde_json serialization for a client that is already gone.
3. Teardown calls prune() before the sink drops, so a forwarder mid-fan-out sees the connection as gone; then it aborts the tasks, which drops the receivers and the sink.

In the common case (clean navigation) cmd_loop observes the close frame first, so teardown prunes and aborts while the forwarders are still parked on recv() — no send to a closed socket happens at all. The prune()-on-send-error path is the belt for the genuinely racing case.

Severity, honestly: no functional failure. Nothing was dropped or duplicated, and the log line is DEBUG-only. What this fixes is debug noise that would mask a real WS problem, wasted serialization per closed client, and the detached-task/receiver leak behind it.

Verification

Ran locally:

• cargo fmt --manifest-path src-tauri/Cargo.toml (applied) and --check (clean). The pre-commit hook's Rust formatting check also passed.
• Read the whole web_server module to confirm the mechanism, including that the subscriber "set" is a pair of tokio::sync::broadcast channels rather than an explicit registry — the ticket's assumed data structure did not exist, hence the new Subscriber seam.

Only CI can prove:

• cargo test was NOT run locally — no local Rust build env in this worktree (no cargo target dir), so the two new tests in broadcaster.rs (subscriber_count_returns_to_baseline_after_churn, explicit_prune_is_idempotent_with_drop) are unrun. They are written to the existing #[cfg(test)] mod tests / plain #[test] idiom already used in web_server/mod.rs and need no async runtime. Compilation itself is therefore also CI-verified only.
• cargo clippy and cargo build were not run.
• The acceptance check — 20 reloads of the remote-access UI at RUST_LOG=debug producing zero Sending after closing is not allowed lines, and events still reaching live clients — has not been performed; it needs a running app.

[nir-ssvlabs]

The exactly-once claim holds where it matters: prune() gates its decrement on live.swap(false, AcqRel) returning the previous value, so only the first caller ever decrements and Drop calling prune() cannot double-count. That is what keeps the AtomicUsize from underflowing, which on a usize would have been the real bug rather than a miscount.

The second fix is the larger one and it is wired correctly: subscriber.prune() runs at ws.rs:531, before the four .abort() calls at 535-538, so a forwarder mid-fan-out sees the connection gone rather than racing the sink drop — and the heartbeat now returns on ping failure instead of outliving the socket. Detaching on JoinHandle drop is an easy thing to read past; catching it while reading for something else is the good kind of accident.

• 🔵 The acceptance criterion — 20 reloads at RUST_LOG=debug with zero Sending after closing is not allowed — is still unperformed, and the two new tests do not stand in for it: they pin the counter's behaviour under churn and double-prune, not the absence of the log line. Everything structural says it should hold; nothing yet says it does. Worth one manual pass before this is called closed.

Checked: the prune/Drop exactly-once invariant and its memory ordering, teardown order against the aborts, the heartbeat's failure return, and that Rust is green on macOS, Ubuntu and Windows — which covers the compile and the two tests you could not run locally. Not read: the replay-buffer path below the teardown, untouched here.

Merge: ✅ into main.

[Chris-ssvlabs]

🟡 cmd_loop.abort() cancels an in-flight dispatch_command at an arbitrary await point when another task ends the select! first (e.g. a_forward hits the dead socket while a command is still executing); stop_run_impl has already removed the session handle from state.sessions but its update_status(RunStatus::Stopped) never runs, leaving the run stuck in running state — pre-PR the detached task completed the command. Abort only a_forward/b_forward/heartbeat_task and await cmd_loop (it exits when ws_rx yields None after the socket drops), or run side-effectful dispatches in a spawned task so teardown cannot cancel them mid-write.