[wagenet]

The bug

A browser seat on the live rig kept receiving RTP but stopped producing usable video, and held its slot for 45 minutes. Every rejoining client got a 503, and #753's takeover never fired. Liveness was stamped by the session pipeline's appsink, which sits upstream of decodebin: it recorded bytes arriving, not frames decoding or reaching a consumer.

What changed

SessionActivity tracks two ActivityStamps — ingress (the session appsink: the publisher is sending) and output (a BUFFER probe on the slot's output tee, past decodebin and the converter: frames are reaching the flow's consumers). idle() returns the staler of the two, and both the watchdog and slot takeover read it, so both now catch a seat that receives without producing.

• The output stamp belongs to the slot, which outlives the sessions using it, so claiming a slot resets it.
• idle() does not judge a session until DECODE_GRACE (5 s) after its first arriving buffer. H.264 cannot decode before a keyframe brings its parameter sets; measured delay 0.9-1.0 s.
• fix(whip): let a rejoining client take over a dead session's slot #753's audio gate stays on the arriving side: one output stamp serves a slot's audio and video tees and cannot tell them apart.

Takeover. Both sessions share the slot's appsrc, and the displaced session is still tearing down when the newcomer resets the stamp. #753's session_finished gate stops it pushing new samples, but frames already inside the chain still cross the tee and move the newcomer's output stamp. idle() therefore holds the grace regardless of output, so that tail cannot let the next POST evict a newcomer that has not decoded anything yet. The cost: a session that dies within 5 s of its first buffer cannot be displaced until those 5 s are up, where before it could be after 2 s. The grace covers that tail only while it is short. Nothing flushes the slot appsrc on takeover, so the tail is bounded by its queue, APPSRC_MAX_TIME (10 s) or 20 MB, not by the grace. The queue fills only when the chain has fallen behind real time. In that case a newcomer that decodes nothing reads as live until the predecessor's backlog has drained, up to ~10 s past the grace, and is reaped after that.

Which side stopped. idle() returns the idle time together with the stamp that produced it, and both reap messages carry it. ingress frozen is a publisher that went away; ingress still moving with output frozen is a fault inside the flow that will do the same to whoever reconnects. Only the log distinguishes them. The stamps keep separate epochs and store whole milliseconds, so a publisher drop freezes both within a millisecond in either order — STALL_SIDE_MARGIN (100 ms) keeps that noise from reading as a stuck consumer. It labels only: the duration is the staler stamp whichever side is named, so the reap decision is unchanged.

Tests

Each new test fails with its fix reverted: ingress-only idle(), the tee probe deleted, the conditional grace restored, the stall side hardcoded, STALL_SIDE_MARGIN dropped, or the Ingress label returning ingress_idle instead of the staler stamp. The last three were run on this head; the first three when written.

• whip_session_manager.rs: a_session_receiving_media_it_never_decodes_is_displaced, a_session_whose_output_has_stalled_is_displaced, a_session_still_waiting_for_its_first_decoded_frame_is_not_displaced, a_new_session_does_not_inherit_the_slots_previous_liveness, a_predecessors_trailing_frames_do_not_cut_the_decode_grace_short, and a_reaped_session_names_which_side_of_the_chain_stopped (both directions plus the near-tie the margin exists for).
• whip.rs: the_slot_stamp_follows_media_out_of_the_decode_chain_not_into_it pushes real buffers through the block's own appsrc -> decodebin -> videoconvert -> tee chain, blocks the consumer, and asserts the stamp stops moving — plugins-base elements only, which CI installs. watchdog_reaps_a_session_that_receives_media_it_never_decodes runs under a 5 s bound so a regression fails rather than hangs, and now also asserts the reap is reported against the slot's output.
• watchdog_detects_inactivity_within_one_poll_of_the_timeout (fix(whip): evaluate the inactivity watchdog per poll tick, not per timeout #752) backdates its session and asserts the publisher-side label.
• fix(whip): let a rejoining client take over a dead session's slot #753's guards pass unchanged, including a_finished_session_stops_feeding_its_slot and a_dead_session_is_displaced_past_a_quieter_video_only_one.

Ran locally on macOS at this head: cargo test --features efp — 679 lib tests plus all integration suites passed, 0 failed, 2 pre-existing ignores — with clippy --all-targets -D warnings and fmt --check clean. Not run: the nvidia feature, the WASM clippy.

Rig verification (pre-rebase)

Six trials on the pre-#753 base, not repeated since. The commits after them only lengthen protection for sessions younger than 5 s and change reap wording, and every trial was judged well past 5 s.

Headless, max_sessions: 1, a whipclientsink H.264 publisher feeding a flow that ends in an MP4 recorder. Stopping the publisher's audio makes the muxer back-pressure video through the slot's tee while RTP keeps arriving.

• Stalled seat (3 trials): displaced 2027-2049 ms after its output froze, publisher still sending. The watchdog reaped nothing.
• Healthy seat (3 trials): 75 rejoin attempts over 32 s, all refused within 101-107 ms (one poll), none displaced.

Trade-offs

• A stuck consumer costs a seat repeatedly, not once. If something below the slot's tee blocks, output freezes while ingress keeps moving — the slot appsrc is leaky-type=downstream, so the stall never back-pressures the publisher. The seat is reaped, the publisher reconnects onto the same blocked chain, and is reaped again: every ~7 s (DECODE_GRACE + TAKEOVER_IDLE_THRESHOLD) with a client waiting, or ~15 s (DECODE_GRACE + INACTIVITY_TIMEOUT) from the watchdog alone. The stall-side label makes it visible in the log; fixing the stall is out of scope.
• Grace margin. DECODE_GRACE has no hard bound behind it beyond the ~1 s measurement. It must stay below INACTIVITY_TIMEOUT, and a const assertion beside the timeout makes crossing them a build error.
• Per-slot stamp. Output is stamped per slot, not per medium, so frozen video with live audio still reads as live. feat(whip): track a slot's liveness per medium #792 addresses that on top of this.
• File size. whip_session_manager.rs and whip.rs are both past the split threshold. A pure-move split, stacked on this PR, is in progress and lands before feat(whip): track a slot's liveness per medium #792.

🤖 Generated with Claude Code

[wagenet]

Note for whoever merges this alongside #752.

#752 replaces the watchdog's sleep-then-check with wait_for_inactivity, which polls
idle every WATCHDOG_POLL tick. This PR rewrites the same loop to read
SessionActivity::idle(). Both are wanted, and the resolution is not "keep one":
keep #752's per-tick polling and change wait_for_inactivity to take
&SessionActivity instead of (epoch, last_buffer_ms, ...), so session idleness has a
single definition shared with slot takeover rather than two that can drift.

Done that way on my fork's main; its two tests carry over with the signature. Happy to
fold it into this PR once #752 lands.

[srperens]

Noted as a draft, so I am holding off on a full review until it is marked ready for
review. If you want one before then, say so here or request me as a reviewer.

[srperens]

This branch is based on the pre-review #753, and I think it needs a rebase before it can be reviewed on its own merits.

The base

wagenet/whip-liveness-decoded sits on 0bcf6dc. #753's head is 20007d5, one commit further on — the commit that closed both of the blocking items in the review of 2026-09-14. That commit is reachable from neither this branch nor #792 nor #805:

$ git log --oneline origin/main..pr756
a81cd15 fix(whip): judge a session by the media it produces, not what arrives
0bcf6dc fix(whip): only displace a session that carries audio     <- #753 stops here
f5f48d3 fix(whip): let a rejoining client take over a dead session's slot

So four things from #753 are missing above it:

Missing	What it was
forward_sample_to_slot + the session_finished gate	stops two sessions feeding one slot appsrc with different ts_offsets
!has_audio -> return None, plus has_audio in the max_by_key sort key	refuses an undisplaceable POST at once instead of holding it for 3 s
a_finished_session_stops_feeding_its_slot	the test for the gate
a_dead_session_is_displaced_past_a_quieter_video_only_one	the two-slot test for the sort key

git grep -c 'session_finished.load' returns 1 on #753 and 0 on this branch, #792 and #805.

It is not only a textual conflict

A test merge of this branch onto main + #753 conflicts in both backend/src/blocks/builtin/whip.rs and backend/src/whip_session_manager.rs. That alone is resolvable, but the two changes interact, and resolving the conflict is not the same as getting the behaviour right.

This PR makes the incoming session clear the slot's stamp:

pub fn new(epoch: Instant, output: Arc<ActivityStamp>) -> Self {
    output.reset();

with the reasoning that a newcomer must not be judged on its predecessor's frames. The commit message puts the residual window at "buffers left in flight ... only ever makes the new session look healthier than it is", which is true of a clean leave.

Takeover is not a clean leave. allocate_slot_or_take_over sends a SessionCleanupRequest and returns as soon as the slot frees; the displaced session's pipeline is still coming down while whip_post goes on to build the new one and calls reset(). Both sessions hold the same appsrc — config.slot_video_appsrcs.get(slot).cloned(), built once at flow build time and shared by every session that passes through the slot. Without the gate from 20007d5, the displaced session keeps pushing into it, those buffers cross the output tee, and they stamp the new session's output stamp. That is the inheritance reset() exists to prevent, arriving by a different route.

On #753's current head the gate closes it: the displaced session's cleanup_sent is won by the takeover path before the slot is released, and forward_sample_to_slot returns before push_sample from then on. So the rebase is what makes this PR's own mechanism hold, not just housekeeping.

I have read this rather than run it, so treat the window's width as unmeasured — but the shared appsrc and the missing gate are both checkable in the source.

What I would suggest

Rebase onto 20007d5 and re-push. Same for #792 and #805, which inherit the same base (and additionally carry #786 and #796, so those two need to land first regardless).

For the record, the rest of the review is not affected by any of this. The BUFFER probe added here stays inside the per-buffer budget — one Instant read and relaxed atomics, no lock, no allocation, no formatting — and its closure captures only an Arc<ActivityStamp>, no GStreamer object, so it adds no reference cycle. DECODE_GRACE sitting under INACTIVITY_TIMEOUT checks out. I will review the mechanism properly once it is on the right base.

One thing worth deciding separately: whip_session_manager.rs goes 668 -> 1495 lines on this branch and 1894 by #805, and whip.rs reaches 2686. That is past the point where the repo guidance asks for a split, and the split is easier before #792 than after.

[srperens]

Correction to the rebase target in my comment above: it said "rebase onto 20007d5", which was right when I wrote it and is not any more.

#753 has since been squash-merged to main as 6a22a56, so 20007d5 no longer exists as a base worth targeting — rebasing onto it would carry the same changes a second time against a main that already has them. Rebase onto main. Everything else in that comment stands: the gate and the two tests are on main now, so a rebase picks them up.

#758 landed just before it (2e1abe8), and #754 before that. Nothing else in the comment changes.

[srperens]

Verdict: Comment — right mechanism, real tests that genuinely executed. One residual: judging a seat by its slot's output makes a stuck consumer cost a healthy publisher its seat on a ~15 s cycle, filed in the body as "costs a reconnect, not media".

Requested changes

1. Name the fault direction when a seat is reaped. Both reasons read "without usable media" whichever stamp froze — backend/src/whip_session_manager.rs:770 — "displaced by a new client after {} ms without usable media",. Where ingress is still moving and output is not, say so there and in the watchdog's info!: it is the only signal an operator gets that the flow's consumer, not the publisher, is the suspect.
2. Restate the "Flow-wide stalls" trade-off — one reconnect per ~15 s while the consumer is stuck, not one reconnect.

Claims

Claim	Verdict	Evidence
The tee probe respects CLAUDE.md's BUFFER-probe rules	CONFIRMED	backend/src/blocks/builtin/whip.rs:311 — stamp.touch(); is the whole callback: one clock read, relaxed atomics, no lock or allocation. It captures an Arc<ActivityStamp>, no Element/Bin/Pipeline, so no reference cycle
The new tests run in CI on stock packages	CONFIRMED	Check (Linux) job 105241140495 log: all seven named tests ran and passed, incl. the_slot_stamp_follows_media_out_of_the_decode_chain_not_into_it ... ok. No skip guard
A flow-wide stall costs "a reconnect, not media"	CONTRADICTED	The slot appsrc leaks rather than back-pressuring the session — backend/src/blocks/builtin/whip.rs:444 — .leaky_type(gst_app::AppLeakyType::Downstream) — so ingress keeps stamping while output freezes, idle() takes the max at whip_session_manager.rs:399 — Some(ingress_idle.max(output_idle)) — and the reconnecting publisher lands on the same blocked chain and is reaped again after DECODE_GRACE + INACTIVITY_TIMEOUT
Rig trials	EXTERNAL	Not settleable here; assumed as stated

I did not run the reverts behind "each new test fails with its fix reverted". Traced statically, two hold: receiving_but_never_usable and receiving_but_stalled tick ingress throughout, so an ingress-only idle() returns ~0 and both displacement assertions fail.

Diagnosis — Correct layer: the appsink is upstream of decodebin in the isolated session pipeline and can only see bytes arriving; the tee sink pad is the first point past the whole slot chain. Coverage is BOUNDED — one stamp serves a slot's audio and video tees, so frozen video beside live audio still reads live, correctly deferred to #792.

The grace holds unconditionally rather than only when output is frozen — whip_session_manager.rs:389 — let past_grace = self.ingress.since_first()?.checked_sub(DECODE_GRACE)?; — which stops a predecessor's in-flight frames starting the clock on a newcomer. That is the subtle part and it is right. DECODE_GRACE (5 s) and INACTIVITY_TIMEOUT (10 s) sit in different modules, nothing enforcing the ordering.

Radius — SHARED: two files, but it changes session lifetime policy for every WHIP Input seat, in both decode modes — the probe sits outside the if decode branch, so a non-decoding endpoint stamps straight off its appsrc and behaves as before.

Overlaps — #792 and #805 both contain this PR's commits and are stacked on it, so this is the bottom of the stack and lands first; neither is left for this merge to settle. No open issue owns the stalled-consumer question.

Tests & CI — Green at cb10dc83. macOS/Windows skipping, but no platform code here. Canary pipeline_lifecycle_test.rs ran, 3 passed — it matters because this installs a long-lived pad probe.

Confidence: HIGH

[wagenet]

Rebased onto main (now 45a4c1cc, 0.6.10) — clean, no conflicts. Both requested changes are in 777a6bc, and your CONTRADICTED claim was right.

1. Fault direction in the reap message. SessionActivity::idle_detail() returns the idle time together with a StallSide naming the stamp that froze. Both sites carry it: the displacement warn! in whip_session_manager.rs and the watchdog's info! in whip.rs, plus the SessionCleanupRequest reason both send.

Writing the test for it turned up something I had not expected: the two stamps keep separate epochs and store whole milliseconds, so a publisher drop — which freezes both at once — reads either way round depending on sub-millisecond truncation. A bare output_idle > ingress_idle mislabels roughly half of those as a stuck consumer. STALL_SIDE_MARGIN (100 ms) sits two orders of magnitude above that noise and two below the seconds of separation the real output-side fault produces. It affects the label only; the duration returned and the reap decision are untouched. a_reaped_session_names_which_side_of_the_chain_stopped covers both directions and the near-tie, and fails both with the margin removed and with the side hardcoded — I ran both reverts.

2. The flow-wide stall trade-off. Restated in the body, and it is worse than "per ~15 s": the slot appsrc is leaky-type=downstream (whip.rs:444), so a blocked consumer never back-pressures the publisher, ingress keeps stamping, and the reconnecting publisher lands on the same blocked chain. The cycle is DECODE_GRACE + TAKEOVER_IDLE_THRESHOLD (~7 s) when a client is waiting, or DECODE_GRACE + INACTIVITY_TIMEOUT (~15 s) from the watchdog alone. "Costs a reconnect, not media" was wrong and is gone from the body. The stall-side label is what makes the difference visible in the log; fixing the stall itself is out of scope here.

On DECODE_GRACE and INACTIVITY_TIMEOUT living in different modules with nothing enforcing the ordering — agreed, and still true. It is called out in the body's trade-offs rather than fixed, since a compile-time assertion would mean moving one of the two constants and I did not want that in this PR.

Re-ran on this head: cargo test --features efp green (679 lib tests plus all integration suites, 0 failed, 2 pre-existing ignores), clippy --all-targets -D warnings and fmt --check clean. Not run: the nvidia feature, the WASM clippy.

— Claude (via @wagenet)

[srperens]

Verdict: Request changes — both requested changes from my review at cb10dc8 are in, and CI at this head is green with all nine named tests actually executed. One new claim is contradicted: STALL_SIDE_MARGIN does not label only, it changes the idle duration idle() returns. This supersedes that review; it is COMMENTED and cannot be dismissed, so this is the current verdict.

Requested changes

1. Make the label label-only. backend/src/whip_session_manager.rs:440 — Some((ingress_idle, StallSide::Ingress)) — returns ingress_idle where the pre-margin code returned the max. Return ingress_idle.max(output_idle) there and the body's sentence becomes true; no existing test asserts a duration inside that window, the three idle_detail assertions read only the side (backend/src/whip_session_manager.rs:1595). The alternative is to correct the sentence and state the bound.

Claims

Claim	Verdict	Evidence
STALL_SIDE_MARGIN "labels only; the idle duration and the reap decision are unchanged"	CONTRADICTED	backend/src/whip_session_manager.rs:437 — if output_idle > ingress_idle + STALL_SIDE_MARGIN { — so whenever ingress_idle <= output_idle <= ingress_idle + margin, line 440 returns ingress_idle and not the max. Bounded by backend/src/whip_session_manager.rs:532 — const STALL_SIDE_MARGIN: Duration = Duration::from_millis(100); — one whole backend/src/whip_session_manager.rs:553 — const TAKEOVER_POLL: Duration = Duration::from_millis(100); — so a takeover can be deferred a poll
The margin test fails with STALL_SIDE_MARGIN dropped	CONFIRMED	Traced, not run: the near-tie case backdates output 1 ms staler than ingress, so at a zero margin line 437 takes the Output branch and the StallSide::Ingress assertion fails
The new tests run in CI on stock packages	CONFIRMED	Check (Linux) job 106837320851 log: all nine named tests ... ok, no skip guard, including a_reaped_session_names_which_side_of_the_chain_stopped and the_slot_stamp_follows_media_out_of_the_decode_chain_not_into_it
Rig trials (pre-rebase)	EXTERNAL	Not settleable here; assumed as stated, and the commits since only lengthen protection under 5 s and change reap wording

Diagnosis — Unchanged from cb10dc8. The probe callback is byte-identical at this head (backend/src/blocks/builtin/whip.rs:311 — stamp.touch();), so the BUFFER-probe row stands CONFIRMED in that review rather than being rebuilt. My earlier CONTRADICTED row, "costs a reconnect, not media", is closed: the body now states the ~7 s / ~15 s repeat cycle.

One cleanup, not blocking: backend/src/whip_session_manager.rs:415 — pub fn idle(&self) -> Option<Duration> { — has no production caller left, every live path goes through idle_detail (backend/src/whip_session_manager.rs:862). It is pub, so nothing warns.

Radius — SHARED, as argued at cb10dc8: two files, but session lifetime policy for every WHIP Input seat. The margin narrows that to a bounded timing shift.

Overlaps — #792 and #805 both carry this PR's commits, so this is the bottom of the stack and lands first; neither is left for this merge to settle. #793 also edits whip_session_manager.rs, but adds an independent accessor and owns no design here. No open issue's Ask is answered by merging this.

Tests & CI — Green at 777a6bc; macOS/Windows skipping, no platform code here. Canary pipeline_lifecycle_test.rs ran, 3 passed. Both files are past CLAUDE.md's 1500-line threshold (1705 and 2713); the body commits to splitting them separately before #792, which is the right order.

Confidence: HIGH

[wagenet]

Both points are addressed in d322652.

Label only, now for real. Both branches return ingress_idle.max(output_idle), and the margin only picks the StallSide. The near-tie case in a_reaped_session_names_which_side_of_the_chain_stopped now puts output STALL_SIDE_MARGIN / 2 staler than ingress, not 1 ms, and asserts the returned duration is at least output's. I restored the old Ingress branch and the test fails with 30.000012666s < 30.050012666s. A 1 ms offset could not have caught this, because the truncation noise it models is the same size as the error. The side assertion still fails with the margin dropped: 50 ms > 0 flips it to Output. I checked that by reading the code, not by running it this time.

idle() cleanup. idle() and idle_detail() are merged. The function is now idle() -> Option<(Duration, StallSide)>, which keeps the existing SessionActivity::idle doc links pointing at what the reap paths call. #792 and #805 will need the rename when they rebase.

Re-ran on this head: cargo test --features efp passed, 883 tests with 0 failed and 2 pre-existing ignores. Clippy --all-targets -D warnings and fmt --check are clean. Not run: the nvidia feature and the WASM clippy.

— Claude (via @wagenet)

[srperens]

Verdict: Approve — the one CONTRADICTED row from my review at 777a6bc is closed. idle() now returns the staler stamp whichever side it names, so the body's "it labels only" is true and the reap decision is back to what it was before the margin landed. This supersedes that review; its Request changes verdict no longer stands and I have dismissed it.

Claims

Claim	Verdict	Evidence
STALL_SIDE_MARGIN moves the label only, never the idle duration	CONFIRMED	backend/src/whip_session_manager.rs:437 — Some((ingress_idle.max(output_idle), side)) — the side is chosen separately and the duration is unconditionally the max. Both consumers take the duration from that tuple: backend/src/whip_session_manager.rs:804 — ` .filter(
The new assertion fails with that fix reverted	CONFIRMED	Traced, not run: backend/src/whip_session_manager.rs:1587 — RUNNING_FOR / 2 + STALL_SIDE_MARGIN / 2, puts output 50 ms staler, inside the 100 ms margin, so the side stays Ingress; backend/src/whip_session_manager.rs:1601 — idle >= output_idle, then fails by ~50 ms against the reverted ingress_idle
The nine named tests ran at this head	CONFIRMED	Check (Linux) job 107188638644: all nine ... ok, no skip guard, including a_reaped_session_names_which_side_of_the_chain_stopped and the_slot_stamp_follows_media_out_of_the_decode_chain_not_into_it
Rig trials (pre-rebase)	EXTERNAL	Not settleable here; assumed as stated, and this commit only widens the duration back to the max

Diagnosis — Unchanged from cb10dc8 and 777a6bc; the probe callback and the grace logic are untouched by this commit, so those rows stand CONFIRMED in those reviews rather than being rebuilt. The commit also folds idle_detail back into idle (backend/src/whip_session_manager.rs:416 — pub fn idle(&self) -> Option<(Duration, StallSide)> {), which closes the dead pub fn I flagged as non-blocking; grep finds no remaining caller of the old name, and the only other .idle() in the tree is an unrelated method in backend/src/mcp/session.rs.

Radius — SHARED, as argued at cb10dc8: two files, but session lifetime policy for every WHIP Input seat. The bounded timing shift the margin introduced is now gone, so the takeover ordering key is the pre-margin one (backend/src/whip_session_manager.rs:865 — .max_by_key(|c| (c.dying, c.has_audio, c.idle.map(|(idle, _)| idle)))).

Overlaps — None left for this merge to settle. #854 is the promised pure-move split and is stacked directly on d322652, so it lands after. No open issue's Ask touches WHIP Input session liveness; #839 is WHIP Output.

Not blocking, for the stack owner: #792 and #805 carry this PR's base commit but not d322652, so both need a rebase onto this head before they are read against it.

Tests & CI — Green at d322652, nine jobs, macOS/Windows skipping and no platform code here. Canary pipeline_lifecycle_test.rs ran, 3 passed in 0.94s. Both files remain past CLAUDE.md's 1500-line threshold (1710 and 2713); #854 is the split, in the order the body committed to.

Confidence: HIGH

Superseded by my review at d322652 under the same protocol generation: the requested change landed and the verdict is now Approve.

[srperens]

Verdict: Comment — supersedes my dismissed review at d322652; the verdict does not stand, because the head is a different six-commit series. The mechanism is right and the BUFFER probe is written as CLAUDE.md requires. Two things hold approval back: an unmeasured teardown window the body names itself, and a constant relationship nothing enforces.

Requested changes

1. Bind the two timeouts that must not cross. backend/src/whip_session_manager.rs:530 — const DECODE_GRACE: Duration = Duration::from_secs(5); — must stay under backend/src/blocks/builtin/whip.rs:713 — const INACTIVITY_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);, and the body says nothing enforces it. Make DECODE_GRACE pub(crate) and put a const _: () = assert!(...) beside INACTIVITY_TIMEOUT: one line, and a comment becomes a build error.

Claims

Claim	Verdict	Evidence
The BUFFER probe callback is lock-, allocation- and format-free	CONFIRMED	backend/src/blocks/builtin/whip.rs:309 — ` gst::PadProbeType::BUFFER
The grace is held unconditionally, so a predecessor's draining frames cannot expose a newcomer	CONFIRMED	backend/src/whip_session_manager.rs:418 — let past_grace = self.ingress.since_first()?.checked_sub(DECODE_GRACE)?; — keyed on the per-session ingress stamp, and idle() returns None before it elapses whatever output shows
The displaced session's drain tail is shorter than DECODE_GRACE	UNVERIFIED	Stated unmeasured in the body. It is this repo's own teardown window, so it is settleable here — the consequence if it is ever longer is bounded and mild (a newcomer that decodes nothing looks alive until the tail ends), but it is the assumption the takeover correctness rests on
Post-rig commits only lengthen protection and change wording	CONFIRMED	backend/src/whip_session_manager.rs:437 — Some((ingress_idle.max(output_idle), side)) — the label is chosen separately from the duration, which is the staler stamp either way, so reap timing is unchanged by acf9f42
The nine named tests executed rather than skipped	CONFIRMED	Check (Linux) job 107891171430 at 449fd201: all nine ... ok, and the pipeline_lifecycle_test canary "3 passed"

Diagnosis — Right layer: liveness moves from where bytes arrive to where frames leave the decode chain, which is what the 45-minute held seat needed. Coverage is BOUNDED per slot, not per medium, so frozen video beside live audio still reads as live; #792 owns that.

The new repeated eviction is worth stating plainly. It is CLOSED — it stops as soon as output moves — and costs nothing on the healthy path, because it only fires where output is genuinely frozen. But a blocked consumer now churns the seat every ~7 s instead of holding it for ever. That is the intended trade and the stall-side label makes it legible, so it wants a maintainer's nod, not a change.

Radius — SHARED: every WHIP Input flow's session lifetime decisions move, and takeover and the watchdog both read the new idle(). The collector.rs edit is test-fixture only.

Overlaps — #792 (draft) owns the per-medium refinement and is stacked on this; #854 (draft) is the pure-move split of these two files, sequenced after this. Neither leaves a design question for this merge to settle.

Tests & CI — Nine checks green at 449fd20; macOS/Windows skipped, nothing here is platform-specific.

Confidence: HIGH

[wagenet]

Both addressed.

Timeouts bound. 51b2e88 makes DECODE_GRACE pub(crate) and puts const _: () = assert!(DECODE_GRACE.as_millis() < INACTIVITY_TIMEOUT.as_millis()); beside INACTIVITY_TIMEOUT in whip.rs. It compares as_millis() because Duration's PartialOrd is not const. With DECODE_GRACE temporarily set to 10 s the build fails with E0080: evaluation panicked: assertion failed at that line.

Drain tail. Settled by reading the code, and the claim was wrong as stated: the tail is not guaranteed shorter than the grace. Takeover never flushes the slot appsrc, so whatever sits in its queue when session_finished closes the gate still drains through decodebin and stamps the newcomer's output. That queue is capped at APPSRC_MAX_TIME (10 s) or APPSRC_MAX_BYTES_VIDEO (20 MB) (backend/src/blocks/mod.rs:19-23), not by DECODE_GRACE. It is near empty while consumers keep up with real time, and fills only when the chain was already behind. The worst case is the one you described: a newcomer that decodes nothing reads as live for up to ~10 s past the grace, then is reaped. I've left the behaviour alone. Flushing the appsrc on takeover would close the gap, but it is a new behaviour change with its own teardown risk, for a mild and bounded effect. 47c41fd states the bound in the idle() doc comment, and the body's "I have not measured the tail's width" sentence is replaced with it.

Ran at 47c41fd: cargo test --features efp --lib whip (37 passed), clippy --all-targets -D warnings and fmt --check clean. Not re-run: the full suite, the nvidia feature, the WASM clippy. The two commits change a doc comment, a const item's visibility, and add the compile-time check.

— Claude (via @wagenet)

[srperens]

This review supersedes my review of 2026-09-25 at 449fd20, and the verdict moves from Comment to Approve. Since then the branch gained two commits (ahead_by=2, 12 lines in total), and they settle both points that were holding the verdict.

Verdict: Approve. Every row is now CONFIRMED. Of the earlier concerns, the constant relationship is enforced at compile time, and the drain-tail assumption was resolved in the author's reply, in the direction the old row feared. The author then bounded it and documented it instead of leaving it assumed.

Claims

Claim	Verdict	Evidence
DECODE_GRACE can no longer reach the watchdog timeout	CONFIRMED	backend/src/blocks/builtin/whip.rs:718 — const _: () = assert!(DECODE_GRACE.as_millis() < INACTIVITY_TIMEOUT.as_millis());. This is exactly what I asked for, and the build breaks if it is violated
A predecessor's drain tail can outlast the grace, bounded by the slot appsrc's queue	CONFIRMED	backend/src/whip_session_manager.rs:413 — ` /// `APPSRC_MAX_TIME` of the predecessor's media, and a newcomer that decodes`. The slot appsrc is built with backend/src/blocks/builtin/whip.rs:444 — .max_time(APPSRC_MAX_TIME), and that constant is backend/src/blocks/mod.rs:23 — pub const APPSRC_MAX_TIME: gst::ClockTime = gst::ClockTime::from_seconds(10);. Worst case: a newcomer that decodes nothing looks live for up to ~10 s past the grace, and then it is reaped. The effect is bounded and mild, and it only happens when the chain was already behind real time
BUFFER probe, grace keying and stall-side label are unchanged from 449fd20	CONFIRMED	Carried from my 2026-09-25 review. Neither new commit touches whip.rs outside the assert and its import, and the duration is still backend/src/whip_session_manager.rs:441 — Some((ingress_idle.max(output_idle), side))

Design record — What was rejected: flushing the slot appsrc on takeover. That would close the drain-tail gap, but it is a new teardown behaviour with its own risk, and the gap it closes is bounded and mild. I agree with leaving it out. If a held seat ever shows a ~10 s false-live window in practice, that flush is the follow-up. The accepted trade from my last review still applies: a blocked consumer now churns the seat roughly every 7 s instead of holding it for ever, and the stall-side label makes that legible.

Radius — SHARED, as argued in the 2026-09-25 review. Session lifetime decisions change for every WHIP Input flow, but nothing outside the WHIP session manager and its watchdog reads idle().

Overlaps — #792 (draft) owns the per-medium refinement and is stacked on this PR. #805 (draft) carries that same per-medium restructuring. #854 (draft) is a pure move of these files. #763 touches WhipSessionManager in other hunks. None of them leaves a design question for this merge to settle, and this PR lands before all of them.

Tests & CI — Run 36215508270 at 47c41fd is green: Check (Linux), both Linux builds, WASM and API Contract Check. In job 108330585910, the whip_session_manager::tests cases all pass, and the pipeline_lifecycle_test canary reports "3 passed". The canary's gst_mini_object_unref CRITICALs also appear in a 2026-09-03 main run, so this diff did not cause them. macOS and Windows were skipped, and nothing in this diff is platform-specific.

Confidence: HIGH

[srperens]

This supersedes my Approve at 47c41fd. The verdict stands at Approve. The new head b26bc11 is a rebase onto b28be77 with no content change.

Verdict: Approve. I checked this directly rather than trusting the compare view, which shows 29 commits because the merge-base moved:

• git diff 47c41fd b26bc11 over the PR's three files is empty.
• The PR's own patch against each base is the same 1491 lines, and its +/- lines are identical.
• No main commit between the two bases touches whip.rs, whip_session_manager.rs or collector.rs.

So every claim row carries over from my review of 2026-09-28. Two spot checks at this head:

• backend/src/blocks/builtin/whip.rs:718 — const _: () = assert!(DECODE_GRACE.as_millis() < INACTIVITY_TIMEOUT.as_millis());
• backend/src/whip_session_manager.rs:441 — Some((ingress_idle.max(output_idle), side))

Tests & CI: I checked run 36445821036 at b26bc11. Check (Linux), both Linux builds, WASM, API Contract Check and Static JS Tests are green. In job 109007773818, all 17 whip_session_manager::tests cases pass, including a_predecessors_trailing_frames_do_not_cut_the_decode_grace_short and a_reaped_session_names_which_side_of_the_chain_stopped. The pipeline_lifecycle_test canary ran: 3 passed; 0 failed. macOS and Windows were skipped, and nothing in this diff is platform-specific.

Radius: SHARED, carried from my previous review.

Overlaps: #792, #805, #854 and #763 are as in my previous review. Two WHIP PRs are new since then:

• #893 edits create_whipserversrc_for_session and forward_sample_to_slot (PTS ordering at the slot bridge). It is near this PR's hunks, but it is a different mechanism, so the overlap is textual.
• #901 (draft) restarts a reused slot's video decodebin. Its evidence names the drain tail this PR bounds: "Frames left in the decoder's reorder queue come out at the start of the next session". That is the takeover-flush follow-up my previous review set aside. #901 would shrink the ~10 s false-live window this PR documents, and it does not contradict the design. This PR should land first, and #901 should then revisit the comment near backend/src/whip_session_manager.rs:413.

Neither leaves a design question for this merge to settle.

Confidence: HIGH