Commit Graph
424 Commits
Author SHA1 Message Date
mollusk 437c95380e feat(windows): bundle PixelPass viewer 2026-08-22 21:02:32 -04:00
mollusk f2220ed84e docs(windows): record 0.6.7 VM smoke 2026-08-22 19:36:46 -04:00
mollusk 7217015d1f fix(windows): keep firewall install idempotent 2026-08-22 19:27:10 -04:00
mollusk 6b9a72aee5 fix(windows): restore screen-share cross-build 2026-08-22 18:57:29 -04:00
mollusk f100981ad0 release: prepare PeerSpeak 0.6.7
CI / check (push) Failing after 1m58s
v0.6.7
2026-08-22 14:28:05 -04:00
mollusk c2d82acf82 docs: prepare PeerSpeak 0.6.7 release
CI / check (push) Failing after 2m46s
2026-08-22 06:10:38 -04:00
mollusk 0823f6173f ux: make safe desktop audio the default 2026-08-22 05:44:08 -04:00
mollusk ff2533c95d fix: reap viewers and clear resolved share warnings 2026-08-22 02:41:31 -04:00
mollusk 140e4e7f73 docs: record Phase 9 rig qualification 2026-08-22 00:40:23 -04:00
mollusk db8459fd03 docs: checkpoint Phase 9 correlation rig 2026-08-21 22:25:19 -04:00
mollusk 260815154f docs: close Phase 8 integration 2026-08-21 22:13:33 -04:00
mollusk 2c2b861516 feat(screenshare): integrate desktop audio exclusion 2026-08-21 22:12:48 -04:00
mollusk ba96e59db0 docs: record completed Phase 7 2026-08-21 21:56:06 -04:00
mollusk d72271bd2e docs: close Phase 6 signal qualification 2026-08-21 21:00:38 -04:00
mollusk bf6d0e47b5 docs: record completed Phase 6 matrix 2026-08-21 19:56:18 -04:00
mollusk 46dc5902d6 docs: record Phase 6 refusal gates 2026-08-21 19:45:33 -04:00
mollusk a1df62ce21 docs: record Phase 6 failure-gate checkpoint 2026-08-21 17:35:19 -04:00
mollusk 30a460420a docs: record capture sink recovery checkpoint 2026-08-21 17:01:05 -04:00
mollusk 61960eb76f docs: record audio exclusion status checkpoint 2026-08-21 16:31:29 -04:00
mollusk 3d1f114fd8 docs: record owned fanout checkpoint 2026-08-21 16:13:41 -04:00
mollusk 9e52acf9d3 docs: advance audio exclusion plan into phase 6 2026-08-21 15:40:12 -04:00
mollusk 9ba42c4cda docs: record S3b containment milestone 2026-08-15 15:31:07 -04:00
mollusk f46b2cacc7 build(appimage): harden thin bundle workflow
Keep host multimedia libraries out of the AppImage, document the exact Ubuntu build inputs, and resolve Rust 1.97 release-gate warnings.
2026-08-14 18:05:00 -04:00
mollusk d023621eee chore: enable automatic Nix development shell
CI / check (push) Successful in 2m27s
2026-08-11 11:16:24 -04:00
molluskandClaude Opus 5 59da73c013 docs: the phase-5 gate passed, so stop telling phase 6 it is blocked
CI / check (push) Canceled after 0s
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>
2026-08-07 22:15:03 -04:00
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.
2026-08-07 14:03:35 -04:00
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.
2026-08-07 13:46:18 -04:00
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>
2026-07-31 22:38:26 -04:00
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>
2026-07-31 14:53:02 -04:00
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>
2026-07-31 14:52:10 -04:00
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>
2026-07-31 14:51:59 -04:00
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>
2026-07-31 04:05:17 -04:00
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>
2026-07-31 01:10:11 -04:00
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>
2026-07-31 01:08:13 -04:00
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>
2026-07-31 01:02:41 -04:00
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>
2026-07-27 01:24:46 -04:00
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>
2026-07-26 21:33:57 -04:00
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.
2026-07-26 19:43:17 -04:00
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>
2026-07-26 19:43:04 -04:00
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>
2026-07-26 19:11:19 -04:00
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>
2026-07-26 19:11:06 -04:00
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>
2026-07-26 19:09:58 -04:00
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>
2026-07-26 18:26:05 -04:00
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>
2026-07-26 18:19:07 -04:00
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>
2026-07-26 04:46:25 -04:00
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.
2026-07-26 02:59:43 -04:00
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.
2026-07-26 02:14:25 -04:00
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>
2026-07-26 01:59:36 -04:00
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>
2026-07-26 01:51:14 -04:00
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>
2026-07-26 01:08:17 -04:00