Gemini's merge-round review (P2, CERTAIN): the fault handler ran
`stop_host().await` first, and a host that merely closed stdout but
lives on — trapped SIGINT, wedged — makes that call burn the full 2 s
stop grace before the SIGKILL fallback. For that whole window the dead
share stayed advertised: peers could still click Watch on it, and the
sharer's own UI kept saying "sharing".
The handler now retires the share where the fault is decided, not where
the corpse is confirmed: presence ticket removal and ScreenShareStopped
are emitted before the reap wait, and only the explanatory error (which
carries the unconfirmed-reap caveat) waits for `stop_host`. The
Stopped-before-Error contract is unchanged and still gated.
StopScreenShare's identical reap-then-presence ordering predates S2 and
is deliberately left alone (user-initiated stop, lower stakes); recorded
as a follow-up note instead of churning reviewed main-line code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Gemini's review of the S2 branch found the one hole the harness had not
covered (P2, verified reachable): Join tears the old session down —
deliberately killing the share host — BEFORE validating the ticket, and
an invalid ticket exits the arm early, skipping the late
`current_sharing = None`. The killed host's stdout EOF then passed the
staleness gate and the user got a spurious "Screen share ended
unexpectedly" on top of "invalid room ticket". Pre-S2 the stale value
was toothless on this path; the fault handler gave it teeth.
The share now dies where the session does: cleared unconditionally right
after the teardown block, ahead of every early exit. The live gate grew
a third half — share, Join with a garbage ticket, then require silence
after the ticket error — and the mutant restoring the old placement is
killed by exactly that assertion (spurious re-emitted ScreenShareStopped).
Also Gemini's P3: the test's temp dir is now dropped by a guard, so an
assertion panic no longer leaks the fake-pixelpass scripts in /tmp.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
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>
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>
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, 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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
mollusk
