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>
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>
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>
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.
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>
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>
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>
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>
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>
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>
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.
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.
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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.
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>
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>
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>
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>
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>
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>
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>
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>
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 88ad5a0 / 10203e1).
Folds in: the four AEC gate results, the five corrections that constrain them
(observed correlation not a contract; exact-equality only; index/link-group
reuse and node-id recycling; group prefix = hazard detection not ownership;
application.process.id == pipewire-pulse for module-created streams), the
verified implicit-drop ordering defect in ActiveSession, fail-closed
validation/revocation, the IPC shape, and the split-out prerequisites.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
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>
The live-edge catch-up (8c4f4a0, b4a4c00) landed after the v0.6.5 tag, so
the 0.6.5 artifacts do not contain it — the same gap that left the fix out
of v0.6.4. Cut 0.6.6 so the published build actually carries it.
Local-only changes (no wire change; PROTO planes unchanged), so this is a
PATCH bump per VERSIONING.md.
601 lib tests green, clippy clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
On a lossy link the reliable PixelPass transport turns every loss burst
into buffered latency that nothing trims back, so the viewer settles
seconds behind the host and stays there. Measured on a tc netem satellite
simulation: a viewer parks at a ~6 s standing buffer indefinitely.
--untimed (0.6.5) does NOT fix this and measured marginally worse (+1.38 s
vs +1.24 s): it only unpaces presentation, while audio still drains at 1x
the DAC rate, so an accumulated backlog never shrinks. Drop it.
Instead give mpv a JSON IPC socket in the Low latency posture and drive
playback slightly fast while the buffer is deep, returning to 1x once it
drains. Pitch correction keeps it inaudible and A/V sync is preserved,
because audio and video speed up together.
The control law and IPC message handling are pure functions with unit
tests; the only I/O is livesync::drive, which ends by itself when the
player exits. Smooth is deliberately excluded — its ~2 s readahead is the
point of that posture, and catch-up would fight it every poll.
Known limitation: 1.05x needs ~120 s to clear a 6 s backlog, so recovery
is slower than ideal. Tuning (a proportional law, or a seek-to-live for
large backlogs) is the follow-up.
598 lib tests green (+11), clippy clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Local-only changes since 0.6.4 (no wire change; PROTO planes unchanged),
so this is a PATCH bump per VERSIONING.md.
Ships the low-latency screen-share live-edge fix (4bfc184), which landed
three hours after the v0.6.4 tag and was therefore never released.
Also adds the missing CHANGELOG entry for the participant "Advanced audio"
foldout (26d6600), which shipped without one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the final phase of docs/chat-hardening-plan.md. Two problems: a
locally echoed message always looked sent even when the core had no active
session or the gossip broadcast failed; and a fast burst could broadcast
'successfully' yet be silently dropped by every receiver's per-author rate
bucket (8 burst, then 1/s) with no sender feedback.
Send status: CoreCommand::SendChat/SendChatFile carry a local-only id (never
on the wire); the core replies with UiEvent::ChatSendResult after the gossip
broadcast succeeds or fails, and a no-active-session is now an explicit
failure rather than a silent no-op. gossip send_chat, which previously
returned Ok on a missing sender/topic or an encode failure, now returns Err.
ChatEntry gains local_send: Option<LocalSend>; failed sends render a red
'Not sent — {reason} [Retry]' line, Broadcast/Pending render nothing
(there are no delivery receipts, so silence is the honest success state).
Sender-side pacing (new src/app/sendqueue.rs): sends past the burst queue
locally as 'queued…' and trickle out at the receivers' sustained rate, so
nothing is lost and typing is never blocked (user chose queue-and-trickle
over input throttling). The pacer reuses the gossip gate's own TokenBucket +
per-author constants (now pub(crate)) so the two sides of the policy can't
drift. A 250ms drain subscription runs only while the queue is non-empty.
Retry re-dispatches the retained payload; re-serving the same attachment id
replaces the ServeStore entry rather than double-counting bytes. The pacer
and monotonic send-id counter survive a room reset (receivers' buckets
persist; ids never alias a late result); queue and retry payloads are cleared.
582 lib tests (+11: 4 pacer/queue seam, 7 app-level transition/retry/reset);
all-targets green, clippy -D warnings clean, fmt clean, smoke launch OK. No
wire change (GOSSIP_PROTO stays 5). Tests-green-only — the two owed
two-machine field-test items are logged in the plan.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The playlist drawer duplicated the player bar's |prev/play/next| transport
row even though the drawer can only be open while the bar is visible
(drawer_open gates on show_player_bar), so the drawer copy is removed;
seek, music volume, Browse, and the tune-in checkbox remain drawer-only.
The track list (and the Public tab's broadcast list) was a 160px-fixed
scrollable nested inside a second full-height scrollable, showing only a
few entries. The outer scrollable is gone and both lists now fill the
drawer's remaining height, resizing with the window.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Also re-triggers CI: run 165 on 1d038be died to rust-lld crashes from disk
exhaustion on the runner host (12G free vs ~12G cold-build transient), not a
code failure; 18G of local build artifacts have been swept (30G free now).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 4 of docs/chat-hardening-plan.md — URL and rendering resilience.
Closes the chat-body half of S14 (bidi override strip).
- sanitize: new is_safe_web_url shared link policy (url crate, promoted to a
direct dependency): http/https scheme + non-empty host + no userinfo;
candidates failing it stay plain text (their whole whitespace run, interior
not re-scanned). Scheme detection is now ASCII-case-insensitive.
- sanitize: linkify() -> link_ranges()/segments(): validated byte ranges
computed once, exact-roundtrip slicing, at most CHAT_MSG_MAX_LINKS (8)
clickable links per message; the rest stays selectable plain text.
- sanitize_chat: strips bidi overrides/isolates (U+202A-202E, U+2066-2069)
from message bodies while keeping ZWJ/ZWNJ/LRM/RLM (S14 chat-body half).
- app: ChatEntry caches its link ranges (filled in push_chat), so redraws
slice instead of rescanning/re-validating; only link spans allocate.
- app: chat history now also bounded by 512 KiB total sanitized text
(CHAT_HISTORY_MAX_TEXT_BYTES) alongside the 300-entry cap; the attachment
byte cache is deliberately untouched by history eviction (own budgets).
- app: AppMessage::OpenUrl re-checks the same parsed policy (defence in
depth) instead of prefix checks - non-web schemes can never reach the
opener even if the handler is invoked directly.
571 lib tests green (+3 net); clippy -D warnings + fmt clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 3 of docs/chat-hardening-plan.md — attachments can no longer turn
into unbounded memory, bandwidth, decoder, or task pressure (S15 closed;
S14's filename half closed).
Cache and image cost (3A): AttachmentCache now carries encoded- and
decoded-byte budgets (96 MiB / 64 MiB) on top of the count cap, with
per-entry weights, replacement accounting, and oldest-first eviction; an
individually over-budget fetch services any pending Save/Play from the
bytes in hand and is exposed as Evicted instead of retained.
validate_image_bytes prechecks header dimensions (per-side AND a new
14 MP total-pixel limit) before any decode; the renderer only ever
receives a ≤1600 px downscaled RGBA preview whose w*h*4 cost counts
against the decoded budget — originals stay encoded-only for Save.
sanitize_filename strips the bidi/zero-width spoofing set (RTL-override
extension spoof).
Download policy and state (3B): images auto-fetch only when roster-
authored AND declared ≤4 MiB, gated by a new deterministic
AutoFetchBudget (per-author and session request+byte token buckets,
check-then-take, bounded author map) alongside the existing dedup and
four-permit bound. Attachment state is now explicit — absence/Loading/
Ready/Failed/Evicted — driven by a new AttachmentFetchStarted event, so
skipped or evicted images render a "Load image" button instead of an
indefinite "loading…", and repeated clicks can never spawn duplicate
fetch tasks.
Exact transfers and serve store (3C): fetch_blob requires the received
length to equal the declared size (short = local error, overlong =
bounded-read reject, empty keeps meaning "sender no longer has it");
the file picker's unbounded read is replaced by a metadata-prechecked
cap+1 bounded reader; one Arc<Vec<u8>> now backs the UI cache, command
queue, and serve store; served_files is a count- and byte-budgeted FIFO
ServeStore (16 entries / 128 MiB).
37 new tests (568 lib total) including a real two-endpoint loopback
exercising exact/short/overlong/unknown-id transfers. Plan checkboxes
ticked and constant deviations decision-logged. Tests-green-only: the
plan's two-machine field-test section remains open.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Chat-hardening plan Phase 2 — only current authenticated room members can
create chat UI work, impersonation via the wire name is structurally closed,
and no member can monopolize the event channel:
- core: new ChatRoster (bounded id -> sanitized-name map, shared) replaces the
event task's bare HashSet; upserted on PeerJoined/PeerUpdated, removed on
graceful PeerLeft AND terminal grace-expiry eviction (both timer paths).
Non-roster chat is dropped before attachment handling; the rendered author
label is the roster-bound name — the sender-claimed wire name is never read.
- gossip: ChatIngressGate after verify_gossip, before any sanitize work or
event send: early known-author gate (live + mid-reconnect peers), exact-
replay suppression keyed on the deterministic Ed25519 signature (1024-entry
cap + freshness-window TTL, zero new deps vs the plan's BLAKE3 option), then
per-author (8 burst, 1/s) and room-wide (32 burst, 8/s) token buckets.
Replays are detected before tokens are consumed; a room-bucket reject
refunds the author token; rejection logging is squelched per author.
- The inner Chat.ts is now ignored entirely; RoomEvent carries the signed
envelope timestamp.
550 lib tests (+18), reconnect_eviction +1 (grace keeps chat authority,
terminal eviction revokes it), clippy --all-targets -D warnings clean.
Tests-green-only: the plan's two-machine field-test section remains open.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Chat-hardening plan Phase 1. The chat body policy (2,000-char + 8 KiB
ceilings, single-pass control/whitespace normalization) moves from the UI
layer into src/sanitize.rs and is now enforced at every trust boundary:
cap_chat_input bounds the live input (oversized paste), the gossip sign
point re-sanitizes so non-UI callers can't bypass policy, and gossip
ingress rejects oversized raw text before sanitizing (admit_chat_text)
and drops messages with neither visible text nor an attachment. The
incoming chat author label now uses the strict name sanitizer until
Phase 2 roster-binds it. +8 tests (532 lib green).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The jitter buffer's gap path fed the LOWEST buffered packet to
decode_fec regardless of position. Opus in-band FEC in packet N carries
a copy of frame N-1 and nothing else, so that reconstruction is only
correct when the smallest survivor is exactly next+1 (single loss).
On burst loss it spliced a later frame's audio into the wrong slot —
worse than concealment. Gate FEC on adjacency (new fec_covers_gap(),
wraparound-aware); everything else falls back to plain PLC.
Two new tests: the gate itself, and a burst-loss test proven to bite —
it compares bit-exact against a twin decoder and fails against the old
unconditional-FEC behavior (checked by mutation).
Fixes finding 3 of the 2026-07-16 full-codebase review.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GPT-5.6's 5-phase plan for the chat identity/replay/rate-limit cluster
(2026-07-16 review findings 5-8): roster-bound display names, replay
dedup, quiet rate limiting, bidi-aware sanitization, attachment size
checks. Self-describes as temporary — delete when the work completes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The four live-rebuild sites (deferred rebuild on Join/Leave, idle
SetNetworkMode, idle RegenerateIdentity) all did
`net.shutdown().await` then `build_net_stack(...).await?` — a build
failure propagated out of run_core_loop, which its supervisor only
logs. Every subsequent command went nowhere: window alive, app dead,
user told nothing. (The initial startup build already reported.)
New replace_net_stack() helper: tear down the old stack, build for the
requested posture, and on failure fall back to the posture the old
stack was actually running (tracked in the new `net_mode` local; when
the postures are equal the fallback is a plain retry — e.g. identity
regeneration, where reverting the already-persisted key would be
wrong). If the fallback lands, the UI is told the change didn't stick
and `network_mode` reverts so state stays honest and the change stays
re-attemptable. If both builds fail the UI gets a fatal 'Networking
lost … restart' error before the loop exits — informed, not a zombie.
Retry policy isolated in rebuild_with_fallback(), generic over the
builder: 4 new unit tests cover first-try success, fall-back, plain
retry, and double failure without binding sockets.
Fixes finding 2 of the 2026-07-16 full-codebase review.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cargo-deny.yml (runs-on: ubuntu-latest) and windows-build.yml (runs-on:
windows-latest) target runner labels no registered runner advertises, so
every push queued two runs Gitea auto-cancelled ~24h later — the Actions
page has shown 2 cancelled runs per push since the runner went live.
- cargo-deny.yml: deleted; redundant with ci.yml's deny step, which now
runs `cargo deny --locked check` to preserve the locked-tree stance.
- windows-build.yml: kept but workflow_dispatch-only until a Windows
runner exists; restore instructions in the header comment.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RUSTSEC-2026-0194/0195 (quick-xml 0.39.4, published 2026-06-29) broke the
deny/audit CI gates on every push since June 29. quick-xml is reached only
via the wayland-scanner proc-macro parsing vendored protocol XML at compile
time — attacker input never touches it and it is absent from the shipped
binary. The fixed 0.41.0 is semver-incompatible with wayland-scanner's
`^0.39` req (no upstream bump yet); documented ignores until one exists.
RUSTSEC-2026-0192 (ttf-parser unmaintained, via iced/cosmic-text) joins the
existing unmaintained ignores (paste, audiopus_sys) — same class, same
lockfile-pinning protection.
New .cargo/audit.toml keeps cargo-audit in sync with deny.toml.
Known leftover warning (allowed, non-failing): spin 0.10.0 is yanked but
futures-buffered (via iroh) requires ^0.10 and no unyanked 0.10.x exists.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Six diffs across four files: the 2026-07-08 stable toolchain update
(rustc 1.96.1 / rustfmt 1.9.0) re-flags code that was fmt-clean when
committed under the previous rustfmt. No semantic change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Answer "am I actually P2P right now?" per peer. A 1 Hz session task
snapshots the selected QUIC path of every live audio connection
(IrohTransport::connection_stats), core::connstats::derive turns
consecutive snapshots into RTT/loss/bitrate (path switches and counter
resets invalidate the rate window), and the peer card shows a
Direct/Relay badge with a hover tooltip for address, loss, and up/down
bitrate. No new dependencies, no wire change.
Loopback-integration-tested against real iroh endpoints; not yet
field-verified on a 2-machine call (FEATURES.md row marked 🧪).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bump to 0.6.3 and document the screen-share work merged on this branch:
the advanced Settings section + per-call quality picker (96e41de), the
hardware-decode-defaults-off frame-1 freeze fix (96e41de), the per-call
quality override fix (e378b2e), and the VLC-honors-viewer-settings fix
(faad8ce). All local-only — no wire-protocol change, old configs load
unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The screen-share code only logged pixelpass's high-level JSON events, never
the argv it spawned children with, so a field log couldn't confirm which
encode/viewer settings actually reached the helpers — e.g. the per-call
quality's --bitrate (host) or the hardware-decode --avcodec-hw/--hwdec flag
(player). Log both verbatim at spawn: host args carry no secret, and the
player line omits the local stream URL. Logged per attempt so a player
fallback is visible too.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The viewer playback settings (hardware decode + buffering) only shaped
mpv's argv; vlc_args() was fixed, so a VLC viewer silently ignored them.
The load-bearing case is hardware decode: mpv defaults to software decode
(the A-bug fix), but VLC hardware-decodes by default, so a VLC viewer with
the default hardware_decode=false still got GPU decode and could hit the
frame-1 freeze the default exists to avoid — the toggle did nothing.
vlc_args() now takes the settings and maps the knobs that translate
cleanly to VLC: hardware decode (--avcodec-hw=none/any) and buffering
posture (network/live caching ms). The genuinely mpv-specific knobs
(cache_mb byte-cache, extra_mpv_args) stay mpv-only; the Settings UI
hints are reworded to say which knobs are mpv-only vs universal. +2 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Share control's inline quality dropdown sets a session-only
`share_quality_selection`, but ToggleScreenShare (which opens the audio
picker on the only real path to a share) unconditionally reset it back to
the saved config default before ConfirmShareScreen read it. The picker has
no quality control of its own, so the user's per-call pick was silently
dropped 100% of the time and every share used the persisted default.
Drop the reset; add a regression test asserting the override survives
picker-open and reaches the confirm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a local-only "Screen sharing" section to Settings plus a per-call quality
picker on the Share control: in-app control over how a share is encoded
(quality/bitrate/framerate/max-height/max-viewers/software-x264, + extra
pixelpass args) and how it's played back (mpv/vlc, hardware decode, buffering,
cache, + extra mpv args). Settings live in AppConfig.screen_share (all
serde-defaulted, so old configs load unchanged) and become pixelpass host CLI
flags / mpv args at share/view launch.
Hardware decode defaults OFF, which also fixes the frozen-frame-with-audio bug:
forcing --hwdec=auto stalled some viewers' HW decoder on frame 1 while audio
kept playing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Joe (X11) reported that middle-click paste did nothing in the ticket and
node-ID fields. iced's base text_input only binds Ctrl+V to the Standard
(CLIPBOARD) selection and never reads PRIMARY or binds mouse button 2, so
the "select text, middle-click to paste" workflow was dead.
Add a Button::Middle branch to ContextInput::update that reads
clipboard::Kind::Primary, sanitizes it, and pastes at the cursor (reusing
the already-tested pure paste()). Factor the control-char stripping into a
shared, unit-tested sanitize_clip() helper also used by the menu Paste, so
a trailing newline on the PRIMARY selection is dropped. Respects `locked`
so read-only display fields still reject paste.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every click of Watch (`CoreCommand::ViewShare`) spawned a fresh pixelpass
viewer + mpv and pushed it onto an untracked Vec. A field test hit the
consequence: the first click gave a frozen player (the host's capture was
stalling), so the viewer clicked again to retry — and got a SECOND mpv,
doubling the shared audio.
Track viewers paired with their share ticket. On ViewShare, reap players
whose window already closed (try_wait), then if a live player for the same
ticket exists, kill it before spawning the replacement. Re-watching a
share now swaps its player instead of stacking a second one. Pure
`replace_viewer_index` seam + test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The viewer launched mpv with `--untimed`, which displays each video
frame the instant it decodes and ignores audio timestamps. Sharing a
desktop (no audio) that just minimizes latency, but sharing a *video*
made its audio drift progressively out of sync — confirmed in a field
test watching a video together. Remove the flag so mpv paces video to
the audio clock; the remaining low-latency flags keep lag negligible for
desktop pointing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bump to 0.6.2 (Cargo + Inno .iss), promote CHANGELOG [Unreleased] -> [0.6.2].
0.6.2 rolls up the licensing (MIT + THIRD_PARTY_LICENSES) and the friends-list
liveness fixes (active offline marking, 15s refresh, manual Rescan) on the
0.6.x wire format (gossip v5, compatible with 0.6.0/0.6.1).
Adds packaging/appimage: a thin AppImage recipe (linuxdeploy) that bundles the
pixelpass screen-share helper in usr/bin so peerspeak's $PATH lookup finds it
with no code change. Assets are include_bytes!-embedded; the graphics stack and
pixelpass's gstreamer/mpv tools are left to the host. Built on Ubuntu 24.04
(glibc 2.39) for reach across Debian 13+/Fedora 40+/rolling.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex-authored refresh of docs/WINDOWS.md and packaging/windows/{README,INSTALL}.md
from the 2026-07-01 Windows session; scan.rs qualifies super::running_executables()
to drop an unused glob import. PKGBUILD pkgver reflects the last Arch build
(auto-regenerated by makepkg's pkgver()).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes the two P2 efficacy findings from the Codex audit of the W12
profiles feature.
FEC was enabled on the encoder but never used: the jitter buffer's
loss path did pure PLC, so the redundancy was wasted bitrate. Now the
gap path reconstructs the lost frame from the next buffered packet via
Opus in-band FEC (new `AudioDecoder::decode_fec`, libopus decode with
fec=true into a one-frame buffer), keeping that packet for its own
normal decode and falling back to PLC if FEC decode fails. This is the
documented libopus FEC pattern; receiver-side only, no wire change.
DTX was enabled on BadNetwork but provided no benefit — the capture
noise gate already suppresses silence transmission, and the broadcast
DTX silence packets only created seq gaps that grew the jitter cushion.
All profiles now set dtx=false (plumbing kept for a future revisit).
Adds a jitter-buffer test proving FEC reconstruction beats pure PLC
(RMS error < 0.75x) and that the FEC source packet stays buffered.
500 lib tests, clippy + fmt clean, release build clean.
Co-Authored-By: Codex (gpt-5.5) <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a small, named codec-policy picker (Low latency / Balanced / Bad
network) instead of exposing raw Opus knobs. The profile->params mapping
is a pure function (`codec::opus_impl::opus_params`) for unit testing;
profiles tune bitrate, in-band FEC, expected packet-loss, and DTX.
- config: `AudioProfile` enum (serde + Display + ALL + u8 round-trip),
persisted `audio_profile` field (default Balanced).
- codec: `OpusParams` + pure `opus_params()` + `OpusEncoder::apply_params`
/ `apply_profile`.
- core: new `SetAudioProfile` command (Reliable, no coalesce); a shared
`AtomicU8` lets the capture thread re-tune the live encoder on a
mid-call switch and read it at each new call's encoder creation.
- app: Settings "Connection quality" picker in the Audio tab, startup
config-sync send, and a one-line hint per profile.
No wire-format change (GOSSIP/audio planes untouched). 499 lib tests
green (config + codec mapping/apply tests added), clippy + fmt clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cargo-audit flagged memmap2 0.9.10 as unsound (RUSTSEC-2026-0186, unchecked
pointer offset); 0.9.11 is the patched release. Warning-level only (audit/deny
don't fail on it), but cheap to clear. Audit now down to the two deliberately
-accepted unmaintained warnings (audiopus_sys, paste; ignored in deny.toml).
Lockfile-only.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A push that only changed Cargo.lock failed to create any Actions run while the
concurrency group was present; removing it restores reliable push triggering.
Single-dev CI doesn't need run-cancellation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI's cargo-deny flagged RUSTSEC-2026-0190: unsoundness in anyhow's
Error::downcast_mut() (UB via borrow-rule violation after Error::context),
reached transitively (n0-error / iroh + the image/rav1e chain). 1.0.103 is the
patched release; lockfile-only, no API change. cargo deny check now fully clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI's fmt --check caught that Codex's hand-written additions in these two files
weren't rustfmt-formatted (the senior gate ran clippy + tests but not
fmt --check). Pure line-wrapping, no logic change. Keeps the crate fmt-clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI runs on a self-hosted host-mode gitea-runner on the desktop (label `arch`),
so the cheap gitbutter VPS only queues jobs while all compile/test compute runs
locally. Pipeline on push-to-main / PR / manual dispatch: cargo fmt --check,
clippy --all-targets -D warnings, cargo test --all-targets + doc tests, cargo
deny check, cargo audit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Track B code body for the 0.6.2 patch release. No wire change (GOSSIP_PROTO stays
5, interoperable with 0.6.0/0.6.1). Two code commits + two investigation closeouts
(A6 root-caused -> deferred to W5; A3 palette audit -> accepted as-is).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A field build is now self-identifying. `run_gui` logs `PeerSpeak v<version>
starting` (from env!("CARGO_PKG_VERSION")) on launch, and the Settings panel
shows a muted `PeerSpeak v<version>` footer — pinned to the bottom of the
220px category sidebar (wide layout) and appended under the body in the narrow
(<820px) layout. Compile-time string, no new test, no deps, local-only.
Renders the current crate version, so it tracks the Cargo.toml bump at each
release cut (shows v0.6.1 until 0.6.2 is stamped in Track A).
Codex-implemented (gpt-5.5 xhigh), senior-reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The multitrack recorder wrote every per-stem WAV frame (and the potentially
large late-joiner back-pad) inline on the caller thread while holding the
recorder mutex, so a slow/contended disk stalled the playout mixer (local
underruns) and the events loop. This is the multitrack counterpart to A17
(e0325d4), which moved the single-file recorder's writes off the mixer path.
Design: the front (MultitrackRecorder) now keeps only cheap in-memory state
(known-peer set, mic FIFO, a pending-cycle builder) and on each end_cycle
assembles ONE whole-cycle batch (new peers + mic frame + optional mix frame +
the map of peer frames written this cycle) and try_sends it over a bounded
sync_channel(256) to a dedicated writer thread. The writer thread owns every
WavWriter, is authoritative for its own cycle count, back-pads a brand-new
peer by cycles_written*frame_samples, fills absent peer/mix frames with
silence, latches the first write/create error then drains, and finalizes all
headers on channel close.
The unit of hand-off is a whole cycle, not a track: the writer appends exactly
frame_samples to every existing track per applied batch, and a full queue
DROPS the entire batch (counted + logged at 1 and every 256). So a dropped
cycle omits the same 20ms from every stem at once and all tracks stay
equal-length and sample-aligned by construction even under disk back-pressure.
On drop the batch's new-peer announcements are rolled back out of the known set
so they re-announce (and correctly re-back-pad) on the next applied cycle.
Public method signatures are unchanged -> zero core/mod.rs edits. The
WAV/file format is unchanged (no wire/on-disk change), no new deps
(std::sync::mpsc + std::thread, as A17). Writer logic is factored behind a
generic SampleWriter seam so the apply-batch alignment invariant is unit-tested
without spawning the thread; new tests cover the back-pad-on-apply invariant,
the dropped-cycle equal-length property, and async create-error surfacing at
finalize. The three existing end-to-end tests pass unchanged (now exercising
the threaded path). 496 lib tests, clippy --all-targets clean, release builds.
Codex-implemented (gpt-5.5 xhigh), senior-reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The const comment claimed "v5 (0.7.0)" while GOSSIP_PROTO has been 5 since the
v0.6.0 tag (introduced by bca2ccd, "release 0.6.0"). Git confirms the value went
straight 3 -> 5 in that one release and a GOSSIP_PROTO == 4 build never existed.
Merge the two mislabeled v4/v5 bullets into one accurate v4-v5 (0.6.0) entry and
note the 3->5 jump + that this breaking gossip change correctly rode the
0.5.1 -> 0.6.0 MINOR bump per VERSIONING.md (0.6.1 is a wire-compatible PATCH,
still proto 5). Comment-only; no wire/behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0.6.1 refinements release: A19 atomic config, S5 temp-WAV hardening, A15b slider
coalescing, A2 window-position clamp, A17 single-file recording I/O off the mixer
path, and a crate-wide cargo fmt. All wire-compatible (no *_PROTO change) with
0.6.0 peers -- no resync required.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The repo never enforced rustfmt, so formatting had drifted broadly. This is a
single mechanical `cargo fmt` pass over the whole crate (no behavioral change;
lib suite green, 493 passed). Going forward fmt should be enforced (planned CI
fmt --check step). Part of the 0.6.1 hygiene pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Recorder::write_frame ran on the playout mixer path and did a blocking write_all
to disk per 20ms frame; slow/contended storage could stall the mixer and cause
local playback underruns. Now the mixer thread only does the cheap mic-sum
(extracted as the pure mix_with_mic helper) and try_sends the frame to a
dedicated writer thread over a bounded sync_channel(256). The writer thread owns
the WavWriter, writes queued frames, records the first write error then drains
without writing, and patches the WAV size fields on channel close. A full queue
DROPS the recording frame (counted + logged at 1 and every 256) rather than
blocking call audio; a disconnected writer surfaces BrokenPipe. finalize() closes
the channel, joins the thread, and returns the first write error or the finalize
result (thread panic handled).
Scope: single-file Recorder only; WavWriter unchanged so the multitrack recorder
is untouched (its writer-thread offload is deferred as A17b). Public method
signatures preserved -> no core/mod.rs changes. New end-to-end threaded WAV
readback test + mix_with_mic helper tests; existing FIFO/mic-sum intent kept.
No new deps, no wire change. Codex-implemented (gpt-5.5 xhigh), senior-reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
initial_window_position fed saved window_x/window_y straight into
Position::Specific with no bounds check, so a saved position on a since-
disconnected monitor (or after a resolution shrink) could open the window fully
off-screen on a bare X11 WM that doesn't clamp. New pure clamp_window_position
seam: given display bounds it pulls a partly-offscreen window back inside,
centers one parked on a vanished monitor, and crucially PRESERVES legitimate
multi-monitor negative-origin coordinates (a naive clamp-to-0 would break that).
iced 0.14 has no dependency-free way to learn the virtual-desktop bounds before
the window exists, so screen_bounds() returns None for now and the clamp applies
a sanity envelope (reject |coord| > 32000 -> Centered) while preserving today's
restore behavior; the full clamp is unit-tested and ready for when bounds can be
supplied. Five clamp tests (inside, edge-clamp, disconnected, negative-origin,
None-sanity) + existing tests updated. No new deps, no wire change.
Codex-implemented (gpt-5.5 xhigh), senior-reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A fast slider drag could burst past the bounded(100) best-effort command queue
and try_send would drop commands -- possibly the FINAL value of the drag, leaving
a gain/pan/volume stuck mid-drag until the next interaction. Replace the
best-effort queue with a coalescing latest-value map keyed by control
(CoalesceKey) plus a bounded(1) wake channel: send() overwrites the latest value
per control (never drops, never blocks) and wakes the loop, which pops one
coalesced command at a time and self-re-arms while entries remain. The existing
single-command match handler is reused unchanged.
command_sender() now returns a typed CoreCommandSender that routes by
delivery_class, so the window-close Shutdown (Reliable) goes through the
unbounded reliable channel (drained biased-first) instead of the best-effort
path -- a small correctness improvement. Mute/PTT remain Reliable, untouched.
Pure seams coalesce_key/coalesce_insert/coalesce_pop with unit tests
(overwrite-same-key, distinct-peers, global control, empty pop, drain-each-once)
and a coalesce_key<->BestEffort invariant assertion. No new deps, no wire change.
Codex-implemented (gpt-5.5 xhigh), senior-reviewed; tests-green (487 lib).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cached_path materialized each embedded chime to a fixed, predictable path
(/tmp/peerspeak-<name>.wav) via fs::write, which follows symlinks -> a local
attacker on a shared host could pre-plant a symlink and redirect the write. New
write_private_wav seam writes to a randomized peerspeak-<stem>-<pid>-<counter>-
<nanos>.wav name with OpenOptions::create_new (O_EXCL, refuses to write through
an existing path) and 0600 mode at creation on Unix. Per-process cache and the
None-on-error fallback (chime simply doesn't play) are unchanged.
Unit tests: exact bytes, 0600 mode, unique paths, create_new-refuses-existing.
No new deps, no wire/schema change. Codex-implemented (gpt-5.5 xhigh), reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AppConfig::save now writes to a same-dir temp file and atomically renames
over the target (mirrors identity.rs/friends.rs), and surfaces errors via
log_msg instead of swallowing them. AppConfig::load distinguishes a missing
config (silent default, first run) from a present-but-corrupt one: the damaged
file is moved aside to config.json.corrupt.<unix_secs> before falling back to
defaults, so a later save can no longer clobber the user's real prefs.
Path-injectable seams save_to/load_from + LoadOutcome with unit tests
(round-trip, missing, corrupt-preserves-bytes, no leftover temp). No new deps,
no schema/wire change. Codex-implemented (gpt-5.5 xhigh), senior-reviewed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The .deb recipe already lives in Cargo.toml's [package.metadata.deb], but the
build *environment* (bookworm distrobox, glibc floor, the mandatory separate
CARGO_TARGET_DIR) was only captured in handoff notes. Add a packaging/debian
README so the deb path is as self-documenting as the Arch + AppImage paths.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The skip-back/forward buttons rendered orange in every theme because the
emoji glyphs ⏮/⏭ (U+23EE/U+23ED) are drawn by the system color-emoji font,
which ignores the button's text color. Replace them with |◀ / ▶| built from
the text-presentation triangles ◀/▶ (U+25C0/U+25B6) — the same family the
play button already uses — so they honor .color() and follow the active
theme like the play button does. Applies to both the now-playing player bar
and the full music drawer panel. Pure visual change; no behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
molluskandClaude Fable 5
