[wagenet]

Problem

The WHIP ingest page is slow to come back after its transport dies. Two timers stack up: ICE disconnected gets 10 s to recover (whip.js), then every retry waits a flat 10 s (ingest.html). On top of that Chrome takes ~6 s to notice at all, so the first reconnect attempt lands ~26 s after the drop.

The server frees the seat long before that. The inactivity watchdog reclaims it at 10 s, and #753 will hand it over as soon as ~2 s. Neither helps while the page is not asking.

What this does

ICE_DISCONNECT_GRACE_MS = 4000 — disconnected is genuinely transient and often recovers, so it still gets a grace period, just a shorter one. Tearing down early costs a needless renegotiation; waiting costs the publisher, who keeps encoding into a dead transport meanwhile.

RECONNECT_DELAYS = [1000, 2000, 4000, 8000, 16000, 30000] replaces the flat 10 s, last value repeating. Better on both axes: first attempt at 1 s instead of 10 s, and 15 attempts now cover 331 s instead of 150 s.

RECONNECT_DELAYS, MAX_RECONNECT_ATTEMPTS and the delay lookup live in whip.js rather than inline in ingest.html, so CI can reach them. The page picks them up as globals from the script tag it already had.

The first retry is early on purpose. With #753 the server holds an arriving POST for up to 3 s while it decides whether the sitting session is dead, so a retry that lands before the seat is free is answered rather than refused. Without #753 it collects a 503 and the next attempt is 2 s later.

Verification

Playwright drives the real page in Chromium with a fake camera and mic, waits for media to reach the server (Pad video_0), then issues a WHIP DELETE for the session from outside the browser. The page is not told — from its point of view the server has vanished, which is what network loss looks like. Same rig, same drop, before and after:

	v10 (before)	v11 (after)
Chrome reports ICE disconnected	+6.2 s	+6.3 s
first re-POST reaches the server	+26.4 s	+11.6 s
media flowing again	+27.4 s	+12.1 s

Backoff sequence and grace value read out of the page source and stepped through all 15 attempts: 1, 2, 4, 8, 16, 30 × 10, no undefined at the boundary. Both files pass node --check (the inline script extracted from the HTML).

Server was built from main — no #753 — so this measures the client timing on its own.

The table was measured against client v11. v12 moves the constants into whip.js without changing a value, which the unit tests below assert.

What this does not fix

Chrome's ~6 s detection is now the floor, and a page cannot shorten it. That keeps the first re-POST at ~11.6 s, still past the 10 s watchdog, so #753's takeover window remains out of reach for this page even after this change. Getting under it means detecting the drop ourselves — polling getStats() for outbound RTP with no returning RTCP — rather than waiting for the browser's ICE state. Worth doing, not here.

whep/player.html still has the flat 10 s reconnect and is deliberately untouched. It is a subscriber, not a publisher, so it is not racing the ingest watchdog and the fix is not the same one. Separate change.

Risk

An ICE blip that would have recovered between 4 s and 10 s now causes a full renegotiation instead: new POST, new session, new keyframe wait, a second or two of black. The measurements above put Chrome's own detection at ~6 s, so a blip has to be quite specifically timed to land in that band.

Tests

New Static JS Tests job in .github/workflows/ci.yml, green on this head. Runs on pull_request and push to main, no dependency on the Rust jobs.

ingest.html loads whip.js with a plain <script src>, so whip.js cannot take ESM exports, and the delay computation is unreachable from node while it sits inline in the HTML. Move the constants and the lookup into whip.js and give that file a typeof module !== 'undefined' export guard: inert in the browser, enough for node's CJS loader to require() the same source the page runs. No package.json, no bundler — node:test and node:assert are built in.

Eight tests, green locally and in CI:

Test	Asserts
every attempt within the retry budget has a delay	attempts 1..MAX_RECONNECT_ATTEMPTS yield a finite positive number, never undefined
attempts past the end of the list clamp to the last delay	len, len+1 and 100 all give 30 s
the delays are the schedule the page advertises	computed sequence equals RECONNECT_DELAYS
the backoff grows	monotonic, last delay strictly greater than first
the first retry beats the server freeing the slot	ICE_DISCONNECT_GRACE_MS + delay(1) < the server's 10 s INACTIVITY_TIMEOUT
the retry budget covers a long outage	total span >= 300 s
the browser path still works	whip.js through node:vm with no module: the guard stays inert, the globals still land in the page's scope
ingest.html does not redeclare the shared reconnect globals	a duplicate top-level const is a page-wide parse error, since classic scripts share one global lexical scope

The assertions read the production constants rather than restating them, so a wider MAX_RECONNECT_ATTEMPTS or an edited RECONNECT_DELAYS stays covered without editing the test.

Revert check. With the flat 10 s put back (RECONNECT_DELAYS = [10000], ICE_DISCONNECT_GRACE_MS = 10000), 3 of 8 fail:

the schedule is flat - it does not back off
first reconnect POST lands at 20000ms, at or past the server's 10000ms slot timeout
retries span only 150s

Injecting a duplicate const RECONNECT_DELAYS into ingest.html fires the redeclaration guard. Both reverts undone.

Scope. The unit tests cover whip/whip.js only. webrtc/webrtc.js, webrtc/devices.js and whep/whep.js get a node --check parse gate and nothing more — they are DOM-driven end to end, so loading them outside a browser means restructuring files this PR does not touch. backend/static/tests/ sits outside every #[folder] in assets.rs, so nothing new is embedded in the binary.

Not run: cargo test, and the openapi snapshot — no Rust or API surface changed. cargo clippy and cargo fmt passed via the pre-commit hook. The Playwright rig was not re-run against v12; the constants changed file, not value, which the tests assert.

🤖 Generated with Claude Code

[srperens]

Verdict: Comment — correct diagnosis, sound fix, and every checkable claim confirms, but no check run in this repo's CI exercises the changed logic, so approval is blocked on that ground alone.

Claims

Claim	Verdict	Evidence
Server's inactivity watchdog fires at 10s, matching the "server frees the seat" claim	CONFIRMED	backend/src/blocks/builtin/whip.rs:506 — `const INACTIVITY_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);`
RECONNECT_DELAYS indexing never hits undefined, including past attempt 6	CONFIRMED	backend/static/whip/ingest.html:293 — `const delay = RECONNECT_DELAYS[Math.min(reconnectAttempt, RECONNECT_DELAYS.length) - 1];` — reconnectAttempt starts at 0 (ingest.html:113) and is incremented before use, so the index is [0, 5] for attempts [1, 15]
#753 is not yet merged, so this PR's measured numbers reflect current main, not a #753-assisted handoff	CONFIRMED	gh pr view 753: state=OPEN, mergedAt=null
CI never exercises this JS	CONFIRMED	.github/workflows/ci.yml contains no reference to static, whip, node --check, or a JS runner

Diagnosis — Root cause and fix are both right: two stacked flat 10s waits are slower than the server's own 10s reclaim, so the page's retry consistently loses the race. Backoff starting at 1s instead of a flat 10s, and a shorter (still nonzero) ICE grace period, directly close that gap. The PR is explicit about what it does not fix (Chrome's own ~6s ICE-detection floor) rather than overselling the win — that's the right scope statement.

Not covered — backend/static/whep/player.html has the identical pattern: its own flat RECONNECT_DELAY = 10000 (player.html:120) and a hardcoded 10000ms wait on its own disconnect path (player.html:810), untouched by this diff and not sharing whip.js. BOUNDED, not ABSOLUTE — WHEP playback has no server-side "seat" to lose the way WHIP ingest does, so the urgency argument in this PR's body doesn't automatically transfer, but the same slow-reconnect UX symptom is still there.

Radius — LOCAL: whip.js has exactly one consumer (ingest.html; grepped backend/static/ for other includes), and the diff changes no server code.

Tests & CI — Check (Linux), both Build (Linux ...), Check & Build (WASM), API Contract Check green at this head — expected, since none of them touch JS; the repo genuinely has no JS runner (package.json doesn't exist under backend/static/), matching the PR's own statement. The Playwright numbers in the body are not something I ran or can verify here; the source-level logic they describe (bounds, delay values) I did verify directly, above.

Confidence: HIGH

[srperens]

This supersedes my review at a06661c — the verdict does not stand. That review's sole blocker was that no CI check exercises the changed JS; 02b9362 adds a CI job that does, and the diagnosis and fix logic are unchanged from what I already confirmed.

Verdict: Approve — CI now runs the covering tests, and the reconnect logic they exercise is the same logic my prior review verified at the source level.

Claims

Claim	Verdict	Evidence
A CI job now runs the new JS unit tests, not just a syntax check	CONFIRMED	.github/workflows/ci.yml:246 — run: node --test backend/static/tests/*.test.js, job static-js added in this diff
The tests executed for real, none skipped	CONFIRMED	CI run 33974392418, job Static JS Tests: # tests 8, # pass 8, # fail 0, # skipped 0
whipReconnectDelay reproduces the exact bound the old inline expression used	CONFIRMED	backend/static/whip/whip.js:21 — `return RECONNECT_DELAYS[Math.min(attempt, RECONNECT_DELAYS.length) - 1];` — same clamp my prior review confirmed in the inline expression this diff replaces
Page version bumped so the deployed script is verifiable, per CLAUDE.md	CONFIRMED	backend/static/whip/ingest.html:571 — log('WHIP ingest client v12');

Diagnosis — Unchanged from my a06661c review: two stacked flat 10s waits lost the race against the server's own 10s seat-reclaim; backoff starting at 1s closes the gap. whipReconnectDelay is extracted into whip.js behind a typeof module guard so Node's test runner can drive it directly, and ingest.html now calls the shared function instead of duplicating the formula.

Not covered — Unchanged from my prior review: backend/static/whep/player.html still has its own flat 10s reconnect pattern, untouched by this diff. BOUNDED, not ABSOLUTE — noted there for the same reason (no server-side seat to lose on the WHEP path), not a blocker here.

Radius — LOCAL: whip.js has one page consumer, the new CI job runs only against backend/static, and no server code changed.

Tests & CI — Check (Linux), Build (Linux x86_64/ARM64), Check & Build (WASM), API Contract Check, sccache preflight, and the new Static JS Tests all pass at 02b9362; Build (macOS)/Build (Windows) skip as usual.

Confidence: HIGH