59da73c01364558ccf634e45f9a51ac0115efdcb
400
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  molluskandClaude Opus 5
|
59da73c013 |
docs: the phase-5 gate passed, so stop telling phase 6 it is blocked
CI / check (push) Waiting to run
The plan's two status lines both predated the gate's second run. The header still read "approved to start Phase 0a" nine phases in, and the DAG note still claimed the re-run had not happened and that phase 6 waits on a passing results file. That file has existed since 2026-07-26 — screenshare-audio-exclusion-phase5-results.md records GATE PASSED on run 2 with all 13 §5.1 rows, including rows 4 and 5 at the real tagging sites. Both lines now state what actually blocks phase 6: the 0c -> 0d -> 6 edge, with 0c step 2 still open (S1/S2 merged, S3a built but unmerged, S3b/S4/S5 not started) and 0d unbuilt. The superseded 2026-07-25 note is kept for the trail rather than deleted. Docs only; no code or gate changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
mollusk
|
0e395e5c0c |
build(nix): pin the Rust toolchain to 1.97.1 via rust-overlay
CI / check (push) Canceled after 0s
nixpkgs 26.05 ships rustc 1.95.0, but this crate was developed and verified on 1.97.1 (what CachyOS had, installed 2026-07-17). Taking the compiler from oxalica/rust-overlay decouples "which Rust the project targets" from "which release the audio stack came from", so a nixpkgs bump can no longer move the compiler under the lint gate as a side effect. Chosen over rustup, which would also have worked here (nix-ld is enabled, so its prebuilt binaries run) and would have let one rust-toolchain.toml cover the packaging distroboxes too. The deciding factor is purity: rustup records nothing in flake.lock, so a fresh clone or darp5 would resolve whatever it fetched that day. rust-overlay gives the same exact-version control with the choice pinned in the lock. `.default` is the rustup "default" profile — rustc, cargo, rust-std, rustfmt and clippy — so those are no longer listed individually. rust-src and the x86_64-pc-windows-gnu target are added for win-cross-build.sh, which needs `-Z build-std=std,panic_abort`; that script still expects the peerspeak-win distrobox for the mingw half. Verified on 1.97.1: 640 lib tests pass, fmt clean. NOT fixed here, and pre-existing rather than a migration artifact: `cargo clippy --all-targets -- -D warnings` fails with 15 warnings — 13 `float_literal_f32_fallback` (bare 0.05/0.01 into `.step()`, wants `0.05_f32`), one `manual implementation of Option::filter`, one `redundant reference in format!`, all in src/app/mod.rs. The f32 lint is `future_incompatible` and is slated to become a hard error, so it needs fixing regardless of platform. CachyOS was already on 1.97.1 well before the 2026-07-31 S2 merge logged as "clippy clean", so that claim reflects a plain `cargo clippy` run, which exits 0 on warnings. `cargo clippy --fix` applies all 15 automatically. |
||
|
mollusk
|
774922c6a9 |
build(nix): add a devShell so peerspeak builds on NixOS
CI / check (push) Canceled after 0s
The repo assumed a distro with a system-wide Rust, which NixOS does not provide. This adds a flake devShell carrying the full dependency surface: - Build: rustc/cargo/clippy/rustfmt, pkg-config, clang (pipewire-sys and libspa-sys need a real libclang for bindgen, via LIBCLANG_PATH), cmake (audiopus_sys's vendored-libopus fallback), and git (build.rs stamps PEERSPEAK_GIT_SHORT from `git rev-parse`). - Link: alsa-lib, libopus, pipewire. - dlopen'd at runtime: vulkan-loader, libxkbcommon, wayland and the X11 libs. Nothing links these, so they never land in the binary's rpath and are reachable only through LD_LIBRARY_PATH. Omitting them builds fine and then fails at window creation, which is a confusing way to find out. - The supply-chain gates CI runs (cargo-deny, cargo-audit) plus cargo-deb. These were `cargo install`ed on the CachyOS side, which does not carry over — those binaries link that distro's glibc. It also carries the screen-share tools (GStreamer + plugin search path, pactl, mpv). Those look like they belong only to pixelpass, but tests/screenshare_host_fault.rs starts a REAL pixelpass host, which aborts at its own preflight without them — so they are a dependency of this test suite. They are duplicated from pixelpass's flake rather than imported: the two projects are mutually optional by design, and having one flake consume the other would reintroduce the build-level dependency that rule prevents. nixpkgs is pinned to nixos-26.05, the same channel the hosts run, so the libraries here match the running PipeWire daemon and Vulkan ICD. Verified: 640 lib tests pass, all 4 screenshare_host_fault live gates pass (real audio backend, real network bind, real pixelpass child), fmt clean. Known delta: clippy 1.95.0 (nixpkgs 26.05) flags one collapsible_match in src/widget/selectable_text.rs that clippy 1.97.1 on CachyOS did not. |
||
|
molluskandClaude Opus 5
|
63c246d976 |
docs: record the jitter buffer's unreachable shrink path as a known bug
CI / check (push) Canceled after 0s
`target_delay` grows +1 per disruption to MAX_DELAY_FRAMES (240 ms) but only shrinks after 250 consecutive clean frames — 5 s of unbroken audio. Two of the five `clean_run` resets fire on every natural pause in speech (jitter.rs:201 benign underrun, jitter.rs:181 re-prime), and the sender stops transmitting outright while the gate is closed (core/mod.rs:2041). The AIMD decrease half is therefore unreachable under conversational voice: one early jitter burst pins the extra latency for the rest of the session. Found by code review; not yet reproduced live. Pairs with field-test debt #5. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
52d842160d |
Merge s2-host-fault: the screenshare host-fault path (S2)
A pixelpass host that dies mid-share is now torn down instead of staying advertised: stdout EOF is synthesized as a terminal fault, routed back into the core on a dedicated channel behind the reliable arm of the biased select, gated by the ActiveShare generation so a reaped child's late EOF is dropped as stale, and handled by retiring the share — presence ticket removal and ScreenShareStopped ahead of the reap wait, the explanatory error after. Reviewed by Gemini (three rounds: branch review, full-range merge review, fix verification round). Its P2s — Join's early-exit ordering hole, the reap-then-presence advertising window, and the missing presence-side gate — are fixed and mutation-verified. 640 lib tests; three live gates green on the desktop, including a two-process observer gate that reads the sharer's presence from a second real node. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
e043810eb0 |
test: presence gate — a host fault clears the ticket for peers, promptly
The 🟡 S2 gap: presence-side ticket removal on fault was asserted by no gate, and the "same code path as StopScreenShare" argument turned out false — the fault handler duplicates the presence statements, so a mutant deleting them passed all 644 tests. Nothing on the sharer's own UiEvent channel can witness presence; it is only observable from another node. The gate runs a real second core as an observer in a SEPARATE PROCESS (this test binary re-invoked as `presence_probe_helper`, its own XDG_CONFIG_HOME): two in-process cores would load the same identity.key and collapse into one node id, and swapping the env var between spawns races other threads' getenv. The observer asserts the sharer's PeerState.sharing goes Some → None on fault. The fake host is a wedge — valid-shaped ticket (the OBSERVER's gossip ingest sanitizes peer tickets; a garbage one is nulled to None and the gate goes vacuous), ~1 s of life, then closes stdout while trapping SIGINT — so the reap burns the full stop grace and TIME discriminates the ordering, like the SIGINT gate: presence-first clears in ~1 s, the old reap-then-presence ordering in ~3 s, asserted < the 2 s grace. Mutation-verified both ways: presence removal deleted ⇒ observer times out; old ordering restored ⇒ 3002 ms measured, assert fires. Standalone (a plain --ignored sweep) the helper no-ops; the probe is kill_on_drop so a parent panic can't orphan it (Gemini P3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
5b80a1a010 |
core: pull the ticket off presence before the reap wait on host fault
Gemini's merge-round review (P2, CERTAIN): the fault handler ran `stop_host().await` first, and a host that merely closed stdout but lives on — trapped SIGINT, wedged — makes that call burn the full 2 s stop grace before the SIGKILL fallback. For that whole window the dead share stayed advertised: peers could still click Watch on it, and the sharer's own UI kept saying "sharing". The handler now retires the share where the fault is decided, not where the corpse is confirmed: presence ticket removal and ScreenShareStopped are emitted before the reap wait, and only the explanatory error (which carries the unconfirmed-reap caveat) waits for `stop_host`. The Stopped-before-Error contract is unchanged and still gated. StopScreenShare's identical reap-then-presence ordering predates S2 and is deliberately left alone (user-initiated stop, lower stakes); recorded as a follow-up note instead of churning reviewed main-line code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
b9803f93fb |
core: retire the share at Join's session teardown, not after ticket parse
Gemini's review of the S2 branch found the one hole the harness had not covered (P2, verified reachable): Join tears the old session down — deliberately killing the share host — BEFORE validating the ticket, and an invalid ticket exits the arm early, skipping the late `current_sharing = None`. The killed host's stdout EOF then passed the staleness gate and the user got a spurious "Screen share ended unexpectedly" on top of "invalid room ticket". Pre-S2 the stale value was toothless on this path; the fault handler gave it teeth. The share now dies where the session does: cleared unconditionally right after the teardown block, ahead of every early exit. The live gate grew a third half — share, Join with a garbage ticket, then require silence after the ticket error — and the mutant restoring the old placement is killed by exactly that assertion (spurious re-emitted ScreenShareStopped). Also Gemini's P3: the test's temp dir is now dropped by a guard, so an assertion panic no longer leaks the fake-pixelpass scripts in /tmp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
3df2378831 |
test: the owed Stop Share SIGINT gate, against the real pixelpass
0c half (ii) had never been field-run: SIGINT sent, child exits within the bound, no fallback kill on the normal path. Now it's a repeatable live gate instead of a one-off manual check: a real whole-desktop host (idle — no viewer, so no capture) is stopped and must reach ScreenShareStopped inside STOP_GRACE. The SIGKILL fallback is indistinguishable from success in the event stream, so time is the discriminator: the fallback first waits out the full 2 s grace, while a host honouring SIGINT exits in milliseconds. Also holds the SIGINTed host's late stdout EOF to the same staleness contract as the fake-host gate. Verified green on this desktop; no stray pixelpass processes after the run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
81c230a09c |
test: live S2 exit gate — dead host torn down, clean stop stays clean
Drives the real core loop through CoreController with the pixelpass override pointed at fake shell scripts (a host that emits its ticket and dies; one that lives until signalled). The command loop has no unit seam, so this is the only harness reaching the fault handler. Half 1 pins the whole death path: ScreenShareStopped arrives BEFORE the "ended unexpectedly" error. Half 2 stops a share deliberately and then requires silence while the retired host's late stdout EOF lands as a stale fault. The `exec sleep` in the living host is load-bearing: it makes SIGINT close stdout so the stale fault actually arrives, keeping the staleness assertion non-vacuous. Both core-side mutants verified killed: swallowing the forwarder's fault times out half 1; disabling the staleness gate panics half 2 with the spurious re-emitted ScreenShareStopped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Fable 5
|
be3740f5f9 |
screenshare/core: a host that dies is no longer advertised as sharing (S2)
The defect: pixelpass's stdout EOF was silently discarded, the notice channel existed only for app-audio shares, and nothing cleared the host from the teardown slot or the ticket from presence — so a crashed host stayed advertised in the room and the UI kept saying "sharing". Every share now gets a notice channel. The drain task synthesizes a terminal HostNotice::Eof when the stream ends (EOF or read error — a crash can abort across `extern "C"` before any JSON line is written, so the stream ending is the only reliable death signal). The core's forwarder turns that into a ScreenShareHostFault scoped to the spawn's generation; a stale fault (already stopped, or a newer share running) is dropped. The handler reaps the child through the existing confirmed-reap path, pulls the ticket off presence, and emits ScreenShareStopped BEFORE the error, so the UI never shows "sharing" next to the explanation. The ticket and its generation live in one ActiveShare value on purpose: they must appear and vanish together, or the staleness gate drifts. Both drain gates are mutation-verified: swallowing the Eof fails both tests; skipping it only on the read-error path fails exactly the error-path test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
0d836d14c2 |
docs: round 18 — repair moves to libpulse; two reviews, three reusable lessons
Rounds 17c and 4 of the repair review, recorded together because they resolve to one decision: `pactl`'s text output cannot carry the guarantees repair claims, so observation and unloading now go through libpulse introspection over a single verified-local connection. The dependency was taken with the user's sign-off after vetting (details beside the dep in pixelpass Cargo.toml). Three lessons that generalise beyond this phase: - A *prescription* can fail reachability just as a finding can. "Use `pactl -f json list modules`" is sound reasoning against an API that does not exist — those records carry no module index, and `unload-module` accepts only an index. - Auditing my own fixes paid a third time: two of the four fixes applied in round 17a were themselves defective, including a correlation scheme that is unsound whenever module names repeat. - The live field test caught a bug unit tests structurally cannot reach, and it was phase 0b's bug one layer down: fields drop in declaration order, the Pulse context's teardown frees IO events owned by the mainloop, and declaring the mainloop first turned a fully successful repair into SIGABRT and exit 134. Also recorded: the newline defect needed no adversary and was confirmed on the live server, and the remaining namespace hole is left open with its trade stated — an owner token would close it but would make orphans from older builds uncleanable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
76c1a13e11 |
docs: round 17 — the repair review, the 0c actor review, and 0c's slicing
Two reviews in one round. The repair planner returned changes-requested (no P1s, four reachable P2s, all applied in pixelpass `9145b2a`); the 0c actor design returned four blocking issues, all accepted, plus a concession on epoch. Recorded because three of them generalise: - A prescribed fix was not implementable as written. "Use `pactl -f json list modules`" is sound reasoning against an API that does not exist: on pactl 17 those records carry no module index, and `unload-module` takes only an index. The reachability rule now applies to prescriptions, not just findings. - The actor's bounded join would have disarmed `Drop` by moving the thread handle into `spawn_blocking` — the same defect shape as round 15's, a defence disarmed exactly when needed. - Epoch was over-specified and my vacuity instinct was right: serial equality is the entire identity guarantee, so epoch is diagnostic and explicitly not a gate. Measured on the live graph rather than argued: recorded module arguments are byte-exact with `@DEFAULT_SINK@` unresolved (both load-bearing for exact-form matching), and two sinks may share one `node.name` with capture attaching to the OLDER one in 3 of 3 trials — so a surviving wedged owner silently steals the next session's capture instead of merely risking a collision. 0c step 2 is sliced into S2–S5 so each lands reviewed. Nothing reopens D6: the connection-owned-sink design is unchanged, and a material part of the growth is pre-existing debt 0c forced into the light. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
mollusk
|
aa0515af1c |
Merge phase 0b + the peerspeak half of 0c: teardown ordering, reaping, graceful stop
Two invariants land here, both of which phase 6 depends on: 1. The echo-cancel module cannot unload while a pixelpass host is alive. The ordering-critical fields moved into `ScreenshareTeardown` with `echo_cancel` declared LAST, and killing is no longer taken for reaping — `kill_on_drop` only signals, so `ReapOnDrop` blocks on a bounded poll until the child is actually gone. The defect was real, not theoretical: `echo_cancel` sat ahead of `screenshare_host` in declaration order, so any unwind unloaded the AEC first, and unwind is reachable (no `panic=abort`, many `unwrap()`s). 2. Stop Share asks before it insists — SIGINT, a bounded grace, then SIGKILL — so pixelpass runs its own cleanup instead of leaking a null-sink module every time. SIGINT specifically: pixelpass installs only a `ctrl_c()` handler. Reviewed by Codex across two rounds: changes-requested (two blocking findings, both real, both the same shape — a defence disarmed exactly when it was needed) then approve-with-follow-ups (five P3s, all applied). The mutation matrix was revised from five to four after one pinned mutation was proved unreachable by construction, and teardown was hoisted to one unconditional post-loop site so every loop exit is covered structurally. Owed and recorded: the live Stop Share SIGINT gate has never been field-run, and the hoisted call site's live proof belongs to the phase-9 lifecycle row. 638 lib tests, clippy clean, fmt clean. |
||
|
molluskandClaude Opus 5
|
9f06741b99 |
core/teardown: an unconfirmed stop is not a clean stop
Codex's re-review of the branch returned "approve with follow-ups" — no blocking findings, five P3s. All five are applied here rather than carried as debt, since each is a few lines. The one with user-visible consequences: `stop_host` returned a bare "was sharing" bool, so the single case where the availability-first policy gives up (SIGKILL queued, reap never confirmed) still sent `ScreenShareStopped` with nothing else. The UI would say sharing had ended while pixelpass might still be alive and fanning out — a claim the user cannot see through. `shutdown` now returns `StopOutcome`, `stop_host` returns `Option<StopOutcome>`, and an unconfirmed *user-initiated* stop raises a UI error naming the stray process. Session and viewer teardown discard the outcome deliberately: nobody is waiting on an answer there, and the residual risk is already logged. Also: the three failure diagnoses in `shutdown` (the signal never left, the child ignored it, the wait itself broke) were collapsed into one log line and are now distinct — they mean different things to whoever reads the log. The second cancellation gate is the one worth keeping. The review pointed out that all cancellation coverage sat in the *graceful* wait, so a mutant that disarmed the wrapper between the two waits would survive. It was right, with a wrinkle: the naive mutant does not compile, because the child is borrowed from `self` for the whole function — the borrow checker is doing real work here. The restructured form (`self.child.take()` once cooperation has failed) does compile, and the pre-existing mid-wait test passes it. `cancelling_shutdown_after_the_kill_leaves_the_fallback_armed` kills it. Mutation-verified, both new gates: reporting an unconfirmed stop as `Reaped` fails exactly `a_failed_wait_is_not_treated_as_a_confirmed_reap`; disarming between the waits fails the new cancellation test (and the failed-wait test, which also asserts armedness) while leaving the old mid-wait test green — which is the proof the new test is not redundant. The logging split is diagnostics only and has no gate; said plainly rather than dressed up as covered. Docs: the "four mutations" line is now an explicit table naming each target and its test, with 0c's pair counted under 0c; and the aggregate teardown latency is recorded as a deferred item with a trigger (a fourth routine child, or a measured teardown over 5 s) instead of an unwritten known cost. 638 lib tests, clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
3aa768af52 |
docs: the 0b DAG row says four mutations, matching §10 round 14
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
1cfa932fbe |
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> |
||
|
molluskandClaude Opus 5
|
d8b8fd79cf |
core/teardown: stay armed across the wait, and never call a failed wait a reap
Codex's review of the two commits below returned "changes requested" with two blocking findings. Both were real. 1. `ReapOnDrop` disarmed itself across the async wait. `shutdown` moved the child out of the wrapper with `take()` before the first `.await`, so if that future was cancelled or unwound mid-wait, the raw child dropped with nothing but `kill_on_drop` (signals, does not reap) while `Drop` found `None` and did nothing — the AEC could then unload over a live child. That is precisely the hole the type exists to close, left open for the duration of every wait. The child now stays owned by `self` across every await and is released only on a *confirmed* reap. 2. A failed wait was silently converted into success, and the hard-kill path was unbounded. `wait_reaped` discarded `io::Result`, so a wait error made the timeout return `Ok` and shutdown returned as though the reap were confirmed; meanwhile a process stuck in uninterruptible sleep after SIGKILL could wedge the core command loop forever. The trait now preserves the result, both waits are bounded, and the conflict case has an explicit written policy: we choose availability, leave the child owned so the bounded Drop retry stays armed, and log the residual risk rather than hiding it. Codex also showed the test double was flattering the implementation in four ways. All four are closed: the fake can now be cancelled mid-wait, can fail its wait, and can take several polls to die, and the grace is pinned independently. That last one caught a flaw in my own gate. The elapsed-time assertion compares against `STOP_GRACE` itself, so setting the constant to zero leaves it vacuously true — both sides move together. `the_grace_is_a_real_interval` pins the constant to a band instead, and now kills that mutation directly. Mutation-verified again, five mutants, each killed by its own gate: disarming the wrapper (cancellation test), treating a wait error as success (failed-wait test, exactly one), a zero grace (the new band test), a single poll instead of the drop loop (delayed-reap test, exactly one), reversed field order (the two ordering tests). Also applies the matrix adjudication, which Codex and I reached independently: teardown moves out of the reliable close arm to ONE unconditional site after the loop, so every `break` is covered structurally — including any added later — instead of duplicating teardown across one live arm and one provably dead one. 637 lib tests, clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
92a64465a4 |
core/teardown: ask pixelpass to stop before killing it
The peerspeak half of phase 0c (design v3.4 §7.4 item 1). Stop Share and session teardown both went straight to `Child::kill()`, i.e. SIGKILL, which skips pixelpass's own cleanup and leaks one null-sink module every time. `ReapOnDrop::shutdown` now asks first: SIGINT, a bounded 2 s wait, then SIGKILL only if the child ignored the request. SIGINT specifically, not SIGTERM — pixelpass installs only a `ctrl_c()` handler, so SIGTERM would take the default disposition and be indistinguishable from SIGKILL. Signalling by pid is safe against pid reuse here: we have not reaped the child, so it is a zombie whose pid the kernel reserves until we wait it, and the pid cannot name a stranger. (Same reasoning that dismissed pixelpass bug #6.) The grace is 2 s because it is awaited inline in the core command loop, so it is also how long a wedged child can delay other commands. A healthy pixelpass never spends it. The drop/unwind path deliberately stays a hard kill: `Drop` cannot await, and there the ordering invariant (§7.1) outranks tidiness. Once the pixelpass half of 0c lands, the capture sink is connection-owned and that path stops leaking by construction. `libc` becomes a direct unix-only dependency, pinned to 0.2.186 — the version already in the tree via alsa/cpal/tokio — so Cargo.lock gains one line and no new code enters the build. Mutation-verified, five mutations, each killing its own gate: no wait (6 fail), reversed field order (2, reap test green), no reap loop (2, ordering test green), no SIGINT (6), no SIGKILL fallback (exactly 1 — the wedged-child test). 633 lib tests, clippy clean, fmt clean. Not yet field-tested: the live Stop Share gate (SIGINT sent, child exits within the bound, no fallback kill on the normal path) still owes a real run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
6ba763774d |
core/teardown: reap the screen-share children before the AEC unloads
Phase 0b, fixes 1-3 of design v3.4 §7.2 (decision D4). The invariant is that the echo-cancel module must not unload while a pixelpass host is alive and fanning out; two paths have to honour it and only one is code we get to run. The explicit path: `ActiveSession::shutdown` now awaits `ScreenshareTeardown::shutdown_children`, and the reliable command channel's close arm tears the session down explicitly instead of letting it drop on the way out of `run_core_loop`. The drop/unwind path: the ordering-critical fields move out of `ActiveSession` into `core::teardown::ScreenshareTeardown`, where `echo_cancel` is the LAST declared field and therefore the last dropped. Previously it was declared first (`:682`, ahead of `screenshare_host` at `:685`), so an unwind unloaded the AEC while the host was still live — and unwind is reachable, the core is full of `unwrap()` and has no `panic=abort` profile. Killing is not enough. `kill_on_drop(true)` only signals: it hands the child to the runtime's orphan queue and returns, which an unwinding runtime may never drain. `ReapOnDrop` blocks on a bounded 250 ms budget until the child is really gone, because a bounded stall beats unloading the AEC out from under a live pixelpass. Everything is generic over a narrow `ChildProcess` trait and over the guard type, so ordering is unit-testable without spawning processes or loading PipeWire modules — the seam idiom already used by `replace_viewer_index`. Mutation-verified, and the plan's demand that mutations 4 and 5 prove *different* defenses holds: reversing the field order fails only the AEC-ordering tests and leaves the reap test green; removing the reap loop fails only the reap tests and leaves the ordering test green. Removing the explicit wait fails the explicit-path tests. 631 lib tests, clippy clean, fmt clean. ⚠️ Mutations 1 and 2 of the pinned matrix do not both exist: the best-effort wake arm is unreachable by construction, twice over. Documented at the site; adjudication owed in the impl plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
692ad677d2 |
docs: record F11-1 closed — boundedness needs a resolved Client
Design v3.7 §6.1.1 gains the round-13 box (the rule, the ordering that is load-bearing in both directions, and why bridging deliberately still uses the full union); the phase-5 results file records the close with the measurement the deferral was waiting for; the impl plan's phase-6 gate note drops F11-1. pixelpass c78eb2d is the implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
mollusk
|
bf908adbf0 |
docs: phase 5 matrix PASSED (13/13) — design round 10, results run 2
The §5.1 dry-run audit gate passes. All 13 rows completed with a non-empty eligible half in every one, and O5 is re-measured on the fixed graph: worst recompute 67 µs across every run, 10 µs mean under deliberate churn, busy fraction 0.0006, readiness 1-2 ms with 18 binds against a 2000 ms budget. Round 10's finding, and it is the third of exactly the same shape: the pipewire-pulse PID derivation required a SINGLE repeated pipewire.sec.pid. WirePlumber repeats one too (two Clients, both sec_pid 1747), so the derivation returned None permanently on a stock desktop, key 4's suppression never fired, and every Pulse-emulated node fused into one owner. Row 1's CLEAN control forwarder and Firefox were both excluded. Fixed in pixelpass 91c4ded by deleting the heuristic: probe every distinct sec_pid and let /proc/<pid>/comm decide. Three measured rounds now, all at the observation boundary, none in the architecture -- and all three were fail-closed and silent, caught only because §5.1 requires asserting what must remain ELIGIBLE. An exclusion-only checklist would have passed every one of these builds. Rows 4-6 are closed through peerspeak's REAL tagging sites (call, mpv, notify, plus clip) with a hand-launched mpv staying eligible, so the cross-repo contract is proven end to end on live nodes. Row 10 covers the full sticky lifecycle including retirement; row 11 is provably non-vacuous (the recycled node.link-group came back byte-identical and did not inherit taint). Recorded and NOT claimed as passes: substitutions in rows 8, 9 and 13, and two reporting-only findings (the audit's sticky flag is uninformative; a bridge's named key is lost when a leg reappears under a new serial). Phase 6 remains blocked by F11-1, phases 0b/0c/0d and the Stereo Mix design call -- this file removes one gate, not all of them. |
||
|
mollusk
|
b68fca689e |
Merge phase 1: ownership tagging (SPA-JSON carriers via libspa)
peerspeak-side half of phase 1 of the screenshare audio-exclusion work: every
node peerspeak owns carries two registry-visible ownership carriers, so the
taint engine has a primary root that survives the registry's filtered global
event (design v3.5 section 6.7).
Reviewed by Codex over rounds 10-12; all findings verified and dispositioned.
Round 12's F12-1 (rfind('}') spliced carriers inside a trailing comment, a
fail-open) and F12-2 (depth ceiling taken from the consumer,
pw_properties_update_string, not from the spa-json-dump grammar) are fixed and
live-verified through the real ALSA plugin.
623 lib tests green, fmt clean, clippy clean.
|
||
|
molluskandClaude Opus 5
|
c82ef07464 |
audio/ownership: take the depth ceiling from the consumer, not the grammar
Round 12 review, finding 2 — filed as P2, and the interesting part is
that its author retracted it to P3 once we had measurements, while the
remedy it originally proposed would have been a fail-open.
The finding was that our validator rejects nesting `spa-json-dump -s`
accepts, and the suggested fix was a recursive sub-iterator walk to
match the dump tool. Both halves rest on the dump tool being the
reference. It is not. Nothing reads `PIPEWIRE_PROPS` or `PIPEWIRE_ALSA`
with `spa-json-dump`; `pw_properties_update_string` does, in the client
process.
Measured live on this host, against the real ALSA plugin:
depth 513 dump accept plugin accept ours accept
depth 514 dump accept plugin accept ours REJECT
depth 515 dump accept plugin REJECT ours reject
depth 1000 dump accept plugin REJECT ours reject
At 515 the plugin discards the whole object: the node came back as
`alsa_playback.aplay` with no properties at all. So matching the dump
tool would have made us splice carriers into values the consumer throws
away wholesale — losing both, which is the echo this feature exists to
prevent. Over-rejecting costs a routing preference; over-accepting costs
a carrier. Those are not the same price.
What was genuinely wrong is narrower: we sat exactly one level below the
consumer. `pw_properties_update_string` calls `spa_json_container_len`
on a container value, which enters one more sub-iterator before its flat
walk, and that single level is the entire discrepancy. Doing the same
puts the boundaries on the same number.
Codex reached the same three numbers independently by calling
`pw_properties_update_string_checked(NULL, ...)` directly, having
disassembled both call sites; I measured through the live plugin. Two
methods, one table.
The dump differential stays, but it is now labelled a *grammar* oracle
with a warning not to add deep values — it would fail by design. The
acceptance oracle is the new boundary test.
Mutation-verified: removing the container step fails the 514 assertion.
622 -> 623 lib tests, fmt clean, clippy clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
molluskandClaude Opus 5
|
9eab6c118d |
audio/ownership: let libspa say where the object closes
Round 12 review, finding 1 — a measured fail-open, and the third
distinct door into the same failure.
A trailing comment is valid SPA-JSON and *ends the document*
(`case __COMMENT: return 0` in spa/utils/json-core.h), so an object may
close before the last `}` in the string. `merge_pipewire_props` located
the closing brace with `rfind('}')`, which is a byte scan and not a
parse, so for
{ "target.object" = "my-sink" } # trailing }
it selected the comment's brace and spliced both ownership carriers
*into the comment*. The re-validation did not catch it, because the
result parses perfectly well — as `{ target.object = "my-sink" }`, with
neither carrier present. Confirmed against `spa-json-dump -s`.
That is an untagged node, so no taint root, so echo — exactly what
rounds 10 and 11 each closed by a different route. Latent rather than
live: pixelpass's evaluate() is still audit-only, so today it corrupts
an audit classification and becomes a leak when phase 6 consumes
eligibility.
The whole thesis of round 11 was "do not re-implement someone else's
grammar". The scanner went, but this brace hunt stayed behind in the
caller, which is the same defect wearing different clothes.
So spa_object now reports the object's own closer, taken from libspa:
closing a container at depth 0 writes the brace's position back to the
parent iterator, and spa_json_enter made `outer` that parent. Read
before the trailing check, which advances past it.
Also:
- whatever followed the object is preserved, so a user's trailing
comment survives instead of being silently deleted;
- the output check now asks whether the object closes where we put our
brace, not merely whether the string parses. A parse-only check is
what this finding defeated.
Mutation-verified: restoring `rfind` fails the new test, and dropping
the tail fails it on the deleted comment. Honest note in the code —
mutation cannot distinguish the closer comparison or the is-object
test; both are labelled belt-and-braces rather than presented as
tested.
621 -> 622 lib tests, fmt clean, clippy clean, and the ignored
spa-json-dump differential still agrees.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
molluskandClaude Opus 5
|
21ba633825 |
audio/ownership: validate inherited SPA-JSON with libspa, not a scanner
Round 11 review, findings 2, 3 and 4.
The round-10 fix replaced a brace check with a hand-written scanner. That was
the wrong shape: a second implementation of someone else's grammar drifts in
both directions at once, and measured against `spa-json-dump -s` on this host
it did.
It ACCEPTED `{ "foo" = { garbage } }` (only brackets were balanced, contents
never validated), `{ "a" = "\é" }`, `{ "a" = é }` and `{ "a" = foo\bar }`.
Merging into those put an invalid pair before our carriers, so the daemon
stops at it and drops both -- recreating the exact fail-open the round-10 fix
existed to close. Its own test even pinned `"\é"` as a valid token.
It REJECTED `{ target.object, "my-sink" }`, `{ key == "value" }` and
CR-terminated comments, all valid -- so a user with one of those in their
environment silently lost their routing policy to an overwrite. That half
affects a running Linux user.
Now libspa's own parser validates, and the merge splices into the validated
text instead of re-emitting parsed pairs. Splicing preserves the user's bytes
exactly, which also answers the review's point that re-quoting a bare key can
invent a different one (`foo\bar` -> a string with a \b escape). Three
measured properties make the splice safe -- the last `}` is the object's, a
validated object's brace is never mid-comment, and commas are pure separators
-- and the result is validated again before it is returned.
Mutation testing then deleted the rest: every pairing and recursion check I
had written turned out to be redundant, because spa_json_next already errors
on `{ garbage }` and on nested garbage, and skips containers rather than
descending. ~60 lines of my own grammar logic removed. What remains is gated
by a new differential test against `spa-json-dump -s` over a 27-value corpus
-- the check whose absence caused this round. It found a real disagreement on
its first run (a bare document, which we reject by design, not by accident).
One mutation HUNG rather than failed: dropping the `length < 0` check makes
libspa report the same error without advancing, spinning forever. Kept, now
labelled load-bearing for termination, with a token-count bound beside it.
Finding 4: the ordering test took the first textual match of `fn main`, so a
raw-string decoy above the real function satisfied it while the real one
spawned a thread first. Now requires each of the three anchors to be unique.
Mutation-verified with the review's own decoy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
molluskandClaude Opus 5
|
45b1b97dd8 |
audio/ownership: pin the no-lost-carrier invariant, and harden the byte scan
Verification round on the round-10 review fixes. Adds the property the whole of finding 3 is about, stated directly: over 20,000 deterministic inputs built from the exact characters that break SPA-JSON (braces, brackets, quotes, separators, comment marks, escapes, newlines, multi-byte characters), the merge always emits both carriers in an object it can read back. Either outcome — parse and rebuild, or overwrite — has to end that way, and now nothing can quietly change which. Also replaces two byte-index steps with character-boundary steps. Both were correct on the ASCII input they actually see, but `index + 1` after a reverse find would have split a multi-byte character and panicked the slice. scan_token gains multi-byte cases for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
ae2e9de523 |
tests/fixtures: the ownership contract says exact-match, not truthy
Round 10 review, finding 6. The cross-repo contract still documented carrier 1 as "any value other than false/0 is truthy" after R10-4 made pixelpass match it exactly. A future producer following the fixture could emit "true" and silently lose the carrier. Committed byte-identical with pixelpass's copy in the same session, as the file's own rules require. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
985c63806b |
audio/ownership: state the playlist policy, and gate main's ordering properly
Round 10 review, findings 2 and 5. Finding 2 — R10-2's rationale for tagging local playlist audio was factually wrong. It claimed a local track is "already being broadcast to peers on the same keypress", but shared listening is opt-in: music_broadcast defaults to false, play_music_index starts local playback unconditionally, and broadcast_track returns immediately when can_broadcast_music is false. So a default-config playlist is not already broadcast. The tag stays, now as an explicit policy with the real reason: the carriers reach rodio through PIPEWIRE_ALSA, which is process-wide, and clip_player and music_player are two ClipPlayer instances in one process — no value of that variable can tag one and not the other. Exempting the playlist means giving it a separately taggable stream, which is a large change for a case with a one-step workaround (play it in any other app). Tagging is not optional for received clips and peer music, which are the far end's own audio. Finding 5 — the ordering test proved only "before run_gui", which a thread::spawn inserted above the tag still satisfies while making the set_var a data race. It now requires the tag to be the first executable statement in main: attributes, `unsafe` and block punctuation are stripped, and any residue fails. Mutation-verified against a spawn, an unrelated statement, and the call deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
6fc55a286d |
audio/ownership: parse inherited SPA-JSON instead of trusting its braces
Round 10 review, finding 3. The merge's shape check was the outer braces
only, so an inherited `PIPEWIRE_ALSA='{ garbage }'` was spliced into rather
than overwritten, producing an object the daemon does not accept.
Measured live 2026-07-25, and the failure is worse than a rejection: with
PIPEWIRE_ALSA set to the old merge's output, a real aplay node came up as
node.name=alsa_playback.aplay, no peerspeak.owned, and a junk property
`garbage = "peerspeak.owned"` — the lenient parser ate our key as their
value and stopped. Both ownership carriers lost on a live
Stream/Output/Audio node, which is an echo.
So: parse the inherited object and REBUILD it with our pairs last, rather
than splicing before the closing brace. Rebuilding is what makes the result
independent of the input's formatting — a value ending in a `#` comment
would otherwise swallow everything appended after it.
The three values the new merge emits were verified against the live daemon
(user props preserved, both carriers present) and are pinned byte-for-byte.
scan_token is gated on its own postcondition: at the object level an
unterminated string is also caught by "the object never closed", so the two
implementations only disagree at the seam.
Also parameterizes the malformed-value warning, which always named
PIPEWIRE_PROPS even when PIPEWIRE_ALSA was the malformed one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
molluskandClaude Opus 5
|
d63db68318 |
audio/ownership: apply the merge rule to the ALSA carrier too
Verification round on round 10's own fixes, not on the next layer.
R10-5 preserved a user's PULSE_PROP and PIPEWIRE_PROPS but
tag_this_process_alsa_audio still clobbered their PIPEWIRE_ALSA, which is
the same kind of routing policy and deserves the same treatment. Both it
and tag_child now merge.
MEASURED, rather than assumed, because "our pairs go last so they win"
was load-bearing for the whole merge design and was never checked:
PIPEWIRE_PROPS='{ "node.name"="theirs_first", "media.role"="music",
"node.name"="ours_last" }' on pw-play
-> node.name=ours_last, media.role preserved.
The PULSE_PROP equivalent on paplay -> the same.
So last-wins holds on both grammars: a user who already sets node.name
cannot silently untag us, and their other keys survive.
That also makes tag_child's ALSA carrier merge from the inherited value
safely: in production main has already put this process's `clip` tag
there, and the child's own role now overrides it by coming last. The
existing row could not see this — the test binary never runs main, so it
only ever exercised the merge-into-nothing case. Added a row that drives
the real shape directly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
molluskandClaude Opus 5
|
e7923a1b5c |
audio/ownership: merge inherited player env vars instead of clobbering
PULSE_PROP and PIPEWIRE_PROPS can legitimately carry a user's own routing policy — media.role, a target sink — and replacing them changes where the user's audio goes as a side effect of a tagging mechanism that is supposed to be behaviourally invisible. PULSE_PROP is space-separated key=value, so merging is appending; PIPEWIRE_PROPS is a SPA-JSON object, so it is an insert before the closing brace. Our pairs go last in both, so they win a duplicate key — without that, a user with node.name already set would silently untag us. A value that does not match the expected shape is logged and overwritten: a half-merged string that fails to parse would drop the tag silently, which is worse than losing a routing preference. No full SPA-JSON parser, which would be over-engineering for a case with no live consumer (measured: neither variable is set anywhere in this user's env or config). Also sets PIPEWIRE_ALSA on the child, with the child's own role. A player configured for ALSA output is reached by neither of the other two variables, so this closes a real gap rather than only a cosmetic one — and without it such a child would inherit this process's `clip` tag from tag_this_process_alsa_audio and report the wrong role in the audit. Corrects a stale doc comment on OWNED_PROP_VALUE that still claimed pixelpass accepts any truthy value; R10-4 made the match exact. Codex's F5 was reasoned partly from a stale comment of mine, so these are worth fixing on sight. Codex phase-1 review F4. Round 10, R10-5. 8 new rows; 5 mutations verified (clobber PULSE_PROP, our pairs first, naive object concat, doubled trailing comma, drop the ALSA carrier). All 4 live ownership gates re-run green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
b5569fe2c6 |
audio: tag the fourth playback path, rodio's ClipPlayer
ClipPlayer opens a rodio default sink, which on Linux reaches the graph
through PipeWire's ALSA plugin. It was untagged through all of phase 1,
and it is a real echo path: B broadcasts music, A tunes in, A shares
their desktop, B hears their own track played back at them. Confirmed
live as `alsa_playback.peerspeak-...` with no ownership properties.
rodio exposes no way to set PipeWire node properties, so the carrier is
PIPEWIRE_ALSA, set once at the top of main while still single-threaded.
Measured, with PIPEWIRE_PROPS and PULSE_PROP unset, to establish that
setting it process-wide is safe:
- aplay (ALSA plugin) -> both carriers land. Confirms the mechanism.
- pw-play (native) -> untouched. Our own call-playback and capture
streams are native, so they keep their own
explicit tagging and are unaffected.
- arecord (ALSA capture)-> IS tagged, on a Stream/Input/Audio. Not
surgical in the role dimension; harmless only
because R10-1 honours the carriers on
producers alone. This is why R10-1 lands first.
Local playlist tracks are tagged too, not just inbound peer audio. A
local track is already broadcast to peers over the call on the same
keypress, so sharing it again through the screen share would send the far
end two copies at differing latency. That is a defect, not a feature.
Codex phase-1 review F1. Round 10, R10-2.
New live exit-gate row drives the real ClipPlayer; mutation-verified
(drop the tag -> no node within 5s). The wiring guard is mutation-
verified too, and its first version was WRONG: it searched raw source and
passed against a main with the call deleted, because the comment above it
named the function. It strips comments now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
molluskandClaude Opus 5
|
503f78153b |
audio/ownership: refuse an ambiguous contract fixture
Producer half of the same fix (Codex phase-1 review, finding 3, P2). This side collected fixture lines into a map, so a duplicated key silently took the last value while pixelpass took the first — both repos green on different contracts. Mutation-verified in both repos with a duplicated `prop_value`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
d40385f85c |
notify: correct a measured claim about the aplay fallback
The comment said aplay ignores PULSE_PROP/PIPEWIRE_PROPS. Measured: it reaches the graph through PipeWire's ALSA plugin and carries both carriers exactly like pw-play and paplay. Comment only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 5
|
bcf1343a55 |
phase 1: tag every audio node peerspeak owns, on both carriers
Zero behaviour change. This is what makes the screenshare exclusion engine able to see us at all (plan §5.1, impl plan §3): pixelpass must refuse to fan out our own playback, and until now it had no way to recognise it. Two carriers, matched by pixelpass as a union — `peerspeak.owned=1` and a `node.name` prefix `peerspeak_owned_<role>_<pid>`. Round 8 added the second after the phase-5 audit found a node property is invisible to the PipeWire registry `global` event and recoverable only by binding the node; the prefix is announced directly. A union is also the fail-closed direction: a missed tag leaks call audio into a share, a spurious one only over-excludes. Three tagging sites, all three verified live on this host: - native call playback → props on the stream dict - screenshare mpv/VLC → PULSE_PROP + PIPEWIRE_PROPS on the child - notification chimes → same, on pw-play/paplay The literals are a cross-repo wire contract, so they appear once here as named constants and are pinned in a fixture committed byte-identical in both repos (tests/fixtures/ownership-tag-contract.txt). The contract test is black-box: it builds a real child `Command` and reads back the environment it would carry, rather than testing our own formatter. Three live `#[ignore]`d exit-gate tests drive the real call sites and poll `pw-dump` for the resulting node — the plan requires the tag be shown landing on a live node, not just in the env. All three mutation-verified (drop either carrier, or the role, and the matching gate fails). Measured while verifying: mpv, VLC, pw-play and paplay all honour `node.name` from those env vars. The native stream set neither `application.name` nor a description, so a mixer fell back to `node.name` — which the tag turns into an internal identifier. Added an explicit `node.description = "PeerSpeak"` there, which keeps the plan's rule (the prefix must not reach `node.description`) while preserving its intent: mixers stay readable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
mollusk
|
6773a3882b |
docs: round 9 — uncertainty is not history (design v3.6)
Phase 3r landed §6.7 and the phase-5 audit was re-run against it immediately.
It found a second measured defect within minutes: a real hardware sink
carrying `unresolved-ancestry` permanently, from one link observed while its
output node was still unbound during enumeration. Round 8 made that
systematic rather than rare, because every node is now withheld until its
bind resolves.
New §6.8: sticky taint is a claim about history, and uncertainty is not
history. Retiring by reason code would not be enough — an unresolved node
propagates `TaintedUpstream`, which is indistinguishable from real
contamination once recorded — so the split is by provenance: the engine runs
its fixpoint twice, and only the evidence-only pass may feed sticky state.
Decisions are unchanged and still fail closed.
Also recorded in §6.8, both from Codex's round-9 review and both pre-existing:
hardware playback-to-capture paths ("Stereo Mix") defeat the `session_device`
classifier in a way the driver denylist cannot detect — a real echo path
needing a design call — and the 2 s readiness budget has no calibration
argument beyond one measurement on one idle desktop.
Impl plan: phase 3r marked built and merged with its gate results, including
the extra Device-side live gate and why row 1 alone could not cover it.
|
||
|
molluskandClaude Opus 5
|
1cd19b355f |
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> |
||
|
molluskandClaude Opus 5
|
297f4397a7 |
docs: phase 5 dry-run audit results — GATE FAILED, two findings
Impl plan §5's required results file. Phase 6 does not start. F1 (fatal): the PipeWire registry `global` event delivers only a filtered subset of node properties, and eight of the properties the phase-3 adapter reads are not among them — peerspeak.owned, pulse.module.id, node.link-group, application.process.id, node.passthrough, device.api, factory.name, alsa.driver_name (plus port.exclusive on Ports). They are silently absent, so the primary taint root never fires, the AEC identity can never validate, and session_device is universally false. Measured on PipeWire 1.6.8 / WirePlumber 0.5.15, with the full announced key set for all five object types recorded. Links and Clients are unaffected; pulse-PID derivation works. F2: with F1 in force no node has a strong owner key, so any tainted capture stream is an unbounded tainted reader and phase 2's fail-closed backstop excludes every Stream/Output/Audio on the machine. Fail-closed, so silence rather than echo — but entirely non-functional, and non-functional in a way an exclusion-only checklist would have scored as passing. The eligible half of the §5.1 partition is what caught it, exactly as the plan argued it would. The fix direction is measured and recorded: binding each Node and reading its info props recovers every missing property, which is the pattern phase 3 already built for Links. factory.id is not a shortcut — it resolves to "adapter", not api.alsa.pcm.sink. O5 is closed with ~4 orders of magnitude of headroom: 308 graph events in 6.5s under churn, every recompute under 50us (max 15us), busy fraction 0.0004. Caveat recorded — measured on the degraded graph, and the F1 fix adds per-node bind I/O this run did not measure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
283d938b79 |
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> |
||
|
molluskandClaude Opus 4.8
|
fd72e6f018 |
docs: close the AEC default-sink question raised by §6.1.0
Checked ~/.config/peerspeak/config.json: output_device and input_device are both pinned to the Arctis, so echo_cancel::enable always passes sink_master explicitly and the AEC binds to real hardware regardless of Sunshine owning the default sink. Not live for this user. Kept as a low-priority general defect: on "system default", the master args are omitted (echo_cancel.rs:89-94) and module-echo-cancel binds to whatever the default is, which on a box like this one is a null sink. Hardening would be to resolve and validate the default before load. Own task, not this feature. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
8768cd242c |
docs: v3.4 audio-exclusion — CONVERGED, ready for implementation planning
Round 7. Codex ratifies: v3.3 is ready to become the implementation plan. Seven rounds, every blocker fixed or refuted with evidence. Design approval only — nothing approved for merge, no code written. Two subtle catches from the ratification round, both applied: - The owner-key union had a wording trap that would have preserved the exact bug it was written to fix. "Resolves" must mean "yields a MATCH between the two legs", not "first property present on the node" — client.id IS present on both gst-launch legs but differs, so a first-present implementation stops at key 3, sees a mismatch, concludes "different owners" and leaks. Now specified as try-in-order-until-equal, with a dedicated test. - Sticky taint must be lifetime-aware, not keyed on raw ids. client.id, node ids, module indices, link-groups and PIDs all recycle on this stack, so a bare key would hand an unrelated future app permanent inherited taint. Stored against live owner components, cleared only when all members vanish. Also added: how pixelpass learns the pipewire-pulse PID itself (consistent pipewire.sec.pid across Pulse clients, validated against /proc/<pid>/comm), with the failure modes in both directions — safe only because unresolved ancestry is fail-closed, which is the invariant the section rests on. NEW LIVE FINDING (§6.1.0), the strongest reachability evidence yet and one Codex's sandbox could not have seen: the user's CURRENT DEFAULT SINK is sink-sunshine-stereo, a support.null-audio-sink. Every hardware sink is SUSPENDED; the only RUNNING sink is Sunshine's virtual one, with Firefox playing into it and sunshine reading its monitor. The hazardous forwarder topology is live in the default audio path full time, with no EasyEffects involved. It also means the rejected hardware-sink-only shortcut would have captured NOTHING on this machine. Flagged separately, explicitly UNVERIFIED: what module-echo-cancel binds to when the default sink is an app-owned null sink. §12 expanded with a graph-engine test surface (node-local tests cannot catch C2/C3-class defects). §14 rewritten: convergence table, agreed v1 scope, and what is deliberately out. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
da72541e18 |
docs: v3.3 audio-exclusion — owner-key union + sticky taint
Round 6. Codex disagreed with two of my three round-5 claims and was right about both; I had independently refuted one of them with a sharper test. C2 REFUTED (by my own measurement): client.id is NOT an owner bridge. One gst-launch process doing capture+playback produced TWO client objects (209 input, 210 output), no link-group, same application.process.id 20172. So client.id bridges a *connection*, not an owner, and GStreamer — the same framework pixelpass uses — splits them by default. Replaced with a conservative union, strongest first: node.link-group, owned pulse.module.id, client.id, node application.process.id, else fail closed. With a trap Codex did not flag: application.process.id is pipewire-pulse's PID for module-created streams, so bridging on it would fuse every Pulse module's legs into one owner and mass-exclude tunnel/RTP/loopback audio the user may legitimately want shared. Never bridge on that key when it equals the pipewire-pulse PID; keys 1-2 already cover those precisely. PID thus returns to the design in the CORRELATION role while remaining unusable in the IDENTITY role — and in that role a wrong answer fails closed. C3 CONCEDED: taint must be STICKY. Current-topology taint forgets buffered audio — an app that reads a tainted monitor, buffers, then closes its input leg would be relinked while still emitting peerspeak audio from the buffer, and no graph event marks the drain. Taint now persists per owner until its nodes disappear. Added §6.1.4 quantifying the arrival-side window (~10.6-21.3 ms quantum plus scheduling) and noting it is zero when taint roots already exist, which is the common case. C1 SUSTAINED with Codex's caveat: node-granular traversal is free for the monitor boundary, but over-taints Audio/Duplex nodes. Fail-closed, accepted for v1, documented as a known contradiction of the "Firefox with a mic stays shareable" promise on duplex devices. S1: endpoint props demoted to an optimization; bind-LinkInfo fallback is the correctness path. S2: readiness epoch + revalidate before each link creation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
8610ab2eb6 |
docs: v3.2 audio-exclusion — signal-graph taint, measured
Round 5. Codex and I converged independently on the same conclusion — a Link-only ancestry walk does not catch the leak — it from the crate/header/ WirePlumber sources, me from the live graph. Its sandbox could not reach the daemon (pw-dump: Operation not permitted), so the measurements are mine. Reproduced the EasyEffects topology with module-null-sink + module-loopback (same shape, no EasyEffects needed). Result: there is NO Link object between a forwarder's input leg and its output leg. Walking upstream from the leaking node over Links alone finds no inbound links at all — a dead end that reads as "clean". The legs are related only by shared node.link-group / client.id / pulse.module.id. So the signal graph needs three edge types: 1. Link edges — measured: registry Links carry all four endpoint props. 2. Sink-monitor — measured FREE at node granularity: the monitor connection IS a real Link whose output node is the sink itself. Codex held that this must be modelled explicitly; that is true only for a port-granular walk. Taint walks at node granularity, links are created per port. 3. Owner bridge — node.link-group when present, else client.id (measured shared across the forwarder's legs, distinct per app). Only modules set link-group, so client.id is what covers ordinary apps. New §6.1.1: bridge taint must be CONDITIONAL on the input leg being tainted. "Client has both legs ⇒ exclude" would exclude every app using a microphone. Firefox in a Meet call stays shareable; Firefox sharing desktop audio does not. Also: §6.5 rejects the cheap "hardware-sink-only" predicate with a measurement — the forwarder's output leg links directly to alsa_output, so the shortcut passes the leak and excludes the innocent app, backwards on both halves. §6.3 barrier corrected: core sync/done is a previous-work roundtrip, not graph quiescence. §6.4 adds crate version, endpoint fast path + bind fallback, and full-recompute cost. §5.2 correction 5 rewritten: application.process.id lives on the Node and is the app's own PID; pipewire.sec.pid lives on the Client and is pipewire-pulse's for every Pulse client. That resolves four rounds of contradictory PID claims. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
100117085d |
docs: v3.1 audio-exclusion — apply Codex round-4 findings
Round 4 (review-2026-07-21-design-v3-round4.md) returned 5 findings, 3 of them blocking. All claims re-verified against source before acceptance. Biggest correction: eligibility is a GRAPH property, not a node property. Exclusion does not propagate downstream — a filter-chain/loopback/combine-sink re-emits the mix as a fresh untagged Stream/Output/Audio that passes both the peerspeak.owned and pulse.module.id checks, re-injecting the whole call into the share. Reachability confirmed: easyeffects IS installed on this machine (it merely wasn't running during the fan-out spike, which is why the spike missed it). §6 rewritten around transitive upstream reachability, tracking Node/Port/Link globals, with a registry sync barrier and revalidation immediately before each link creation. Also applied: - §5.3 is now a bounded validation state machine, not a one-shot check. wait_for_nodes only waits for the virtual source/sink, never the playback hazard leg, and pixelpass capture spawns lazily on first viewer, so the one-shot check raced in both directions. Revocation redefined as loss of the module identity, not transient absence of one leg. - §7.2: reordering ActiveSession fields is NOT sufficient — kill_on_drop sends SIGKILL without waiting, so AEC can still unload while pixelpass lives. Fix is explicit shutdown().await at both channel-close breaks, field order as defence in depth, plus a fake-resource ordering test. - §5.1 relabelled implementation sites; none of them tag anything today. - Stop Share citation corrected to :699/:3480. - D1-D7 resolved; readiness section added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
cab6bafce5 |
docs: v3 audio-exclusion design — rewrite around Option C + AEC gate
v1/v2 described a move-based design that Option C superseded on 2026-07-20, and the AEC playback-leg identity gate has since passed. Roughly two thirds of v2 documented problems Option C does not have, so this is a rewrite rather than a patch (v1/v2 remain at |
||
|
molluskandClaude Opus 4.8
|
10203e1edb |
docs: adopt fan-out (Option C) after feasibility spike
Ran the direct-link spike on the live graph (PipeWire 1.6.8, WirePlumber 0.5.15). Fan-out carries full-level audio for paplay, mpv and VLC while the application keeps its existing speaker link; WirePlumber does not reap foreign links across default-sink switch, suspend/resume or 100s steady state; and non-lingering links are destroyed automatically when their owning connection is SIGKILLed. The decisive result is that destroying the capture sink mid-share left the application playing to its speakers undisturbed, so capture-side failure degrades to "not captured" rather than breaking the user's audio. That is the property the move-based design had to work hard to approximate. Records what the spike does not prove: fidelity beyond signal presence, daemon restart, quantum perturbation, and exclusive/passthrough streams. The capture null sink is still pactl-owned, so Stop Share continues to leak a module every time and the graceful-stop work is still owed. Eligibility becomes a broad guarded selector rather than a narrow allowlist, since copying no longer risks disturbing the source. Option A and its attendant cleanup, restore and output-switch machinery are retained for the record but are no longer the plan. A v3 rewrite is owed once the AEC playback-leg identity is settled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
88ad5a0807 |
docs: screenshare audio exclusion design v2
Rewrite after Codex's adversarial review of v1 found four release blockers, all independently verified against source. v1's premise was wrong: whole-desktop capture bypasses Routing::start entirely (pipeline.rs:121), so this needs a new capture mode rather than an inverted predicate. v2 replaces PID-based identity with ownership by inherited tag, and makes the router an allowlist so unrecognized infrastructure is left alone rather than optimistically moved. Graceful stop becomes a prerequisite: Stop Share is currently SIGKILL, so cleanup never runs on the normal path. Records live measurements taken 2026-07-20: PULSE_PROP tagging reaches the graph for paplay, mpv and VLC, and application.process.id is the client's own PID, not pipewire-pulse's — correcting a claim both the review and v1 relied on. Adds Option C (fan out a second owned link instead of moving streams), which deletes most of the cleanup, latency and multi-host problems the move-based design has to solve. Not yet implemented; gated on a feasibility spike. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
molluskandClaude Opus 4.8
|
0588d92537 |
release: 0.6.6
CI / check (push) Failing after 5m35s
The live-edge catch-up (v0.6.6 |
||
|
molluskandClaude Opus 4.8
|
b4a4c00711 |
fix(screenshare): make live-edge catch-up actually recover
CI / check (push) Failing after 2m37s
The first cut used a fixed 1.05x drain, which measurement showed was too gentle to matter: clearing a 6 s backlog would take two minutes, which a viewer experiences as still broken. Two changes, both measured on the netem satellite rig (loopback impairment, gst -> ffmpeg HTTP relay -> mpv, matching the http:// URL production actually serves): 1. Proportional drain. Speed now scales with buffer depth, 1 + 0.05*(cache - 0.5), clamped to 1.15x, keeping the hysteresis band so it cannot oscillate. Deep backlogs recover in tens of seconds; small excursions still get an inaudible nudge. 2. Bound the byte cache in Low latency. The demuxer cache is a *byte* budget, so at a given bitrate it sets the worst-case backlog: 2 MiB held ~6 s of a 2.5 Mbps share. Capping Low latency at 1 MiB halved the standing buffer, 6.0 s -> 2.8 s, on its own. Smooth keeps the user's value, since a deep buffer is that posture's whole point. Measured effect with both: playback consumes 11.6% faster than realtime while behind (ratio 1.1157 vs 0.9988 with catch-up off), i.e. ~9 s of backlog cleared in 80 s where before it recovered nothing at all and the viewer stayed behind for the rest of the call. Rig caveat: its upstream queues hold an unbounded backlog, so the cache never drops back through the low mark and the return-to-1x transition is only covered by unit tests, not the rig. 601 lib tests green, clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |