docs: record the 0c mechanism probe, and revise 0b's mutation matrix
The 0c mechanism probe was run on this host before any structural work, because one unverified assumption could have invalidated the whole approach: whether a hand-created `adapter` node is visible to pipewire-pulse under the name the capture path depends on. It is. Five gates green, including the two that mattered — `<node.name>.monitor` is exposed as a Pulse source, and SIGKILL of the owning connection removes both Pulse-visible names with zero graph residue. No null-sink module is involved at any point. Every O1 stop condition for 0c is retired, and the default sink never moved, so the probe is safe on a live desktop. The probe also settled the native-sink scope question: it applies to every mode that owns a capture sink, not only `DesktopExcluding`. That makes the `--repair` rework load-bearing rather than defensive — repair derives dead PIDs only from `module-null-sink` entries, so once the sink is native its loopbacks become undiscoverable orphans. §10 gains rounds 14 and 15: the 0b matrix drops to four mutations because the best-effort wake arm is unreachable by construction (the loop owns a sender, and the biased select would win anyway), and teardown is hoisted to one unconditional post-loop site instead of being duplicated across one live arm and one dead one. Round 15 records the two blocking implementation-review findings and the vacuous gate of my own that the review's test-double critique exposed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -151,7 +151,19 @@ v3.4 §7.2, decision D4. All **three** of v3.4's fixes:
|
||||
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
|
||||
> ✅ **0b IMPLEMENTED 2026-07-26** (peerspeak branch `phase-0b-teardown`). The ordering
|
||||
> defect was live: `echo_cancel` was declared *ahead* of `screenshare_host`, so any unwind
|
||||
> unloaded the AEC while the host was still fanning out. Fields moved into
|
||||
> `src/core/teardown.rs` with `echo_cancel` declared last, and `ReapOnDrop` added because
|
||||
> `kill_on_drop(true)` only *signals* — it hands the child to the runtime's orphan queue,
|
||||
> which an unwinding runtime may never drain. Matrix revised to four mutations; see §10
|
||||
> round 14, and round 15 for the two blocking review findings that followed.
|
||||
>
|
||||
> **Owed to phase 9:** an explicit lifecycle row — *drop the controller / close the command
|
||||
> channel while sharing* — which is the live proof for the hoisted teardown call site.
|
||||
|
||||
⚠️ **Mutation testing: five mutations, each independently breaking a named test.**
|
||||
⚠️ **SUPERSEDED by §10 round 14 — mutation 2 is vacuous and the matrix is now four.** 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:
|
||||
|
||||
@@ -167,6 +179,45 @@ Both channel-close arms get their own test; a single "closes the command channel
|
||||
exercise one arm and leave the other unsafe.
|
||||
|
||||
### 0c. Graceful stop + connection-owned capture sink — both
|
||||
|
||||
> ✅ **MECHANISM PROBE PASSED on this host, 2026-07-26** (PipeWire 1.6.8). Run *before* any
|
||||
> structural work, on the reviewer's insistence, because a single unverified assumption could
|
||||
> have invalidated the entire approach: whether a hand-created adapter is visible to
|
||||
> pipewire-pulse under the name pixelpass's capture path depends on. It is.
|
||||
>
|
||||
> ```
|
||||
> pw-cli> create-node adapter factory.name=support.null-audio-sink \
|
||||
> node.name=pixelpass_probe_<pid> media.class=Audio/Sink \
|
||||
> audio.channels=2 audio.position=[FL,FR] node.virtual=true \
|
||||
> monitor.channel-volumes=true object.linger=false
|
||||
> ```
|
||||
>
|
||||
> Five gates, all green:
|
||||
> 1. `pactl list short sinks` shows the sink under the **exact** `node.name`.
|
||||
> 2. `pactl list short sources` shows **`<node.name>.monitor`** — the derived monitor name is
|
||||
> a pipewire-pulse contract, not a property of Pulse-created sinks. This was the one that
|
||||
> could have sunk the approach.
|
||||
> 3. `gst-launch-1.0 pulsesrc device=<node.name>.monitor num-buffers=40 ! fakesink` pulled its
|
||||
> buffers and exited clean, and a real recording stream attached — so the pixelpass capture
|
||||
> path works against it unchanged.
|
||||
> 4. **No null-sink module was loaded** (`pactl list short modules | grep -c null-sink` stayed
|
||||
> at its baseline of 3). It is genuinely not a Pulse module.
|
||||
> 5. **SIGKILL of the owning connection removed both Pulse-visible names**, with zero residue
|
||||
> anywhere in `pw-dump`. That is the entire point of 0c, demonstrated on the real graph.
|
||||
>
|
||||
> The default sink never moved, so this is also safe to run on a live desktop.
|
||||
> **Every O1 stop condition listed for 0c is retired.** `object.linger=false` is load-bearing:
|
||||
> the bundled pipewire-rs example sets `linger=1` for the opposite behaviour.
|
||||
>
|
||||
> ⚠️ **`--repair`'s job does not shrink — it BREAKS.** Discovery derives dead host PIDs
|
||||
> **only** from `module-null-sink` entries (`pixelpass/src/repair.rs`), and only then matches
|
||||
> loopbacks against that PID set. The native sink is scoped to **every mode that owns a
|
||||
> capture sink**, not just `DesktopExcluding`, so legacy Pulse loopbacks will coexist with a
|
||||
> connection-owned sink; when that host dies the sink vanishes automatically and its loopbacks
|
||||
> become **undiscoverable orphans**. Candidate PIDs must be derived independently from all
|
||||
> three module shapes (`null-sink sink_name=`, `loopback sink=`, `loopback source=…monitor`),
|
||||
> with a liveness recheck immediately before each destructive unload. This makes the repair
|
||||
> rework **load-bearing, not defensive**.
|
||||
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.
|
||||
@@ -758,6 +809,51 @@ mid-share load stays a **synthetic** test until a second `enable` site or hot re
|
||||
|
||||
## 10. Adjudication record
|
||||
|
||||
**Round 14 (2026-07-26) — 0b's five-mutation matrix is revised to four, and one pinned
|
||||
mutation is retired as vacuous.** Reached independently by both reviewers, then agreed.
|
||||
|
||||
- **Mutation 2 cannot be killed by any test, because its site cannot execute.** The
|
||||
best-effort wake arm (`core/mod.rs`, the `besteffort_wake_rx` close arm) is unreachable
|
||||
**by construction, twice over**: (i) `run_core_loop` owns a clone of `besteffort_wake_tx`
|
||||
— created at `CoreController::new` and used for the `has_more` re-arm inside the loop —
|
||||
and a tokio `Receiver::recv()` yields `None` only once *every* sender is dropped; (ii)
|
||||
even without that clone, both `CoreController` and `CoreCommandSender` hold `reliable_tx`
|
||||
alongside the wake sender, and the `select!` is `biased` with the reliable arm first, so
|
||||
the reliable arm always wins the race to exit. Writing teardown there would be code that
|
||||
provably never runs, dressed as a tested path.
|
||||
- **Mutation 1's site is reachable but not unit-testable.** It sits inside `run_core_loop`,
|
||||
which builds a real iroh endpoint and loads identity; no unit test can drive it.
|
||||
- **Decision: (b) + (c).** Teardown is **hoisted to one unconditional site after the loop**,
|
||||
so every `break` is covered structurally, including any added later — strictly better than
|
||||
duplicating teardown across one live arm and one dead one. The **seam-level mutation gates
|
||||
are the real ordering proof**, and the call site's live proof is owed to **phase 9**, which
|
||||
gains an explicit row: *drop the controller / close the command channel while sharing*.
|
||||
"UI crash" is not precise enough to serve as that row.
|
||||
- **Rejected: an integration test built to preserve the number five.** It would pay for a
|
||||
full iroh core plus test-only observability and prove only that a method was called — not
|
||||
the ordering invariant, which is the thing that actually breaks.
|
||||
- The 0b gate is therefore **four mutations** (no wait · reversed field order · no reap loop ·
|
||||
plus the 0c pair below), each killed by its own named test, with 4-vs-5 verified separated:
|
||||
reversing the field order leaves the reap test green and removing the reap loop leaves the
|
||||
ordering test green.
|
||||
|
||||
**Round 15 (2026-07-26) — review of the 0b/0c-peerspeak implementation returned two blocking
|
||||
findings, both accepted.** Recorded because both are the same shape: a defence that existed
|
||||
but was disarmed exactly when it was needed.
|
||||
|
||||
- **The drop fallback was disarmed across its own wait.** `shutdown` took the child out of
|
||||
the wrapper before the first `.await`; a cancellation or unwind during the wait left the
|
||||
raw child to drop with `kill_on_drop` (which signals without reaping) while `Drop` found
|
||||
`None`. The child now stays owned until the reap is **confirmed**.
|
||||
- **A failed wait was reported as a reap, and the hard-kill wait was unbounded.** The
|
||||
`io::Result` was discarded, so a wait error returned "reaped"; and a process in
|
||||
uninterruptible sleep after SIGKILL could wedge the core loop forever. Both waits are now
|
||||
bounded and the conflict case has a written policy: availability wins, the child stays
|
||||
owned so the bounded `Drop` retry stays armed, and the residual risk is logged.
|
||||
- **A gate of mine was vacuous and the review's fourth test-double point caught it.** The
|
||||
elapsed-time assertion compared against `STOP_GRACE` itself, so zeroing the constant left
|
||||
it trivially true. `the_grace_is_a_real_interval` now pins the constant to a band.
|
||||
|
||||
**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
|
||||
|
||||
Reference in New Issue
Block a user