audio/ownership: state the playlist policy, and gate main's ordering properly

Round 10 review, findings 2 and 5.

Finding 2 — R10-2's rationale for tagging local playlist audio was factually
wrong. It claimed a local track is "already being broadcast to peers on the
same keypress", but shared listening is opt-in: music_broadcast defaults to
false, play_music_index starts local playback unconditionally, and
broadcast_track returns immediately when can_broadcast_music is false. So a
default-config playlist is not already broadcast.

The tag stays, now as an explicit policy with the real reason: the carriers
reach rodio through PIPEWIRE_ALSA, which is process-wide, and clip_player
and music_player are two ClipPlayer instances in one process — no value of
that variable can tag one and not the other. Exempting the playlist means
giving it a separately taggable stream, which is a large change for a case
with a one-step workaround (play it in any other app). Tagging is not
optional for received clips and peer music, which are the far end's own
audio.

Finding 5 — the ordering test proved only "before run_gui", which a
thread::spawn inserted above the tag still satisfies while making the
set_var a data race. It now requires the tag to be the first executable
statement in main: attributes, `unsafe` and block punctuation are stripped,
and any residue fails. Mutation-verified against a spawn, an unrelated
statement, and the call deleted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/audio/ownership.rs

@@ -95,15 +95,41 @@ pub const PIPEWIRE_ALSA_ENV: &str = "PIPEWIRE_ALSA";
 /// Role of in-process audio played through `rodio`: received chat clips, peer
 /// music, and locally chosen playlist tracks (round 10, R10-2).
 ///
-/// **All three are tagged, deliberately**, including the local playlist. The
-/// case for exempting locally chosen music is that the user picked it and may
-/// want it shared; the case against is stronger. A local track played from the
-/// playlist is *already being broadcast to peers over the call* on the same
-/// keypress (`app::play_local_track` → `broadcast_track`), so sharing it a
-/// second time through the screen-share sends the far end two copies of the
-/// same audio, offset by the two paths' differing latency. That is not a lost
-/// feature, it is a defect. A user who wants music in the share can play it in
-/// any other application, which peerspeak never tags.
+/// # Policy: peerspeak's own playlist audio is never screen-shareable
+///
+/// **All three are tagged, and for the local playlist that is a deliberate
+/// policy rather than a consequence** (round 10 review, finding 2).
+///
+/// ⚠️ An earlier version of this comment justified it by claiming a local
+/// track is "already being broadcast to peers on the same keypress", so
+/// sharing it again would send the far end two copies. **That claim is false
+/// in the default configuration** and Codex was right to call it: shared
+/// listening is opt-in, `Config::music_broadcast` defaults to `false` (pinned
+/// by `config::tests::test_input_output_volume_fields`),
+/// `app::play_music_index` starts local playback unconditionally, and
+/// `app::broadcast_track` returns immediately when
+/// `can_broadcast_music` is false. With broadcasting off there is no second
+/// copy and no defect — only a track the user chose that they cannot put in
+/// the share.
+///
+/// The policy stands anyway, on cost rather than on that argument:
+///
+/// - The two carriers reach `rodio` through [`PIPEWIRE_ALSA_ENV`], which is
+///   **process-wide**. `clip_player` and `music_player` are two `ClipPlayer`
+///   instances in one process (`app::AppState`), so no value of that variable
+///   can tag one and not the other, and `set_var` cannot be flipped at
+///   runtime once threads exist.
+/// - Exempting the playlist therefore means giving it a separately taggable
+///   stream — a helper process, or rebuilding `ClipPlayer` on peerspeak's own
+///   PipeWire backend — for a case with a one-step workaround.
+/// - Tagging is **not** optional for the other two roles: received clips and
+///   peer music are the far end's own audio, and re-sharing them is the exact
+///   echo this feature exists to prevent.
+///
+/// So: a user who wants music inside the screen share plays it in any other
+/// application, which peerspeak never tags. When shared listening *is* on,
+/// the original two-copies argument does apply, and the policy is simply
+/// right for that case too.
 pub const CLIP_ROLE: &str = "clip";
 
 /// Role of the native call-playback stream — the node carrying the far end's
@@ -809,6 +835,16 @@ mod tests {
     /// A source-text assertion is crude. It is also the *only* check available
     /// short of driving the real GUI binary, and phase 1's miss was precisely
     /// a call site nobody verified existed.
+    ///
+    /// ⚠️ **The check is "first statement in `main`", not "before
+    /// `run_gui`"** (round 10 review, finding 5). Ordering against `run_gui`
+    /// proved far less than the safety obligation needs: inserting a
+    /// long-lived `std::thread::spawn` above the tag still satisfied it, while
+    /// making the `set_var` a data race — the exact thing the `unsafe` block's
+    /// SAFETY comment claims cannot happen. `set_var` is sound only while the
+    /// process is single-threaded, so the property to gate is that **nothing
+    /// executable precedes the call**, which is checkable and is what `main`
+    /// actually does today.
     #[test]
     fn main_tags_this_process_before_anything_starts() {
         const MAIN: &str = include_str!("../main.rs");
@@ -829,6 +865,46 @@ mod tests {
         let call = code
             .find("tag_this_process_alsa_audio()")
             .expect("main must call tag_this_process_alsa_audio (round 10, R10-2)");
+
+        let body = code
+            .find("fn main()")
+            .and_then(|start| code[start..].find('{').map(|open| start + open + 1))
+            .expect("main has a body");
+        // Back up over the module path the call is written with, so the
+        // statement's own `peerspeak::audio::ownership::` is not mistaken for
+        // code preceding it.
+        let call_start = code[..call]
+            .rfind(|c: char| !(c.is_alphanumeric() || c == '_' || c == ':'))
+            .map_or(0, |index| index + 1);
+        assert!(
+            call_start > body,
+            "the call must be inside main, not above it"
+        );
+
+        // Everything between `main`'s opening brace and the call, with the
+        // syntax that cannot *run* removed: attributes, the `unsafe` keyword,
+        // block punctuation, whitespace. Anything left is a statement that
+        // executes before the tag — which is what must not exist.
+        let mut before = code[body..call_start].to_string();
+        while let Some(start) = before.find("#[") {
+            let end = before[start..]
+                .find(']')
+                .map(|offset| start + offset + 1)
+                .expect("an attribute closes");
+            before.replace_range(start..end, "");
+        }
+        let residue: String = before
+            .replace("unsafe", "")
+            .chars()
+            .filter(|c| !c.is_whitespace() && !matches!(c, '{' | '}'))
+            .collect();
+        assert!(
+            residue.is_empty(),
+            "nothing may execute before the tag; found {residue:?} in main"
+        );
+
+        // And it must still be the real `main` of a program that starts the
+        // GUI, or the check above is asserting about the wrong function.
         let gui = code
             .find("run_gui")
             .expect("main runs the GUI; this test's ordering check assumes it");