docs: design round 8 — the observation boundary (v3.5)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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_<role>_<pid> 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:<idx>` 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
|
||||
|
||||
@@ -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 <sink>.monitor`, not `pw-record --target <sink-node-id>`, 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 <Type>` (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.
|
||||
|
||||
Reference in New Issue
Block a user