From 1cd19b355f69c4e21a696a31ba4d9b1e423d65f8 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sat, 25 Jul 2026 17:04:51 -0400 Subject: [PATCH] =?UTF-8?q?docs:=20design=20round=208=20=E2=80=94=20the=20?= =?UTF-8?q?observation=20boundary=20(v3.5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phase-5 dry-run gate failed on its first live run: the engine built to v3.4 could not see its own primary taint root (echo, AEC off) while excluding every stream on the machine (silence). One cause — the PipeWire registry `global` event carries only a filtered subset of an object's properties, and eight the design depends on are never announced. Design doc (v3.4 → v3.5): - NEW §6.7 — the observation boundary. The global is an index, not a source of truth: bind every Node and Device, `info` props are the sole source, live prop tracking, one readiness obligation per unbound node, fail closed. Four user design calls recorded. - §5.1 — a second, registry-visible tag carrier (`node.name` prefix) alongside `peerspeak.owned`, so the primary root does not rest on one mechanism. - §6.4 — node/device props are not an optimisation to skip, they are unavailable from the global; the round-6 Link lesson was right and applied to exactly one object type. - §6.1.0, §6.1.4 — the two corrections the impl plan owed v3.5: a time-dependent "hazard is LIVE" claim, and an unreachable nominated test case (twice over). - §9.1 measured facts, §12 rig discipline (pw-dump binds; the registry does not), §14 readiness. Impl plan: - NEW phase 3r with a four-part exit gate, the first the direct inverse of the finding. Ports deliberately not bound in v1, with a revisit trigger. - Phase 1 pins the second carrier literal as a cross-repo contract. - Phase 5 marked GATE FAILED; matrix and O5 re-run after 3r and 1. - Risk register: the over-exclusion row fired and worked; new row for the observation boundary class. Architecture is unchanged and vindicated: fed correct properties, the engine decided correctly in every fixture. The §5.1 exact-partition requirement is what caught this — every exclusion was defensible and the eligible half was empty. Co-Authored-By: Claude Opus 5 --- docs/screenshare-audio-exclusion-impl-plan.md | 126 ++++++++- docs/screenshare-audio-exclusion-plan.md | 252 ++++++++++++++++-- 2 files changed, 352 insertions(+), 26 deletions(-) diff --git a/docs/screenshare-audio-exclusion-impl-plan.md b/docs/screenshare-audio-exclusion-impl-plan.md index 214e266..603a57a 100644 --- a/docs/screenshare-audio-exclusion-impl-plan.md +++ b/docs/screenshare-audio-exclusion-impl-plan.md @@ -82,6 +82,7 @@ if it differs, failing closed. | 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** | +| **3r** | **Observer revision — bind every Node/Device (v3.5 §6.7)** | pixelpass | read-only | **four-part gate (plan §4 "Phase 3 revision")** | | 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 | @@ -92,13 +93,19 @@ if it differs, failing closed. **Landing DAG** (development may be concurrent; *landing* order may not): ``` -0a ──────────────────► 2 ──► 3 ──► 4 ──► 5 ──► 6 ──► 7 ──► 8 ──► 9 - ▲ -0b ────────────────────────────────────────────┤ -0c ──► 0d ─────────────────────────────────────┘ -1 ────────────────────────────────────────► 5 +0a ──────────────────► 2 ──► 3 ──► 4 ──► 5 ──► 3r ──► 5 (re-run) ──► 6 ──► 7 ──► 8 ──► 9 + ▲ +0b ──────────────────────────────────────────────────────────────────┤ +0c ──► 0d ───────────────────────────────────────────────────────────┘ +1 (r8 carriers) ──────────────────────────────────────► 5 (re-run) ``` +⚠️ **Status 2026-07-25: the phase-5 gate FAILED on its first live run** and put 3r into the +DAG. Phase 5's machinery is built and correct — it is the audit that found the defect — so +"5 (re-run)" is a *re-run of the matrix*, not a rebuild. **Phase 6 does not start** until a +passing results file exists. Phases 2 and 4 need no change; the defect is entirely in 3. +**Phase 1 is now a hard prerequisite of the re-run for both carriers** (plan §3). + - **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 @@ -253,6 +260,31 @@ key: peerspeak.owned value: 1 ``` +⚠️ **Round 8 — a SECOND carrier is required, and its literal is pinned here too** (v3.5 §5.1). +`peerspeak.owned` is invisible to the registry `global` event and readable only via a node +bind (v3.5 §6.7); the prefix below is announced by the registry and needs no bind, so the +primary taint root no longer rests on a single observation mechanism. + +``` +key: node.name +format: peerspeak_owned__ e.g. peerspeak_owned_mpv_31284 +prefix: peerspeak_owned_ ← the matched literal +``` + +- **Both carriers are set at every tagging site.** A node is owned if **either** matches — + union, the fail-closed direction. The engine's tag root is `peerspeak.owned == 1` **OR** + `node.name` starts with `peerspeak_owned_`. +- **`node.description` is NOT touched**, so mixers still show "mpv". Only `node.name`, which + is the internal identifier, carries the prefix. +- The prefix mechanism is already proven here: `pixelpass_capture_*` is matched on + `node.name` and was the only root that kept working under the F1 defect. +- Same three requirements as the property literal: one named constant per repo, the + black-box cross-repo test driven from a shared fixture, and phase 5 as the real proof. +- ⚠️ Native call playback sets both on its own stream dict. The child spawns set the prefix + through the same `PULSE_PROP` / `PIPEWIRE_PROPS` env that carries the property — + `node.name` is settable there, and **the phase-1 exit gate must show it landing on a live + mpv node**, not just in the env. + **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: @@ -299,6 +331,11 @@ fail-closed default asserted at the boundary. 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`). +> ⚠️ **Built and merged, then superseded in part by "Phase 3 revision (round 8)" below.** This +> section's node-property requirements assume the registry `global` event carries them. It does +> not (v3.5 §6.7). Everything here about removals, the readiness epoch, PID derivation and the +> Link path is unaffected and still holds. + - 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. @@ -319,6 +356,59 @@ removals are ignored, the fallback is dead code, and readiness releases early: | **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 3 revision (round 8) — bind every Node and Device 🔴 blocks the phase-5 re-run +v3.5 §6.7. Phase 3 shipped reading node properties off the registry `global` event, where +**eight of them are never announced**. This is the fix. Scope is the observer only — phases 2 +and 4 are unaffected, and the phase-5 audit machinery is already correct. + +**Requirements.** + +1. **Bind every `Node` global**, unconditionally, no `media.class` filter. Retain the proxy + and its `info` listener in that global's slot in the existing per-id FIFO + (`LiveGlobal.bound_link` generalises to a bound-proxy slot). + ⚠️ The phase-3 review's finding 3 — record the id and apply the add as **one** step, so + the proxy FIFO stays lockstep with the model's `live_ids` — now applies on the **hottest** + path in the observer. A recycled Node id must not pop another generation's proxy. +2. **The global is an index; `info` is the source of truth.** Read from the global only what + must exist before the bind resolves: `object.serial` (identity), the object's id, and + `device.id`/`node.id` linkage. **Every** taint-relevant property — including `node.name` + and `media.class`, so there is exactly one source — comes from the bound `info` props. +3. **A node with no `info` yet is WITHHELD from the snapshot and is a readiness obligation** + (`pending_nodes`, beside `withheld` and `pending_links`). `graph_ready` false while any is + outstanding; the existing bounded deadline makes an unresolvable bind sticky-`TimedOut`, + fail closed. No provisional-ownership admission, ever (v3.4 §6.1.3). +4. **Track props for the node's lifetime.** On a later `info` with `PROPS` in `change_mask`, + re-read, re-classify, and apply a `NodePropsUpdated` event. + ⚠️ **Suppression rule:** a prop update may be dropped **only** when the resulting + `Projection` is identical to the current one. Anything looser breaks phase 4's + no-coalescing contract; anything stricter (emitting on every `info`, including + state-only changes) inflates the O5 event rate with non-events. +5. **Bind every `Device` global** and read `device.api` **and** `alsa.driver_name` from its + `info` props — authoritative, and the phase-3 review's owed fix (on PipeWire ≥ 1.2.6 with + WirePlumber < 0.5.13 the driver name is not copied to the node, and the fail-closed + absent-driver rule would over-exclude real cards). `factory.name` exists only on the node. + `classify` takes both sides; node values are the fallback, Device values win. +6. **Ports are NOT bound in v1 — an explicit accepted limitation.** `port.exclusive` is the + only port property missing from the global, and it guards a *mutation* (don't fan out into + an exclusive port), not echo: an exclusive port rejects the second link, so phase 6 sees a + clean link-create failure it must handle correctly anyway. Binding ~21 more objects at + rest to pre-empt an error that surfaces safely is not worth the obligation surface in v1. + **Revisit trigger:** any phase-6 link-matrix row where an exclusive-port link failure is + not cleanly recoverable. (`node.passthrough`, the *other* half of that §6.2 row, is a node + property and **is** recovered by this revision.) + +**Exit gate — four parts.** The first is the direct inverse of the F1 finding. + +| gate | proves | +| --- | --- | +| **live prop recovery**: a `module-null-sink` tagged `peerspeak.owned=true` plus a `module-loopback` reading its monitor — assert the projection carries `peerspeak.owned`, `pulse.module.id`, `node.link-group` **and** `factory.name`/`device.api`/`alsa.driver_name` on a real ALSA node | the eight properties actually arrive — F1 cannot recur silently | +| **pure-model prop-update matrix**: props-changed → re-classified; identical props → suppressed; a `session_device`-relevant change flips classification. (A *live* prop mutation has no reliable CLI trigger — the pure test is the gate, a live sighting is opportunistic) | the lifetime-tracking path exists and its suppression rule is exact | +| **readiness with node binds**: no projection reports `graph_ready` while a node bind is outstanding; an `info` that never arrives ends in sticky `TimedOut` | withholding and fail-closed timeout still hold with the new obligation class | +| **recycled Node id under churn**: repeated add/remove of the same id; no proxy leak, no cross-generation misattribution | the FIFO lockstep rule survives being moved to the hot path | + +**Then re-run the whole phase-5 §5.1 matrix and re-measure O5** with bind I/O included — the +existing numbers were taken on the degraded graph and inherit nothing. + ### 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`**; @@ -343,6 +433,18 @@ observable in Phase 5 before they gate anything real. ## 5. Phase 5 — dry-run audit mode 🚦 MAJOR GATE +> **🚦 STATUS 2026-07-25: BUILT, RUN, AND THE GATE FAILED.** Results: +> `docs/screenshare-audio-exclusion-phase5-results.md`. The machinery is correct and needs no +> rework — **it found the defect on its first live run**, which is the phase working exactly as +> designed. What failed is the observer beneath it (v3.5 §6.7). **Phase 6 does not start.** The +> matrix re-runs after phase 3r and phase 1's second carrier land; no row was completable +> under the defect, so none of it carries over. O5's numbers do not carry over either. +> +> Read this before re-running: `PIXELPASS_AUDIO_AUDIT_FILE=… PIXELPASS_AUDIO_AUDIT_AEC=off +> pixelpass --audit-audio`. **Every partition row must run with `AEC=off`** — a +> configured-but-unvalidated AEC shuts the fan-out gate and empties the eligible half of every +> row, which reads as a failure that is really a harness error. + **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 @@ -608,7 +710,8 @@ mid-share load stays a **synthetic** test until a second `enable` site or hot re | 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 | +| Over-exclusion ships as "working" | mode captures silence, all gates pass | exact partitions (plan §5.1) + link-matrix row 9 — **🟢 FIRED 2026-07-25 and worked**: the build *was* the exclude-everything degenerate case, and the empty eligible half is what exposed it | +| **A property the engine reads is silently absent at the observation boundary** | engine correct, context permanently `None`; fails in *both* directions at once (F1: no taint root ⇒ echo; F2: no owner key ⇒ exclude everything) | **v3.5 §6.7 — never read node/device props off a registry global.** Phase 3r's live prop-recovery gate asserts each one arrives. General form: `pw-dump` is a **bound** view; the registry is not, and the difference is silent | | 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 | @@ -665,9 +768,10 @@ is right. 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) +## 11. Corrections owed to the design doc — ✅ APPLIED in v3.5 (round 8, 2026-07-25) -Two measured findings that belong in v3.4 itself, both agreed by both reviewers: +Both are now in the design doc (§6.1.0 and §6.1.4 respectively), alongside round 8's own +finding (§6.7, the observation boundary). Kept here as the record of what was owed and why: 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 @@ -680,6 +784,12 @@ Two measured findings that belong in v3.4 itself, both agreed by both reviewers: ## 12. Not in this plan +**Round 8 additions:** **port binding** (so `port.exclusive` is never observed — plan §4 "Phase +3 revision" item 6, with its revisit trigger), **per-node quarantine** (an unresolvable node +bind fails the whole readiness epoch closed instead of isolating that one node — v3.5 §6.7 +decision 3), and the **serial-continuity signal** for the AEC validator's no-coalescing +contract (phase 4's owed F4 hardening). + 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 diff --git a/docs/screenshare-audio-exclusion-plan.md b/docs/screenshare-audio-exclusion-plan.md index ef85475..459e438 100644 --- a/docs/screenshare-audio-exclusion-plan.md +++ b/docs/screenshare-audio-exclusion-plan.md @@ -1,7 +1,10 @@ # Design v3: whole-desktop screen-share audio without self-echo -**Status:** 🟢 **v3.4 — CONVERGED after 7 review rounds. Ready for implementation planning; NOT approved for merge.** -**Date:** 2026-07-21 (v1: 07-19 · v2: 07-20 · Option C 07-20 · v3.1 r4 · v3.2 r5 · v3.3 r6 · v3.4 r7) +**Status:** 🟠 **v3.5 — round 8, reopened by a MEASURED implementation finding.** v3.4's +architecture is unchanged and still converged; what changed is the **observation boundary** +(new §6.7), which v3.4 got wrong in a way that made the built engine non-functional. +**Date:** 2026-07-25 (v1: 07-19 · v2: 07-20 · Option C 07-20 · v3.1 r4 · v3.2 r5 · v3.3 r6 · +v3.4 r7 · v3.5 r8) **Origin:** Joe's suggestion — "whitelist all audio except audio coming from peerspeak." **Scope:** a new capture mode in pixelpass (`src/host/pipeline.rs`, `src/host/audio.rs`), playback tagging + AEC-identity export + teardown-ordering invariants in peerspeak. @@ -21,6 +24,7 @@ v1/v2 remain in git history at `88ad5a0` and `10203e1`. | v2 review | `…/review-2026-07-20-audio-exclusion-design-v2.md` | blocked; produced Option C | | fan-out spike | `~/Documents/handoff-docs/Claude/peerspeak/fanout-spike-results-2026-07-20.md` + Codex rounds 3/4 | **Option C adopted**, ratified | | AEC identity gate | `~/Documents/handoff-docs/Claude/peerspeak/aec-playback-leg-identity-2026-07-20.md` | **🟢 gate passed**, both models agree after 2 adversarial rounds | +| **phase 5 dry-run gate (r8)** | `docs/screenshare-audio-exclusion-phase5-results.md` | **🚦 GATE FAILED** — the observation boundary is wrong (§6.7); architecture unaffected | --- @@ -164,6 +168,24 @@ back at them," not "defend against a hostile local process." Stated, not assumed ⚠️ **Known leak: a stable boolean is inherited by grandchildren.** Anything mpv or VLC spawns is exempted too. Accepted for now; revisit if it bites. +⚠️ **Round 8 — the tag needs a SECOND, registry-visible carrier (user's call, 2026-07-25).** +`peerspeak.owned` is invisible to the registry `global` event (§6.7) and is readable only via +a node bind. That is safe — an unread tag means the node is withheld or the epoch fails +closed, i.e. silence, never echo — but it makes the primary taint root depend on the one +mechanism the round-8 finding proved fragile. So each tagging site sets **both**: + +| carrier | visibility | role | +| --- | --- | --- | +| `peerspeak.owned=1` | bind only | primary root; keeps the measured `PULSE_PROP` env-inheritance mechanism for mpv/VLC/paplay | +| a pinned `node.name` prefix | **announced by the registry** | secondary root; fires with no bind at all | + +The prefix mechanism is already proven in this codebase — `pixelpass_capture_*` is matched on +`node.name` and was the *only* root still functioning under the defect. Either carrier alone +marks a node owned (union, not intersection: fail-closed direction). `node.description` is +deliberately left untouched so volume mixers still read "mpv", not "peerspeak: mpv". The +literal is a cross-repo wire contract and is pinned in the implementation plan §3 alongside +the property literal. + ### 5.2 The AEC playback leg — 🟢 gate passed, with five load-bearing corrections `module-echo-cancel` is loaded via pactl (`peerspeak/src/audio/echo_cancel.rs:83-94`), so @@ -302,10 +324,24 @@ If any node re-emits audio it received, the re-emitting node is a fresh, *untagg `Stream/Output/Audio` carrying the mix — including peerspeak's playback and the AEC leg, both of which were correctly excluded one hop earlier. -### 6.1.0 🔴 The hazard is LIVE on this machine right now — not hypothetical +### 6.1.0 The hazard is REACHABLE on this machine — it was live when measured -Discovered 2026-07-21 while measuring something else. **The user's current default sink is -not hardware.** `pactl info` reports: +⚠️ **Round-8 correction to this heading, and it is the point of the correction.** v3.4 said +"🔴 LIVE on this machine **right now**." That was true at 2026-07-21 ~09:00 and **false by +~14:55 the same day** — six hours later the default sink was +`alsa_output.pci-0000_10_00.6.analog-stereo` (IDLE), all three `sink-sunshine-*` null sinks +SUSPENDED, with Sunshine still running the whole time. So the topology appears when +**Sunshine actually routes desktop audio through its null-sink chain**, which is strictly +narrower than "Sunshine is running." + +**Nothing may gate on this topology being present.** The reachability argument below is +unaffected and stands: a stock third-party application puts a virtual-sink forwarder in the +default audio path on an ordinary desktop, no EasyEffects required. But as a *test* input it +is opportunistic only — the controlled `module-null-sink` + `module-loopback` fixture is the +authoritative one because it is deterministic and always available. + +Measured 2026-07-21 ~09:00. **The user's default sink was not hardware.** `pactl info` +reported: ``` Default Sink: sink-sunshine-stereo ← factory.name = support.null-audio-sink @@ -550,7 +586,25 @@ worst case is a sub-quantum-to-tens-of-ms audible sliver at the transition. merely become non-silent (the common case — the AEC and call playback nodes are created at join, long before anyone speaks), then topological taint has **already** excluded everything downstream and there is no window at all. The window exists only when a taint -root is *newly created* mid-share. That makes AEC-load-mid-share the case to test. +root is *newly created* mid-share. + +⚠️ **Round-8 correction: v3.4 nominated an unreachable test case here** ("that makes +AEC-load-mid-share the case to test"), **and so did its first replacement.** The conclusion +above is unaffected; only the example was wrong. + +- *AEC-load-mid-share is unreachable.* There is exactly one `echo_cancel::enable` site, at + session join (`core/mod.rs:1850`); the guard moves into `ActiveSession` (`:2729`); and + `StartScreenShare` rejects `active_session == None` (`:3397-3404`). The AEC always + predates the share. +- *"A peer joins and their playback node is created" is also unreachable.* peerspeak starts + **one mixed playback stream** at session construction (`core/mod.rs:1900`); `PeerJoined` + (`:2396-2409`) admits and connects the sender and creates no per-peer node. + +**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 a hot reload arms it. This means pixelpass must **track Node, Port and Link globals** — not just node global-add, which is all the existing router does (`pixelpass/src/host/audio.rs:523-584` @@ -614,6 +668,16 @@ An add-only listener never sees it. Required: **drop the observer proxy**. Retained proxies are needed only for *our own* fan-out links, where dropping is the ownership/lifecycle mechanism (§4.2) — not for external links we merely observe. + ⚠️ **Round 8: measured, the Link endpoint props are always present** — this fallback is + correctness insurance that never fires on this host. The bullet above it is still right; + it was just under-applied. See **§6.7**. +- 🔴 **Node and Device props are NOT AVAILABLE from the global at all** — not an + optimisation, not usually-present, never. Binding each Node and Device is the **only** + path to `peerspeak.owned`, `pulse.module.id`, `node.link-group`, + `application.process.id`, `node.passthrough`, `factory.name`, `device.api` and + `alsa.driver_name`. **§6.7** is the whole story; it is the round-8 finding and it + supersedes any reading of this section that treats node props as observable off the + registry. - **Readiness epoch.** Start fan-out only when: initial `core.sync`/`done` received **and** every required Link/Port/Client/Node observer fallback has resolved or hit a fail-closed timeout. Then **revalidate in the same graph epoch immediately before creating each @@ -641,6 +705,102 @@ EasyEffects/filter/loopback-routed audio is captured at all." Treat an unvalidated `echo-cancel-*` output node as an excluded hazard rather than an eligible stream (§5.4). +### 6.7 🔴 The observation boundary — registry props are a FILTERED SUBSET (round 8, MEASURED) + +**This is the finding that failed the phase-5 gate**, and it is the one place v3.4 was +materially wrong. Full evidence: `docs/screenshare-audio-exclusion-phase5-results.md` (F1). + +**The fact.** The registry `global` event does **not** carry an object's properties. It +carries a small fixed subset the daemon copies onto the global. Measured on PipeWire 1.6.8 / +WirePlumber 0.5.15, the complete union of keys announced for a `Node` global across every +node on the box is: + +``` +application.name client.api client.id device.id factory.id media.class +node.description node.name node.nick object.path object.serial +priority.driver priority.session +``` + +Eight properties this design depends on are **never** announced, and a props lookup for them +returns absent rather than failing — so an implementation reading them off the global is +silently, permanently wrong: + +| property | what dies without it | +| --- | --- | +| `peerspeak.owned` | **the primary taint root** — §5.1, all of phase 1 | +| `pulse.module.id` | **AEC identity exclusion** — §5.2 and the §5.3 validator | +| `node.link-group` | owner key 1 — echo-cancel, EasyEffects, loopback siblings (§6.1.2) | +| `application.process.id` | owner key 4 — the measured GStreamer split-client case (§6.1.2) | +| `node.passthrough` | the passthrough exclusion (§6.2) | +| `device.api` | `session_device` classification | +| `factory.name` | `session_device` classification — the discriminator itself | +| `alsa.driver_name` | `session_device` classification — the `snd_aloop` denylist | + +`port.exclusive` is likewise absent from `Port` globals, so that §6.2 exclusion never fires. +**Links and Clients are unaffected** — every endpoint key and `pipewire.sec.pid` is announced, +so §6.1.2's pulse-PID derivation works as designed. + +The consequence measured live was not degradation but inversion: with `peerspeak.owned` +invisible the tagged sink is **not a taint root at all** (an echo, with AEC off), while the +one root that still fires — the `pixelpass_capture_*` name prefix, `node.name` being +announced — leaves every candidate with no *strong* owner key, which trips §6.1.1's +fail-closed backstop and excludes every stream on the machine. Both halves broken, in +opposite directions, from one cause. + +**The rule (round 8).** + +> **A node's properties come from a bind, never from the global.** The registry global is an +> *index* — it tells us an object exists, its id, its serial, its media class and its name — +> and nothing it says about ownership, grouping or hardware identity may be trusted or even +> read. Every `Node` global is bound and its `info` props are the sole source of node +> properties. Same for `Device`. + +Verified recoverable by bind on the same objects that lack them from the registry: +`factory.name = api.alsa.pcm.sink`, `device.api = alsa`, `alsa.driver_name = snd_usb_audio`, +`peerspeak.owned = true`, `pulse.module.id`, `node.link-group = loopback-2528-13`, +`application.process.id`. + +Four decisions fix the shape (user's calls, 2026-07-25): + +1. **Bind every `Node` global, unconditionally** — no filtering by `media.class`. Deciding + which nodes matter *before* their props exist is the same class of mistake as reading the + props off the global, and a node skipped for looking irrelevant has no owner keys, which + is exactly the unbounded-reader condition that produced the machine-wide cascade. Cost is + ~14 nodes at rest on this box. +2. **Track prop changes for the node's lifetime.** `info` fires again with `PROPS` set in + `change_mask`; re-read, re-classify, re-emit. A one-shot read (the Link-fallback pattern) + would miss a `node.link-group` or `pulse.module.id` set after node creation. Measured + cheap: one change event in 8 s at rest. +3. **One readiness obligation per unbound node**, and a node with no `info` yet is **withheld + from the snapshot entirely** — never admitted with provisional ownership (the §6.1.3 rule, + already applied to device-unresolved nodes). If a bind never resolves, readiness goes + sticky-`TimedOut` and there is no fan-out at all: identical to a never-resolving Link + bind, one rule for both. ⚠️ *Owed hardening:* per-node quarantine (that node ineligible + **and** taint-bearing, the rest of the graph still working) is strictly better and is + deferred because it is a new concept in the pure engine, not a fix to the observer. +4. **`session_device` reads from both.** `factory.name` exists only on the node, so the node + bind is required regardless. But `device.api` **and** `alsa.driver_name` are on the bound + `Device`'s `info` props (measured — they are absent from the Device *global*, which is + what the phase-5 results file checked). Reading the ALSA driver from the backing Device is + authoritative and closes the phase-3 review's owed fix: on PipeWire ≥ 1.2.6 with + WirePlumber < 0.5.13 the driver name is not copied to the node, and the fail-closed + "absent driver ⇒ not a session device" rule would over-exclude real cards. + +**Why v3.4 missed it, which is the transferable lesson.** Round 6 settled the *same question* +for Links and got it right — "endpoint props are an OPTIMISATION, the bind is the correctness +path" (§6.4) — on the explicit reasoning that `pw-dump` binds, so observing a key via +`pw-dump` says nothing about whether the **global-add** carries it. Every property in the +table above was likewise confirmed via `pw-dump`. The doubt was correctly formed and then +applied to exactly one object type. The general form: **`pw-dump` is a bound view; the +registry is not, and the difference is silent.** + +Two things this does *not* change: the architecture (Option C, taint as a graph property, the +owner-key union, sticky taint — all vindicated, and the fixtures that fed them correct props +produced correct answers), and the Link/Client observation path. + +⚠️ **O5 must be re-measured.** The phase-5 numbers (max 15 µs recompute, `busy_fraction` +0.0004) were taken on the graph this defect produces and do not include per-node bind I/O. + ## 7. Lifecycle and teardown invariants ### 7.1 ⚠️ The invariant @@ -762,6 +922,23 @@ peerspeak spawns it, so: - Naive fan-out of the AEC leg leaks remote audio at ≈desktop level; exact exclusion sits at the control floor. +**Round 8 (2026-07-25), from the phase-5 dry-run against the live graph:** + +- The registry `global` event announces **13 keys** for a Node and **none** of the eight + ownership/identity properties this design reads (§6.7). `port.exclusive` is absent from + Port globals. Link endpoint keys and Client `pipewire.sec.pid` **are** announced, on every + object, always. +- **Binding recovers all of them** — `factory.name`, `device.api`, `alsa.driver_name`, + `peerspeak.owned`, `pulse.module.id`, `node.link-group`, `application.process.id` read off + the bound object's `info` props. +- `factory.id` is **not** a shortcut to `factory.name`: every ALSA node claims `factory.id` + 19, whose Factory resolves to `"adapter"`, not `api.alsa.pcm.sink`. +- `alsa.driver_name` and `device.api` **are** on the **bound `Device`**'s `info` props + (absent from the Device *global*) — the authoritative source for §6.7 decision 4. +- Full recompute per graph event: **max 15 µs, mean 4 µs** over 334 recomputes at a 47 Hz + event rate under deliberate churn; `busy_fraction` 0.0004. Closes O5 for the engine — + ⚠️ but measured on the degraded graph, with no per-node bind I/O in it. + ### 9.2 ⚠️ Wording discipline — three overclaims already made, do not make a fourth The correct statement of the exclusion result is: @@ -878,7 +1055,11 @@ filter on `media.class` **first** when selecting nodes with `jq` — `media.name matches the Client object; **never** filter stderr out of a measurement run; **verify the link exists in the graph** before measuring; reproduce over the transport production actually uses (`parec -d .monitor`, not `pw-record --target `, which -reads −91 dB silence). +reads −91 dB silence); **(r8)** never conclude a property is *observable* because +`pw-dump` shows it — `pw-dump` **binds**, the registry does not, and the eight properties +of §6.7 look identical in `pw-dump` to properties the code can actually see. Compare +`pw-cli ls ` (registry view) against `pw-dump` (bound view) before depending on any +key. ## 13. Decisions — resolved in round 4 @@ -903,11 +1084,33 @@ Both reviewers agree on all seven. Recorded as decided; reopen only with new evi - **D7 — no materially simpler design exists** that still meets Joe's ask. The available simplification is to *narrow v1 scope*, not to change architecture. ✅ -## 14. Readiness — 🟢 CONVERGED (round 7). Ready for implementation planning. +## 14. Readiness — 🟠 v3.5 (round 8): architecture converged, observation boundary REVISED. -**Both reviewers agree v3.4 is ready to become the implementation plan.** Seven rounds; -every blocker raised has been either fixed or refuted with evidence. This is *design* -approval — **nothing is approved for merge**, and no code has been written. +**Round 8 was not a review round.** It was opened by the phase-5 dry-run audit failing its +gate on the first live run: the engine built to v3.4 was measured **non-functional** — it +failed to recognise its own primary taint root (an echo) while excluding every stream on the +machine (silence). One cause, §6.7: node properties are not observable from the registry. + +What that does and does not touch: + +| | verdict | +| --- | --- | +| Architecture — Option C, taint as a graph property, owner-key union, sticky taint, AEC identity state machine | **unchanged and vindicated.** Fed correct properties, the engine decided correctly in every fixture; the defect is entirely at the observation boundary | +| §5.1 tagging | **revised** — a second, registry-visible carrier added | +| §6.4 implementation shape | **revised** — node/device props require a bind | +| §6.7 | **new** — the observation boundary, and the rule that the global is an index, not a source of truth | +| §6.1.0, §6.1.4 | **corrected** — a time-dependent claim and an unreachable test case | +| Phases 2 and 4 (pure engine, AEC validator) | **no change owed** | +| Phase 3 (observer) | **must be revised before phase 5 re-runs** | + +The §5.1 exact-partition requirement is what caught this, exactly as argued: the build's +exclusions were all defensible, and an exclusion-only checklist would have passed it. It was +the *eligible* half of the partition being empty that exposed the failure. + +**Rounds 1–7 record.** Both reviewers agreed v3.4 was ready to become the implementation +plan; every blocker raised was either fixed or refuted with evidence. That remains true — +round 8 found something no design review could have, because it required running the code +against a live daemon. How the blockers closed: @@ -922,6 +1125,7 @@ How the blockers closed: | 6 | current-only taint forgets buffers | **conceded** — §6.1.3 sticky taint | | 7 | "resolves" = first-present would still leak | **fixed** — §6.1.2 wording trap + test | | 7 | stickiness on recyclable ids | **fixed** — §6.1.3 lifetime-aware owner components | +| **8** | **node props are not on the registry global (measured, phase 5)** | **fixed** — §6.7 bind-every-node rule; §5.1 second carrier | ### v1 scope — agreed @@ -936,12 +1140,24 @@ Node-granular taint only (no port modelling, duplex over-taint accepted) · no t no hot-AEC-reload epoch protocol · no native PipeWire AEC support · no incremental dirty-set (full recompute) · no "implausibly many streams" heuristic as correctness · pipewire-pulse/PipeWire daemon restart handled as **revoke and stop**, not seamless -recovery. +recovery · **(r8)** no per-node quarantine — an unresolvable node bind fails the whole +readiness epoch closed rather than isolating that node (§6.7 decision 3) · **(r8)** no Port +binding, so `port.exclusive` is never observed and the §6.2 row it guards relies on the link +create failing cleanly · **(r8)** no serial-continuity signal for the AEC validator's +no-coalescing contract. -### Next step +### Next step (round 8) -Turn this into a sequenced implementation plan (prerequisites §10 items 1/4/5 first, then -the taint engine behind its unit-test surface, then the link manager, then the peerspeak -tagging and `--aec` plumbing), and **field-test §12** — the rig upgrade in §9.2 is still -owed, and nothing in this design has been tested over the real GStreamer/AAC/network path -or on two machines. +The sequenced implementation plan exists (`screenshare-audio-exclusion-impl-plan.md`) and +phases 0a, 2, 3, 4 and 5 are built. Round 8's work is: + +1. **Revise phase 3** to §6.7: bind every Node and Device, live prop tracking, one readiness + obligation per unbound node. Plan §4 "Phase 3 revision (round 8)". +2. **Revise phase 1** to emit both carriers (§5.1), literals pinned in plan §3. +3. **Re-run the whole phase-5 §5.1 matrix** — no row was completable under the defect — and + **re-measure O5** with bind I/O in it. Phase 6 stays blocked until that results file + passes. + +Still owed beyond that, unchanged: the §9.2 rig upgrade before any exclusion claim is +published, and **field-test §12** — nothing in this design has been tested over the real +GStreamer/AAC/network path or on two machines.