docs: sequenced implementation plan for screenshare audio exclusion
Turns the converged v3.4 design into ordered phases with falsifiable exit gates. Three adversarial review rounds with Codex (gpt-5.6-sol, xhigh); findings adjudicated rather than accepted wholesale, with reachability verified against source on both sides. Structural decisions: - Phase 0d closes BOTH unsafe paths into the capture (source string and capture-sink inputs) before any machinery that could take them exists. - Phase 5 dry-run audit mode is a hard gate: the taint engine runs against the live graph, creating no links, asserting exact eligible/excluded partitions with reason codes. - Link manager is deliberately last among the pixelpass components. Two measured corrections owed back to v3.4 (plan §11): §6.1.0's "hazard is LIVE right now" has already flipped and must not be gated on, and §6.1.4 nominates an unreachable test case (as did my first replacement for it). Design approval only. No code, nothing approved for merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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::<u32>` 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 <first-pixelpass-pid>
|
||||||
|
# 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<Routing>, 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/<pid>/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:<idx>` 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 <sink>.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:<idx>` — 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).
|
||||||
Reference in New Issue
Block a user