diff --git a/docs/screenshare-audio-exclusion-impl-plan.md b/docs/screenshare-audio-exclusion-impl-plan.md new file mode 100644 index 0000000..214e266 --- /dev/null +++ b/docs/screenshare-audio-exclusion-impl-plan.md @@ -0,0 +1,687 @@ +# Implementation plan: whole-desktop screen-share audio without self-echo + +**Status:** ๐ŸŸข **v4 โ€” three review rounds applied. Approved to start Phase 0a.** +**Date:** 2026-07-21 +**Design of record:** [`screenshare-audio-exclusion-plan.md`](screenshare-audio-exclusion-plan.md) v3.4 (`8768cd2`), converged round 7. +**Scope:** *ordering, gates and acceptance criteria only.* + +**Reference convention.** `v3.4 ยงN` = the design doc. `plan ยงN` = this document. The two +numbering schemes collide (both have a ยง11 and a ยง12) and an unqualified reference in v2 sent +Phase 6's most important gate to a section that does not exist. Every cross-reference below is +qualified. + +**Review history** +| Round | Findings | Outcome | +| --- | --- | --- | +| 1 | 7 P1 + 5 P2 | 12 accepted, 1 half-rejected โ†’ v2. `~/Documents/handoff-docs/Codex/peerspeak/review-2026-07-21-impl-plan-round1.md` | +| 2 | 3 P1 + 7 P2 + 1 P3 | all accepted โ†’ v3. `โ€ฆ/review-2026-07-21-impl-plan-round2.md` | +| 3 | verification pass: 4 of 7 edits landed, 3 partial; 2 P1 + 2 P2 + 1 P3 | all accepted โ†’ v4. Approved to start Phase 0a. `โ€ฆ/review-2026-07-21-impl-plan-round3.md` | + +Adjudication in plan ยง10. Two of my own claims were refuted by Codex with source evidence and +two of its claims were refuted or narrowed by mine; both are recorded there rather than +quietly dropped. + +--- + +## 0. What this plan is optimising for + +The design is converged; the risk has moved from "is it right?" to "will it be built in an +order where each mistake is caught while it is still cheap." Three properties drive every +ordering decision: + +1. **Nothing that can create an echo runs before the thing that decides eligibility has been + validated against a real graph.** The taint engine is the hull. It is built, unit-tested, + and floated empty (Phase 5, dry-run) before a single link is created. +2. **Every path to unsafe audio is closed structurally before the machinery that could take + it is written.** There are **two** such paths, not one โ€” the capture *source* and the + capture sink's *inputs*. Both are closed in Phase 0d. +3. **Every phase ends in a state that is shippable or trivially revertible**, and every gate + is one a broken implementation can *fail*. A gate that cannot fail is not a gate; where a + gate asserts only that bad things are absent, it must also assert that good things are + present, or "captures nothing at all" passes it. + +The corollary, stated plainly because it is the most likely way this goes wrong: **the +temptation will be to write the link manager early**, because it is the visible feature. +Fan-out is roughly 400 lines and demos beautifully with a hand-picked node. It is also the +component that, shipped ahead of a validated engine, produces exactly the bug this feature +exists to prevent โ€” in front of Joe. + +## 0.1 Cross-repo reality + +Two repos, no Cargo dependency; the contract is pixelpass's CLI plus its `--output json` +event stream (`peerspeak/src/screenshare/mod.rs:1-14`). The bulk of the work โ€” graph engine, +link manager, AEC state machine โ€” is **pixelpass**. peerspeak's share is tagging, argv, +capability gating, teardown ordering and the user-visible status surface. + +**Hard ship-order constraint.** peerspeak spawns whatever `pixelpass` resolves on `PATH`. If +peerspeak passes `--aec=โ€ฆ` to a pixelpass that predates the flag, clap rejects it and **the +share hard-fails** โ€” the documented A23 / audit-P2 skew failure that already governs +`--strict-audio` (`screenshare/mod.rs:249-262`). + +> **pixelpass ships the capability first (Phase 7), including the old-peerspeak/new-pixelpass +> golden test. peerspeak only passes the new flags to a binary that advertised support +> (Phase 8), which owns the new-peerspeak/old-pixelpass golden test. Never "flag present or +> absent" as the protocol โ€” v3.4 D5.** + +โš ๏ธ **Capability is resolved twice, against two independently-resolved binaries.** +`ListAudioApps` resolves pixelpass and probes it at `core/mod.rs:3375-3378`; `StartScreenShare` +resolves it *again* at `:3409-3419`. Between those moments `PATH` or the override can change. +Verified in source. Phase 8 must bind the capability result to the resolved path and re-probe +if it differs, failing closed. + +--- + +## 1. Phase map and dependency DAG + +| # | Phase | Repo | Mutates graph? | Exit gate | +| --- | --- | --- | --- | --- | +| 0a | `object.serial` u64 fix | pixelpass | no | boundary parse tests | +| 0b | Explicit teardown + drop ordering | peerspeak | no | **five independent mutations** (plan ยง2) | +| 0c | Graceful stop + connection-owned capture sink | both | sink ownership | two-host SIGKILL live gate **+ SIGINT-first gate** | +| 0d | **Typed capture plan + internal mode input** | pixelpass | no | mode matrix; neither unsafe source nor unsafe sink input constructible | +| 1 | peerspeak ownership tagging | peerspeak | no | tag on live nodes; literal pinned in plan ยง3 | +| 2 | Graph model + taint engine (pure) | pixelpass | **no PipeWire at all** | v3.4 ยง12 fixture matrix + degenerate-snapshot case | +| 3 | Registry observer + readiness epoch | pixelpass | read-only | six-part gate incl. **PID derivation** | +| 4 | AEC identity validation state machine | pixelpass | read-only | fake-clock transition matrix | +| 5 | **Dry-run audit mode** | pixelpass | read-only | ๐Ÿšฆ **MAJOR GATE** โ€” exact decision partitions (plan ยง5) | +| 6 | Link manager + status events, driven through the real host path | pixelpass | **yes โ€” first mutation** | link-manager matrix (plan ยง6.1) + live dynamic matrix | +| 7 | **Public mode selector** + capability advertisement | pixelpass | no | old-peerspeak/new-pixelpass golden test | +| 8 | peerspeak integration (mode flag, argv, picker, UI, status) | peerspeak | no | new/new argv golden (mode **and** `--aec`); new-peerspeak/old-pixelpass golden; causal status delivery | +| 9 | Rig upgrade + field matrix | both | yes | ๐Ÿšฆ **SHIP GATE** โ€” plan ยง7 | + +**Landing DAG** (development may be concurrent; *landing* order may not): + +``` +0a โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ–บ 2 โ”€โ”€โ–บ 3 โ”€โ”€โ–บ 4 โ”€โ”€โ–บ 5 โ”€โ”€โ–บ 6 โ”€โ”€โ–บ 7 โ”€โ”€โ–บ 8 โ”€โ”€โ–บ 9 + โ–ฒ +0b โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ค +0c โ”€โ”€โ–บ 0d โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ +1 โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ–บ 5 +``` + +- **0b strictly precedes 6.** v2/v3 drew 0b with no continuing edge. Phase 6 is the first phase + that creates objects whose lifetime is tied to pixelpass being alive, so the teardown-ordering + guarantee must exist before it: without it, the AEC can unload while a fanning-out pixelpass + still holds link proxies and a stale module index (v3.4 ยง7.1). +- **0a strictly precedes 2**: the taint engine's lifetime-awareness (v3.4 ยง6.1.3) is keyed on + `object.serial`; building the model against the current lossy `u32` + (`pixelpass/src/host/audio.rs:534-540`; `RouterState::sink_serial` at `:594-598`) means a + cross-cutting migration later. +- **0c strictly precedes 0d** (round-2): 0d's types must be built around the *final* + connection-owned bare sink, not today's `Routing`. Building the type boundary against the + legacy pactl sink means rebuilding it when 0c lands. +- **1 strictly precedes 5**: without tags the taint engine has no roots and the dry-run can + only exercise the forwarder half of the problem. + +--- + +## 2. Phase 0 โ€” prerequisites + +### 0a. `object.serial` u32 truncation โ€” pixelpass +Parse as `u64` throughout; audit the other `parse::` at `:369` (determine whether it is a +serial or a genuinely-32-bit value before changing it). Tests: value > `u32::MAX`, and the +boundary. + +### 0b. Explicit teardown + drop ordering โ€” peerspeak +v3.4 ยง7.2, decision D4. All **three** of v3.4's fixes: + +1. Replace the implicit-drop path at both channel-close sites. **Citation correction:** v3.4 + says `core/mod.rs:1516`; the actual `None => break` arms are at **`:1514`** and **`:1532`**. + Take the session and `shutdown().await` it. +2. Move `echo_cancel` (currently `:682`) to the last declared field, after `screenshare_host` + (`:685`), with a comment naming the invariant. +3. **Last-ditch drop wrapper**: `start_kill` + a bounded `try_wait` reap on the host before the + AEC guard unloads. This is the *only* protection on the panic/unwind path, and unwind is + reachable โ€” the core has numerous `unwrap()` sites and no `panic=abort` profile. + +โš ๏ธ **Mutation testing: five mutations, each independently breaking a named test.** v1 demanded +a mutation that targeted the wrong defense; v2 fixed that but bundled two defenses into one +combined mutant, which proves neither. Final form: + +| # | Mutation | Must break | +| --- | --- | --- | +| 1 | remove `shutdown().await` at `:1514` | close-arm-A teardown test | +| 2 | remove `shutdown().await` at `:1532` | close-arm-B teardown test | +| 3 | remove the explicit **wait** after host kill | explicit-ordering test (host kill+wait strictly precedes AEC unload) | +| 4 | reverse the field order | panic/unwind ordering test | +| 5 | remove the wrapper reap | panic/unwind ordering test (distinct assertion from #4) | + +Both channel-close arms get their own test; a single "closes the command channel" test can +exercise one arm and leave the other unsafe. + +### 0c. Graceful stop + connection-owned capture sink โ€” both +v3.4 ยง7.4. The **largest hidden cost in Phase 0**: moving the null sink off `pactl load-module` +(`pixelpass/src/host/audio.rs:69`, cleaned up only in `Routing::cleanup` at `:259-260`, which +SIGKILL skips) onto a connection-owned PipeWire object. + +- peerspeak: SIGINT (**not** SIGTERM โ€” pixelpass installs only `ctrl_c()`, + `pixelpass/src/common/signal.rs:6`), bounded wait, SIGKILL fallback, at `core/mod.rs:699` + and `:3480`. +- pixelpass: connection-owned sink; `--repair` (`src/repair.rs:15-63`) extended and proven safe + with a second live host. + +**Exit gate โ€” two halves. The round-2 finding was that v2 gated only the first.** + +*(i) Ownership,* a live two-host test โ€” connection-ownership is a runtime property no unit test +can establish: + +```bash +pactl list short sinks | rg 'pixelpass_capture_' +pw-dump | jq -r '.[] | select(.type=="PipeWire:Interface:Node") | .info.props as $p + | select(($p["node.name"] // "") | startswith("pixelpass_capture_")) + | [$p["object.serial"], $p["node.name"]] | @tsv' +kill -KILL +# re-run both: the killed host's object GONE, the second host's REMAINS +pixelpass --repair # the live host must be untouched +``` + +*(ii) Graceful stop,* which the above does not touch at all โ€” it exercises only external +SIGKILL, so Stop Share could remain `child.kill().await` (`core/mod.rs:3480`, still true today) +and every command above would pass: +- a fake-child signal-order test: SIGINT first, SIGKILL only after the bound expires; +- a live Stop Share run: SIGINT sent, child exits within the declared bound, **no fallback + kill on the normal path**. + +**O1 is closed: no demotion path.** v1 pre-authorised moving 0c after Phase 6 if it ballooned. +That is a waiver of settled decision v3.4 D6 hidden in a sequencing document, which is how a +converged design quietly decays. If 0c balloons, **stop and reopen D6 as design round 8.** + +### 0d. Typed capture plan + internal mode input โ€” pixelpass + +Today `setup_audio` returns `(Option, String)` (`pipeline.rs:123-142`) and the `String` +flows untyped into `build_args` (`:151-157`). There are **two** unsafe paths into the capture, +and v2 closed only the first: + +**Path 1 โ€” the source string.** `default_audio_monitor()` has exactly one call site, +`pipeline.rs:138`. That single line hands `pulsesrc` the real default monitor. + +**Path 2 โ€” the sink's inputs (round-2 P1, the defect v2 missed).** Even with a type-safe source, +`Routing::start` loads `module-loopback source=@DEFAULT_SINK@.monitor โ†’ pixelpass_capture_*` +whenever it runs outside strict-app mode (`audio.rs:80-92`), and it runs whenever +`PIXELPASS_AUDIO_VIA_NULL_SINK` is set (`pipeline.rs:124-125`). So `DesktopExcluding` could +correctly read *its own* sink's monitor while the legacy loopback has already filled that sink +with the whole-desktop mix โ€” **full echo, with no source switch anywhere.** v3.4 ยง3 states this +loopback must never load in the new mode; nothing structurally enforced it. + +Both are closed by construction: + +- `LegacyDesktop` โ€” the only variant that can produce `DefaultMonitor`. +- `PerApp { routing }` โ€” legacy `Routing`, unchanged. +- `DesktopExcluding { capture_sink }` โ€” owns a **bare** connection-owned sink type (0c) whose + API **cannot construct the legacy loopback at all**. Not "does not call it": the constructor + is not reachable from this variant. +- **Conflict policy, pinned here rather than discovered later:** the new mode combined with + `--app` or `PIXELPASS_AUDIO_VIA_NULL_SINK` **rejects at CLI parse time**. It must never fall + through to legacy `Routing`, and it must never silently ignore an input the user set. +- **Internal mode input lands here too** (round-2 P1): a non-advertised `HostOpts` field plus a + hidden trigger, so the variant is reachable through the real host/spawn path before Phase 6 + needs to measure through it. `HostOpts` has no mode field today and `setup_audio` selects + solely on `app` + the env override. + +**Why 0d is a prerequisite rather than a Phase 6 deliverable** (my divergence from Codex's +round-1 suggestion; it agreed in round 2): this is a pure non-mutating refactor of one function, +and landing it in Phase 6 means the link manager is written against the untyped API and then +refactored underneath itself, while the two most dangerous paths in the codebase stay unguarded +through four phases of active work around them. Guardrails go up before the scaffolding. + +**Exit gate:** +- mode matrix over every `(app, strict_audio, mode, env-override)` combination asserting the + resulting capture plan, including every conflict combination rejecting; +- type-level: `DesktopExcluding` can name neither the default monitor nor the legacy loopback; +- **graph assertion**: with the new mode active, no default-monitor link or module feeds the + capture sink; +- legacy behaviour byte-identical. + +A constructible-but-not-yet-public variant is acceptable for the interval between 0d and Phase +6 provided it is unit-tested and reachable by the hidden trigger. + +--- + +## 3. Phase 1 โ€” peerspeak ownership tagging (v3.4 ยง5.1) + +Zero behaviour change; it is what makes Phase 5 observable. + +- Native playback: prop on the stream dict, `src/audio/pipewire_impl.rs:374-388`. +- mpv/VLC spawn (`src/screenshare/mod.rs:768-775`), notification spawn (`src/notify.rs:265-272`): + `PULSE_PROP` + `PIPEWIRE_PROPS` on the `Command`. + +โš ๏ธ **The literal is a cross-repo wire contract and is pinned HERE, before Phase 1 starts** โ€” not +deferred with the v3.4 ยง11 product naming, which is a separate and genuinely user-facing question. + +``` +key: peerspeak.owned +value: 1 +``` + +**A per-repo literal test is not a contract test.** Two tests, one per repo, each maintained +beside its own implementation, get updated in lockstep with a rename and prove nothing. Required: + +1. The literal appears **once** per repo as a named constant, commented with a pointer to this + section and to the other repo's constant. +2. A **black-box cross-repo test**: peerspeak constructs the child `Command`, the test reads the + env it would set, and asserts it produces the exact property string pixelpass's engine + matches on โ€” driven from a single shared fixture string committed in both repos. +3. The real proof is the **Phase 5 dry-run**, which requires pixelpass to classify all three + live peerspeak playback paths as `NotEligible` *for the tag reason*. Emission alone proves + only that peerspeak talks, not that pixelpass listens. + +**Exit gate:** `pw-dump` shows the tag on a native call playback node, an mpv node and a +notification node on this box. (Consumption is gated in Phase 5.) + +**Non-goal:** the known grandchild-inheritance leak (v3.4 ยง5.1) stays accepted in v1. + +--- + +## 4. Phases 2โ€“4 โ€” the engine (pixelpass) + +### Phase 2 โ€” graph model + taint engine, pure +All of v3.4 ยง6.1โ€“ยง6.1.3, **with no PipeWire types in any signature**: + +``` +fn evaluate(snapshot: &GraphSnapshot, ctx: &ExclusionCtx, prior: &StickyState) + -> (Decisions, StickyState) +``` + +- `GraphSnapshot` = plain owned Node/Port/Link/Client structs keyed on `u64` serial, with the + recyclable id retained only as a lookup key, never as identity (v3.4 ยง6.1.3). +- `Decisions` carries `Eligibility::NotEligible { reason }` with **stable reason codes**, not + prose. That code is the Phase 5 dry-run output, the Phase 6 JSON status event, and the + eventual "why isn't this app shared" answer. Design it once, here. +- `StickyState` threaded explicitly, so stickiness is testable as a snapshot sequence. + +**Every row of the v3.4 ยง12 taint-engine table is a required deliverable**, plus one addition: +an **empty/degenerate snapshot must yield "nothing eligible", not "everything eligible"** โ€” the +fail-closed default asserted at the boundary. + +**Exit gate:** fixture matrix green; the engine has never been linked against libpipewire. + +### Phase 3 โ€” registry observer + readiness epoch, read-only +v3.4 ยง6.3 and ยง6.4. Replaces (not extends) the existing router, which watches Node and Metadata +adds, forwards raw removals, and binds no graph (`src/host/audio.rs:523-585`). + +- Node, Port, Link **and Client** globals; adds **and removes**. +- Link endpoint props from the global are the **optimisation**; the bind-`LinkInfoRef` fallback + is the correctness path. +- Readiness = `core.sync()`/`done` **plus** no outstanding required observations, fail-closed + timeout. Log which condition released the epoch. +- pipewire-pulse PID derivation (v3.4 ยง6.1.2): consistent `pipewire.sec.pid` across Pulse + clients, validated against `/proc//comm`. + +**Exit gate โ€” six parts.** A one-time `pw-dump` diff passes while Port observation is absent, +removals are ignored, the fallback is dead code, and readiness releases early: + +| gate | proves | +| --- | --- | +| adapter tests: add **and remove** of all four object types | removal handling exists | +| forced-absent Link endpoint props | the bind fallback is live, not decorative | +| unresolved-observer timeout test | readiness fails closed | +| readiness does not release with an observation outstanding | the epoch means something | +| **PID derivation matrix** (round-2): consistent valid PID ยท inconsistent PIDs ยท missing client property ยท `/proc` entry missing ยท `comm` mismatch ยท PID reuse โ€” **every failure makes owner-bridge key 4 unusable** | the pure engine can be correct on a wrong context; this is where the context is built | +| **live**: create and destroy a controlled node/link topology; diff Nodes, **Ports**, Links and Clients before/during/after | the adapter tracks a *changing* graph, not a static one | + +### Phase 4 โ€” AEC identity validation state machine, read-only +v3.4 ยง5.3 verbatim: `NotConfigured / Validating / Validated / Failed / Revoked`; +`--aec=off|pulse-module:` parsing (D5); bounded deadline; **no fan-out while `Validating`**; +revocation = loss of *all* nodes bearing the index, never one leg corking. Foreign +`echo-cancel-*` groups: warn and exclude (D3). + +**Moved ahead of the dry-run gate (round-1 P1).** v1 put this after the dry-run while the +dry-run checklist required AEC validation and revocation semantics โ€” a circular dependency that +made the major gate uncompletable as written. + +**Exit gate โ€” a fake-clock/event-sequence transition matrix**, because these are timing +semantics a live poke cannot cover: `Validating โ†’ Failed` on deadline expiry; `Validating โ†’ +Validated` on first matching node; partial-node disappearance โ‡’ **stays `Validated`**; all +nodes gone โ‡’ `Revoked`; `Revoked` stops fan-out and drops proxies; a retained stale index does +not alias onto a reloaded module (v3.4 ยง5.2 correction 3 โ€” indices *are* reused). Parsing: JSON +number and string forms, `> u32::MAX`, absent, malformed. + +Then wire the state machine's output into the dry-run so `Validating`/`Failed`/`Revoked` are +observable in Phase 5 before they gate anything real. + +--- + +## 5. Phase 5 โ€” dry-run audit mode ๐Ÿšฆ MAJOR GATE + +**Adds no capability. Its entire purpose is to be wrong loudly and safely.** + +A hidden trigger (`PIXELPASS_AUDIO_AUDIT=1`) running Phases 2โ€“4 against the live graph on every +graph event, emitting per `Stream/Output/Audio` node: serial, name, decision, stable reason +code, graph epoch. It creates **no links**. Output goes to **stderr or a defined JSON event** โ€” +never unstructured prose into `--output json`, which peerspeak parses +(`screenshare/mod.rs:92`). + +Why this is the gate: the C2/C3-class defects are graph-*reasoning* defects. A fixture proves +the code matches my model of PipeWire; only a live run proves my model matches PipeWire. A wrong +answer here costs a log line; the same wrong answer in Phase 6 costs an echo. + +### 5.1 Every row asserts an exact partition, not a spot check + +Round 2's sharpest structural point: checking only named targets constrains nothing about +everything else, so **each row must assert the complete candidate universe partitioned into +exact eligible and excluded sets, with reason codes on the excluded side.** That single +requirement is also the answer to O7 โ€” it is the over-exclusion gate, because an +exclude-everything implementation fails the eligible half of every row. + +| # | Scenario | Excluded (with reason code) | Eligible | +| --- | --- | --- | --- | +| 1 | `module-null-sink` + `module-loopback` forwarder (the v3.4 ยง6.1 measured shape) | output leg, reason = **owner bridge**, naming the key โ€” *not* a Link walk | same forwarder shape with **no** tainted input | +| 1b | *opportunistic, non-gating:* Sunshine's null-sink topology while it is routing desktop audio | its forwarder leg, if a re-emitting leg exists | โ€” | +| 2 | `gst-launch pulsesrc ! pulsesink` split clients, input **explicitly rooted on a tainted monitor** | output leg via key 4 | the same process reading an **untainted** source | +| 3 | **two** Pulse modules; **one** tainted input | the tainted module's output only | **the other module's output must be ELIGIBLE** โ€” this is what makes wrong pipewire-pulse-PID fusion observable | +| 4 | peerspeak native call playback | that node, reason = tag | โ€” | +| 5 | peerspeak-spawned **mpv** (watched share) | that node, reason = tag | mpv launched by hand | +| 6 | peerspeak **notification** sound | that node, reason = tag | โ€” | +| 7 | a **second** pixelpass host's capture sink, **plus a controlled forwarder reading that sink's monitor** | the forwarder's **named output serial** (cycle prevention, v3.4 ยง6.2) | โ€” | +| 8 | EasyEffects running | combined output leg | EasyEffects stopped โ‡’ ordinary streams | +| 9 | Firefox: music only / mic on untainted source / capturing a tainted monitor | the third only (v3.4 ยง6.1.1) | the first two | +| 10 | sticky taint: tainted input leg removed, output leg lives | still excluded | after full owner teardown + restart | +| 11 | recycled serial/index/link-group after teardown | โ€” | must **not** inherit taint | +| 12 | AEC loaded, then unloaded | four nodes; then `Revoked` | โ€” | +| 13 | `Audio/Duplex` device | over-taints, recorded as **known accepted** (v3.4 ยง6.1 caveat) | โ€” | + +Rows 3 and 7 were vacuous in v2: row 3 had no tainted module, so incorrect fusion of all +pipewire-pulse modules changed no emitted decision; row 7 observed a capture sink without naming +a downstream candidate, so recognising `pixelpass_capture_*` as a mere sink name would pass +without any transitive propagation. + +### 5.2 Also record, per O5 +Graph-event rate, recompute duration **distribution and maximum**, and whether events queue +behind recompute/logging. v3.4 ยง6.4's "full recompute is fine for v1" then rests on measured +headroom and epoch lag rather than on a node count. + +### 5.3 โš ๏ธ Do not build a gate on a transient topology +v1 leaned on v3.4 ยง6.1.0's "the hazard is LIVE on this machine right now." **Measured +2026-07-21 ~14:55 โ€” no longer true**, six hours after it was written: `Default Sink` is +`alsa_output.pci-0000_10_00.6.analog-stereo` (IDLE), all three `sink-sunshine-*` null sinks +SUSPENDED. Sunshine is still running (pid 4104) and still reads a monitor โ€” but the hardware +sink's, via active link `56 โ†’ 95`, not a null sink's. So Sunshine running is **not** sufficient +for the topology to be present; see plan ยง11. + +The **controlled fixture (row 1) is authoritative** โ€” deterministic and always available. But +the wild sample is not therefore unnecessary: a fixture I build tests my model against my own +assumptions, whereas Sunshine is an uncontrived third-party forwarder nobody designed for this +test. It stays as row 1b, **opportunistic and non-gating**, because it cannot be relied on to +be present. + +**Any surprise here goes back to the design doc as round 8. Phase 6 does not start until this +results file exists.** + +--- + +## 6. Phases 6โ€“8 โ€” mutation, capability, integration + +### Phase 6 โ€” link manager + status events, driven through the real host path +v3.4 ยง4.2 + ยง6.2 + ยง6.3 items 3โ€“4. Non-lingering links (rig gotcha: `object.linger=false` is +*ignored* by `pw-link --props` and `pw-cli create-link`; only `pw-link -m` yields one), proxies +retained for the life of the share, per-port link sets, "captured" only when **every** required +link is `ACTIVE`, same-epoch revalidation immediately before each creation, proxy drop on +ancestry becoming unsafe. + +Failure โ‡’ report the stream unsupported. **Never** fall back to the default monitor โ€” and after +0d that fallback is unconstructible in this mode, by either path. + +**Everything here is measured through the real selector โ†’ sink โ†’ link manager โ†’ `pulsesrc` +path**, using 0d's hidden trigger. A harness-only measurement would pass while the production +CLI still reaches only legacy branches. + +**Status events land here** (round-1/2: no phase owned them). pixelpass's event enum +(`src/common/output.rs:36-66`) has nothing for exclusion status, and capture-spawn failure +(`host/mod.rs:309-312`) replies to the viewer while emitting no event at all. Required as +**versioned wire-shaped events**, not stderr lines. Without them the safe failure mode is +unexplained silence after the first viewer connects โ€” and a sharer who cannot see why will +switch back to unsafe whole-desktop audio. + +There are **four** production causes, and each needs an **exact JSON golden plus a cause โ†’ +emission test** โ€” not a shared "an event is emitted" assertion, which passes while three of the +four remain unwired: + +| cause | event | trigger under test | +| --- | --- | --- | +| per-stream link failure | `stream_unsupported` | link-matrix row 8c | +| AEC validation deadline | `aec_failed` | Phase 4 `Validating โ†’ Failed` | +| AEC identity lost mid-share | `aec_revoked` | Phase 4 `Validated โ†’ Revoked` | +| foreign `echo-cancel-*` present (D3) | `foreign_aec_warning` | a second AEC module loaded | + +Phase 8 owns the other half of each: parse, traverse the **new mode's** notice channel, and +reach the intended UI state. The channel is currently created only for `audio_app` +(`core/mod.rs:3424`) and only the two `AppAudio` events are translated (`:3431-3434`). + +#### 6.1 Link-manager matrix (local anchor โ€” v3.4 ยง12 has only a one-line bullet) + +v2 pointed its most important gate at a "v3.4 ยง12 bookkeeping matrix" that does not exist. +Here it is. Each row is a deterministic test with an injected graph, not a live observation: + +| # | Case | Assertion | +| --- | --- | --- | +| 1 | graph mutated to tainted **between evaluation and `create_link`** | **zero unsafe `create_link` calls** โ€” not "eventually cleaned up" | +| 2 | per-port enumeration | exact set of attempted links and their states | +| 3 | partial activation (FL `ACTIVE`, FR not) | **not** reported captured | +| 4 | duplicate enumeration of the same node | idempotent; no second link set | +| 5 | ancestry becomes unsafe after `ACTIVE` | owned proxies dropped | +| 6a | `port.exclusive` port | refused, reason code emitted | +| 6b | encoded stream | refused, reason code emitted | +| 6c | passthrough (IEC958) stream | refused, reason code emitted | +| 7 | capture sink replaced | relink **succeeds** โ€” every required port back to `ACTIVE` and the node reported captured again; stale proxies dropped | +| 8a | AEC `Failed` (validation deadline) | plan stays `DesktopExcluding`; no capture | +| 8b | AEC `Revoked` mid-share | plan stays `DesktopExcluding`; fan-out stops | +| 8c | link creation error | plan stays `DesktopExcluding`; that stream reported unsupported | +| 8d | capture-sink creation failure | plan stays `DesktopExcluding`; mode fails, does not degrade | +| 8e | readiness-epoch timeout | plan stays `DesktopExcluding`; fail closed | +| 9 | an **eligible** late-arriving node | **positively captured** โ€” the over-exclusion counterpart to row 1 | + +Rows 6aโ€“6c were one combined fixture in v3: a single working refusal predicate would have +masked two missing ones. Row 7 required only "relink attempted", which a permanently-failing +attempt satisfies while v3.4 ยง4.2 requires a live owner to actually restore links after sink +recreation. Rows 8aโ€“8e replace an unenumerated "any failure path", under which testing one +handler passes while another silently swaps the plan to `LegacyDesktop`. + +Row 1 is the one v2 could not falsify: "clean โ†’ tainted mid-share โ‡’ links dropped" can pass by +observing eventual removal, while an unsafe link genuinely existed for a window. Row 8 is O8's +answer: 0d's enum prevents a `DesktopExcluding` value from *containing* `DefaultMonitor`, but +not a failure handler from replacing the whole plan with `LegacyDesktop`, so this needs a +release-mode integration test per failure transition. A `debug_assert!` is cheap and worth +adding, but it is not a gate. + +Plus the live dynamic matrix (SIGKILL removes owned links; node appearing after share start; +sink recreation) and the three-arm leak measurement re-run through the production path with +v3.4 ยง12's rig discipline (`media.class` filter first, never drop stderr, verify the link is +in-graph, `parec -d .monitor`). The deliberately-naive predicate used as that +measurement's positive control lives in a **test-only injected implementation**, never a +shippable runtime override. + +### Phase 7 โ€” public mode selector + capability advertisement (pixelpass ships first) + +โš ๏ธ **Round-3 P1: nothing in v3 ever promoted the hidden trigger to a public flag.** 0d added an +internal mode input; Phase 7 advertised capability and naming; Phase 8 added `--aec`, the picker +and status. No phase required the actual **mode selector** to exist publicly or to be passed. +The result would be a capability-gated picker entry that, when chosen, still spawns legacy +whole-desktop capture โ€” the feature appearing to ship while doing nothing. Reachable: peerspeak's +host argv has no mode parameter (`screenshare/mod.rs:152`) and pixelpass's `HostOpts` has no mode +field (`cli.rs:153`); v3.4 ยง11 requires a distinct mode selector. + +So Phase 7 lands **both**: +1. the public mode flag (naming per v3.4 ยง11), replacing the 0d hidden trigger as the production + entry point โ€” the hidden trigger may remain for testing; +2. D2's **versioned machine-readable** capability response or bitset. Must **not** overload + `app_audio_supported: bool` โ€” per-app-strict and desktop-excluding are independent + capabilities. `--help` substring probing survives only as the legacy fallback. + +**Old-peerspeak + new-pixelpass golden test lands here, before pixelpass ships**: behaviour +byte-identical, absent `--aec` still accepted. + +v3.4 ยง11 public naming is a **blocking user input at the start of this phase**. Internal typed +variant names (0d) do not block on it. + +### Phase 8 โ€” peerspeak integration +- `EchoCancelGuard::module_index()` accessor (currently private; only `source_name()` / + `sink_name()` exist). +- **Emit the public mode flag** when the new picker choice is selected. Gated by an **exact + new/new argv golden** that requires *both* the mode flag and `--aec=โ€ฆ` to be present โ€” the + round-3 P1. An argv test that checks only `--aec` passes while the mode flag is never sent and + pixelpass silently runs legacy capture. +- Always pass `--aec=off|pulse-module:` โ€” absence is not a protocol state (D5). +- **Bind capability to the resolved binary path**; re-probe immediately before constructing + new-mode argv if resolution differs from the probe's; fail closed. Closes the `:3375-3378` + vs `:3409-3419` double-resolution gap. +- **New-peerspeak + old-pixelpass golden test** (round-2: the phase map promised "both + directions" and only one was specified) โ€” against an old-capability response and an old fake + binary: the new picker entry stays absent and **no new flags are emitted**. Path rebinding + alone does not test the failure policy. +- Parse and surface the Phase 6 status events, with a **causal** test: an event emitted by + pixelpass must reach the UI. Matching enums defined independently in both repos would + otherwise pass. The notice channel is currently created only when `audio_app` is set + (`core/mod.rs:3424`) and only the two `AppAudio` events are translated (`:3431-3434`); + everything else is logged and lost โ€” so the new mode needs its own channel creation path. +- Capability-gated picker entry; wording per v3.4 ยง11. +- Regression: existing `--app` / `--strict-audio` argv byte-identical to today. + +--- + +## 7. Phase 9 โ€” rig upgrade and field tests ๐Ÿšฆ SHIP GATE + +v3.4 ยง9.2's rig upgrade is **owed before any exclusion claim is published**: two orthogonal +PN/MLS probes, windowed per-channel normalised cross-correlation reporting max per-window +correlation, plus xrun telemetry. Until it exists the only defensible claim is the gross-leak +distinction, in v3.4 ยง9.2's exact wording. + +**Every row gets a declared pass/fail threshold before the run, not after.** Baseline for all +rows: excluded probe โ‰ค the declared rig criterion; **eligible control audio present**; original +playback routes intact; zero surviving owned links or capture sinks after teardown; xrun and CPU +within recorded bounds. + +The **full** v3.4 ยง12 matrix โ€” v1 silently dropped rows 2 and 7: + +1. Sharer in a call while sharing, AEC on **and** off. +2. **Sharer simultaneously viewing another share while sharing** (restored). Highest-value test + of the child-tag path: mpv playing a watched share while hosting. Reachable โ€” + `StartScreenShare` stores a host at `core/mod.rs:3458`, `ViewShare` stores viewer children at + `:3525`, no mutual exclusion. +3. Lifecycle, each separately: Stop, room leave, UI crash, pixelpass panic, SIGINT, SIGTERM, + SIGKILL, last viewer, pipewire-pulse restart, PipeWire daemon restart, `--repair`. +4. EasyEffects running for the whole share. +5. Output-device switch mid-share via the real `Ctrl+Meta+F` / `Ctrl+Meta+S` scripts. +6. Two concurrent hosts; notification mid-share; app that starts playing after the share. +7. **Sample-rate / channel / passthrough behaviour on real sinks, and CPU cost** (restored). + +โš ๏ธ **Mid-share taint-root arrival โ€” v3.4 ยง6.1.4 names an unreachable case, and so did my first +replacement.** v3.4 says "AEC-load-mid-share is the case to test": unreachable, because there is +exactly one `echo_cancel::enable` site at session join (`core/mod.rs:1850`), the guard moves +into `ActiveSession` at `:2729`, and `StartScreenShare` rejects `active_session == None` at +`:3397-3404` ("Join a call before sharing your screen") โ€” so the AEC always predates the share. +My proposed replacement, "a peer joining creates their playback node," is **also wrong**: +peerspeak starts **one mixed playback stream** at session construction (the sole core +`start_playback`, `core/mod.rs:1900`), and `PeerJoined` (`:2396-2409`) only admits and connects +the sender. No per-peer node is ever created. + +The reachable newly-created mid-share taint roots are: **a notification sound played mid-share** +(`notify.rs:265-272`), and **starting to view another share mid-share**, which spawns a tagged +mpv/VLC (`screenshare/mod.rs:768-775`). Those are the transition-window field tests. Owned-AEC +mid-share load stays a **synthetic** test until a second `enable` site or hot reload arms it. + +--- + +## 8. Open questions โ€” final status + +| # | Question | Status | +| --- | --- | --- | +| O1 | Is 0c a true blocker? | **CLOSED โ€” yes, no demotion path.** The *echo* argument for demoting it is sound and irrelevant: D6 settled it. Balloon โ‡’ round 8. | +| O2 | Ship the dry-run mode? | **CLOSED โ€” keep**, env-gated, stable reason codes + epoch + serial, stderr or defined JSON event so `--output json` stays clean. | +| O3 | Naming | **SPLIT.** The `peerspeak.owned` wire literal is pinned in plan ยง3 now (a contract, not product wording). Public mode/picker wording remains the **user's call**, blocking at the start of Phase 7 only. | +| O4 | Sticky state: engine or observer? | **CLOSED โ€” pure engine.** Observer supplies lifetime-bearing membership/removal facts; the engine decides. | +| O5 | Is full recompute really fine? | **CLOSED โ€” measure it in Phase 5**: duration distribution, maximum, and queueing, not a recompute count. | +| O6 | Can the default-monitor fallback be made structurally impossible? | **CLOSED โ€” yes, but it took two closures, not one.** Source path *and* sink-input path, both in 0d. Placement 0d rather than Phase 6 was my divergence; Codex agreed in round 2 with the added constraint `0c โ†’ 0d`. | +| O7 | Does over-exclusion need its own gate? | **CLOSED โ€” subsumed.** Codex correctly narrowed my premise: an exclude-everything build already fails the eligible controls in six Phase 5 rows *provided they are asserted*. Fix is the exact-partition requirement (plan ยง5.1) plus link-matrix row 9's positive capture assertion. | +| O8 | Runtime assertion for "no source switch on failure"? | **CLOSED โ€” 0d's enum is insufficient.** It stops a `DesktopExcluding` value containing `DefaultMonitor`, not a failure handler swapping the whole plan for `LegacyDesktop`. Release-mode integration test per failure transition (link-matrix row 8); `debug_assert!` in addition, but it is not the gate. | + +--- + +## 9. Risk register + +| Risk | Where it bites | Mitigation | +| --- | --- | --- | +| Engine correct, **source** wrong | full echo, engine bypassed | 0d path 1 โ€” typed capture plan | +| Engine correct, **sink inputs** poisoned | full echo, no source switch anywhere | 0d path 2 โ€” bare sink type + conflict rejection + graph assertion | +| Taint engine subtly wrong about real PipeWire | Phase 6 leaks the call into the share | Phase 5 exact-partition gate with negative controls | +| Unsafe link exists briefly, then is cleaned up | a real leak that "eventual cleanup" tests score as a pass | link-matrix row 1: zero unsafe `create_link` calls | +| Link manager built before the engine is validated | same, discovered in front of a viewer | strict DAG; plan ยง0's stated temptation | +| Tag literal mismatch across repos | v3.4 ยง5.1 silently does nothing, *quietly* | literal pinned in plan ยง3; cross-repo black-box test; consumption gated in Phase 5 rows 4โ€“6 | +| Cross-repo skew | share hard-fails on spawn | pixelpass-first; a golden test in **each** direction; capability bound to resolved path | +| Fail-closed with no explanation | user switches back to unsafe whole-desktop audio | causal status-delivery test, Phase 6 โ†’ Phase 8 | +| Over-exclusion ships as "working" | mode captures silence, all gates pass | exact partitions (plan ยง5.1) + link-matrix row 9 | +| Wrong pipewire-pulse PID | mass over-exclusion from a correct engine on a wrong context | Phase 3 PID-derivation matrix | +| 0c balloons | prerequisites eat the schedule | reopen D6 as round 8 โ€” no silent waiver | +| "It works on my box" | the only box is this box | two-machine field test is the ship gate | + +## 10. Adjudication record + +**Round 1 โ€” 13 items, 12 accepted.** Phase reorder (AEC machine before dry-run); typed capture +plan (accepted, moved *earlier* than proposed); Phase 3 five-part gate; tag-consumption gating; +Phase 6 matrix mandatory; 0b unwind backstop restored **and my mutation test corrected โ€” it +targeted the wrong mutation**; Phase 9 rows restored with pre-declared thresholds; DAG stated; +O1 demotion language removed; 0c two-host gate; skew tests moved to Phase 7; status events +assigned. **Half-rejected:** "the Sunshine topology is unverified and unnecessary" โ€” *unverified* +was right and it has since flipped; *unnecessary* rejected, retained as non-gating row 1b. + +**Round 2 โ€” 11 items, all accepted.** The three that mattered: + +- **P1, the sink-input path.** My 0d closed the source and left the capture sink's inputs open; + `PIXELPASS_AUDIO_VIA_NULL_SINK` + `Routing::start` would have filled the owned sink with the + whole-desktop mix and produced full echo with no source switch. This is the "at least one + comparable error" I asked round 2 to find, and it was in the fix for round 1's headline P1. +- **P2, my v3.4 ยง6.1.4 replacement was also unreachable.** I claimed a peer joining creates their + playback node; verified false โ€” one mixed playback stream at session construction + (`core/mod.rs:1900`), `PeerJoined` only admits the sender. Replaced with notification sound and + watched-share start, both reachable. +- **P1, no invocable production selector**, so Phase 6's "production path" measurement would have + run through a harness. Internal mode input moved into 0d. + +**Round 3 โ€” verification pass, 5 items, all accepted.** It confirmed 4 of the 7 round-2 edits +landed and 3 were partial, which is the reason to run a verification round at all rather than +declaring the fixes done. The one that mattered: + +- **P1, the public mode selector was never assigned to any phase.** 0d added a hidden trigger, + Phase 7 added capability + naming, Phase 8 added `--aec` โ€” and nothing required the mode flag + itself to exist publicly or be passed. A capability-gated picker entry would have appeared and, + when chosen, spawned legacy whole-desktop capture: the feature shipping while doing nothing, + with the echo intact. Fixed in Phase 7 (flag) and Phase 8 (emission + new/new argv golden). +- Three link-matrix rows I had just written were insufficiently falsifiable โ€” a combined + exclusive/encoded/passthrough fixture (one working predicate masks two missing), "relink + attempted" (a permanently-failing attempt passes), and an unenumerated "any failure path". + Split into 6aโ€“6c, a success assertion, and 8aโ€“8e. +- Status delivery was gated by one generic causal test that passes while three of the four + events stay unwired. Now four exact JSON goldens with named triggers. +- `0b` was drawn in the DAG with no outgoing edge; it now explicitly precedes Phase 6. + +**Codex's positions I narrowed:** it agreed the Sunshine sample is worth keeping as non-gating, +and corrected my wording โ€” the topology appears when Sunshine *routes desktop audio through its +null-sink topology*, not merely whenever Sunshine is running, since I measured it running +without that topology. It also correctly narrowed O7's premise (an exclude-everything build does +already fail six rows' eligible controls, *if* asserted) while agreeing the exact-partition fix +is right. + +**Verified by me before accepting:** the single `default_audio_monitor` call site +(`pipeline.rs:138`); the double binary resolution (`core/mod.rs:3375-3378` vs `:3409-3419`); the +no-session guard on `StartScreenShare` (`:3397-3404`); the sole core `start_playback` (`:1900`) +and `PeerJoined`'s scope (`:2396-2409`); and the current default-sink/Sunshine graph state. + +## 11. Corrections owed to the design doc (v3.5 / round 8) + +Two measured findings that belong in v3.4 itself, both agreed by both reviewers: + +1. **v3.4 ยง6.1.0's "๐Ÿ”ด the hazard is LIVE on this machine right now" is time-dependent and has + already flipped.** Measured 2026-07-21 ~14:55 (details in plan ยง5.3). The reachability + argument is unaffected โ€” the topology appears **when Sunshine routes desktop audio through + its null-sink topology**, which is narrower than "whenever Sunshine is running," since it was + measured running without it. Nothing should gate on its presence. +2. **v3.4 ยง6.1.4's nominated test case is unreachable, and so was my first replacement.** + Details in plan ยง7. The conclusion (the transition window exists only for newly-created + roots) stands; the example must become the notification sound or watched-share start. + +## 12. Not in this plan + +Everything v3.4 ยง14 lists as out of v1 โ€” port-granular taint, timed drain, hot-AEC-reload epoch +protocol, native PipeWire AEC, incremental dirty-set, seamless daemon-restart recovery โ€” plus +v3.4 ยง10 items 2 and 3 (per-app debt; D6 says they do not block Option C), the v3.4 ยง5.1 +grandchild leak, and the general "AEC binds to the default sink when no device is pinned" defect +(v3.4 ยง6.1.0, resolved for this user, own task).