fix(ui,plex): a dialog keeps the focus its child asked for, and a hopeless segment fails at once

Both found by UAT in a real browser, after the tests for them were already green — which is the
whole reason this repo settles focus and timing claims by running them.

- **The autoFocus fix worked, and then the dialog took the focus back.** React 18's StrictMode runs
  every effect mount→cleanup→mount in development: the cleanup handed focus back to the trigger, and
  the second mount, finding the focus outside the dialog, moved it to the first focusable — off the
  `autoFocus` field a beat after the browser had put it there. Chrome measured it: INPUT (commitMount)
  → BODY (cleanup) → ✕ (re-mount). The effect is idempotent now — it remembers what the dialog was
  focused on and restores THAT on a re-mount — and the close-time restore is conditional on the
  dialog still holding the focus, which it tracks as it changes rather than computing at cleanup,
  where the panel is already out of the document. A dialog no longer yanks focus back from wherever
  the user has since moved it.

- **Passing ffmpeg's handle to the segment wait only shortened the FINISHED case.** Measured against
  the running localdev API: a request for `seg_99999.ts` on a live session still took 20 195 ms,
  because that ffmpeg has not exited and the wait has no other reason to stop. ffmpeg writes segments
  in order, so a segment whose predecessor does not exist is not the next one and cannot arrive —
  and that is knowable at once. Same request now: 45 ms.

Verified on localdev against the real library (Westworld S01E06, remux, multi-audio): Mod+K opens the
command palette with the focus in its search field and typing reaches it; playback runs on the new
session-scoped HLS URL; a request naming a stale session id is a 404; and a viewer paused for longer
than the finished-session rule keeps their 2.4 GB session across two reaper passes — the progress
checkpoint touches it — and resumes cleanly.

vitest 1009, pytest 1127.
This commit is contained in:
2026-09-12 04:31:51 +02:00
parent 33fddacee2
commit 1f81684b6e
4 changed files with 84 additions and 12 deletions
+30
View File
@@ -758,6 +758,36 @@ def current_session(key: str) -> HlsSession | None:
return s return s
# A segment file, single-audio (`seg_12.ts`) or one rendition of a multi-audio session
# (`seg_12_0.ts`). ffmpeg writes them strictly in order, which is what makes `segment_may_arrive`
# exact rather than a heuristic.
_SEG_NAME_RE = re.compile(r"^seg_(\d+)(_\d+)?\.ts$")
def segment_may_arrive(directory: Path, filename: str) -> bool:
"""Whether waiting for `filename` can possibly end in a file.
ffmpeg produces segments sequentially, so a segment whose PREDECESSOR does not exist yet is not
the next one — it is somewhere beyond the generated region, and no amount of waiting will
conjure it. Saying so at once matters because the route that waits is a SYNC one: it holds a
thread off the ~40-slot pool, and a pooled DB connection with it, for the whole timeout. An
ordinary seek past the produced region was enough to park one for 20 s, and forty concurrent
requests for a name that will never exist parked the whole API (deep-review round 5).
Anything that is not a segment (a playlist, a WebVTT track) is left to the timeout: those are
rewritten continuously and have no ordering to read."""
m = _SEG_NAME_RE.match(filename)
if m is None:
return True
idx = int(m.group(1))
if idx == 0:
return True
try:
return (directory / f"seg_{idx - 1}{m.group(2) or ''}.ts").exists()
except OSError:
return False
def wait_for(path: Path, timeout: float = 20.0, proc: subprocess.Popen | None = None) -> bool: def wait_for(path: Path, timeout: float = 20.0, proc: subprocess.Popen | None = None) -> bool:
"""Wait until a session file (playlist / segment) exists and is non-empty. Segments are """Wait until a session file (playlist / segment) exists and is non-empty. Segments are
produced ~faster than realtime, so a segment just ahead of playback appears quickly; a segment produced ~faster than realtime, so a segment just ahead of playback appears quickly; a segment
+6
View File
@@ -656,6 +656,12 @@ def stream_hls_file(
if s is None or s.id != session_id: if s is None or s.id != session_id:
raise HTTPException(status_code=404, detail="No active playback session") raise HTTPException(status_code=404, detail="No active playback session")
path = s.dir / filename path = s.dir / filename
# A segment beyond the generated region can never arrive: ffmpeg writes them in order, so if the
# predecessor is not there this one is not next. Answering now instead of waiting it out is what
# keeps a seek past the produced region — or a deliberate flood of hopeless names — from parking
# threadpool workers and their DB connections (see `segment_may_arrive`).
if not plex_stream.segment_may_arrive(s.dir, filename):
raise HTTPException(status_code=404, detail="Not available yet")
timeout = 30 if ext == ".m3u8" else 20 timeout = 30 if ext == ".m3u8" else 20
# `proc` so a session whose ffmpeg has already exited answers at once instead of polling out the # `proc` so a session whose ffmpeg has already exited answers at once instead of polling out the
# whole timeout: this is a SYNC route on the ~40-slot threadpool, holding a pooled DB connection # whole timeout: this is a SYNC route on the ~40-slot threadpool, holding a pooled DB connection
+15 -7
View File
@@ -24,8 +24,11 @@ from app.plex import stream as plex_stream
class _Proc: class _Proc:
def __init__(self, running: bool = False):
self.running = running
def poll(self): def poll(self):
return 0 # finished: `wait_for` may not poll out its whole timeout on a missing file return None if self.running else 0
def terminate(self): def terminate(self):
pass pass
@@ -35,10 +38,10 @@ class _Proc:
class _Session: class _Session:
def __init__(self, session_id: str, directory): def __init__(self, session_id: str, directory, running: bool = False):
self.id = session_id self.id = session_id
self.dir = directory self.dir = directory
self.proc = _Proc() self.proc = _Proc(running)
self.last_access = time.time() self.last_access = time.time()
@@ -75,6 +78,7 @@ def _client(db, monkeypatch):
def test_a_segment_is_served_from_the_session_the_url_names(db, monkeypatch, tmp_path, sessions): def test_a_segment_is_served_from_the_session_the_url_names(db, monkeypatch, tmp_path, sessions):
d = tmp_path / "m1_0_0_+0.00" d = tmp_path / "m1_0_0_+0.00"
d.mkdir() d.mkdir()
(d / "seg_11.ts").write_bytes(b"") # its predecessor, so seg_12 is the next one
(d / "seg_12.ts").write_bytes(b"mine") (d / "seg_12.ts").write_bytes(b"mine")
sessions["m1"] = _Session("sid-a", d) sessions["m1"] = _Session("sid-a", d)
client = _client(db, monkeypatch) client = _client(db, monkeypatch)
@@ -93,6 +97,7 @@ def test_a_segment_of_a_replaced_session_is_a_404_not_the_new_sessions_bytes(
next segment must fail honestly — the player reclaims at the playhead — not arrive as B's.""" next segment must fail honestly — the player reclaims at the playhead — not arrive as B's."""
theirs = tmp_path / "m1_1500_0_+0.00" theirs = tmp_path / "m1_1500_0_+0.00"
theirs.mkdir() theirs.mkdir()
(theirs / "seg_11.ts").write_bytes(b"")
(theirs / "seg_12.ts").write_bytes(b"theirs") (theirs / "seg_12.ts").write_bytes(b"theirs")
sessions["m1"] = _Session("sid-b", theirs) sessions["m1"] = _Session("sid-b", theirs)
client = _client(db, monkeypatch) client = _client(db, monkeypatch)
@@ -127,12 +132,15 @@ def test_a_playlist_with_no_live_session_is_a_404_not_a_silent_restart_at_zero(
def test_a_segment_that_will_never_arrive_fails_at_once(db, monkeypatch, tmp_path, sessions): def test_a_segment_that_will_never_arrive_fails_at_once(db, monkeypatch, tmp_path, sessions):
"""`stream_hls_file` is a SYNC route: it runs on the ~40-slot threadpool and holds a pooled DB """`stream_hls_file` is a SYNC route: it runs on the ~40-slot threadpool and holds a pooled DB
connection while it waits. Waiting without the producer's handle polled out the whole 20 s even connection while it waits. It used to poll out the whole 20 s for a name that could never appear
when that ffmpeg had already exited and no segment could ever appear — one ordinary seek past the — an ordinary seek past the produced region was enough to park a worker and a connection, and
produced region parked a worker and a connection for 20 s (deep-review round 5).""" forty hopeless names parked the API (deep-review round 5). ffmpeg writes segments in order, so a
segment whose predecessor is absent is not the next one, and that is knowable at once — even
while ffmpeg is still running, which is the case `proc` alone cannot shorten."""
d = tmp_path / "m1_0_0_+0.00" d = tmp_path / "m1_0_0_+0.00"
d.mkdir() d.mkdir()
sessions["m1"] = _Session("sid-a", d) # its _Proc.poll() reports a finished ffmpeg (d / "seg_0.ts").write_bytes(b"x") # the session is producing; seg_99999 is simply not next
sessions["m1"] = _Session("sid-a", d, running=True)
client = _client(db, monkeypatch) client = _client(db, monkeypatch)
started = time.monotonic() started = time.monotonic()
try: try:
+33 -5
View File
@@ -1,4 +1,4 @@
import { useEffect, useState, type RefObject } from "react"; import { useEffect, useRef, useState, type RefObject } from "react";
// The focus half of the dialog contract, in ONE place. `ui/Modal` owns it for every dialog that goes // The focus half of the dialog contract, in ONE place. `ui/Modal` owns it for every dialog that goes
// through the shell; the onboarding wizard renders its own shell (a bare `ui/Overlay`, because it is // through the shell; the onboarding wizard renders its own shell (a bare `ui/Overlay`, because it is
@@ -54,19 +54,47 @@ export function useDialogChrome(scopeRef: RefObject<HTMLElement | null>): void {
// later (in the effect) would capture a now-inside element that unmounts on close, stranding focus // later (in the effect) would capture a now-inside element that unmounts on close, stranding focus
// on <body>. useState initializer runs exactly once, on first render. // on <body>. useState initializer runs exactly once, on first render.
const [restoreTo] = useState(() => document.activeElement as HTMLElement | null); const [restoreTo] = useState(() => document.activeElement as HTMLElement | null);
// What the dialog is focused ON, so a RE-mount can put the focus back where it was rather than on
// the first control. React 18's StrictMode runs every effect mount→cleanup→mount in development:
// the cleanup hands focus back to the trigger, and the second mount then found the focus outside
// the dialog and moved it to the first focusable — taking it off the `autoFocus` field a beat
// after the browser had put it there. Measured in Chrome: the command palette opened with the
// focus on its ✕ in dev and on its search field in prod, from the same code (round 5).
const focusedRef = useRef<HTMLElement | null>(null);
// Whether the dialog still holds the focus. Read at cleanup rather than computed there: by the
// time a passive cleanup runs on a real unmount the panel is already out of the document and the
// focus has fallen to <body>, so "does the panel contain activeElement" answers false for both a
// close and a user who clicked away — and those two want opposite things.
const heldRef = useRef(true);
useEffect(() => { useEffect(() => {
const prev = document.body.style.overflow; const prev = document.body.style.overflow;
document.body.style.overflow = "hidden"; document.body.style.overflow = "hidden";
// Initial focus: unless an autoFocus child already placed it inside (e.g. a rename input), move // Initial focus: unless an autoFocus child already placed it inside (e.g. a rename input), move
// focus to the first control so the dialog opens focused, not stranded on the background. // focus to where this dialog was focused before — or, the first time, to the first control, so
// the dialog opens focused rather than stranded on the background.
const scope = scopeRef.current; const scope = scopeRef.current;
if (scope && !scope.contains(document.activeElement)) { if (scope && !scope.contains(document.activeElement)) {
(focusablesIn(scope)[0] ?? scope).focus(); const back = focusedRef.current;
const target =
(back?.isConnected && scope.contains(back) ? back : null) ??
focusablesIn(scope)[0] ??
scope;
target.focus();
} }
const onFocusIn = () => {
const inside = scopeRef.current?.contains(document.activeElement) ?? false;
heldRef.current = inside;
if (inside) focusedRef.current = document.activeElement as HTMLElement;
};
onFocusIn();
document.addEventListener("focusin", onFocusIn);
return () => { return () => {
document.removeEventListener("focusin", onFocusIn);
document.body.style.overflow = prev; document.body.style.overflow = prev;
// Restore focus to the trigger, if it's still in the document and focusable. // Restore focus to the trigger, if it's still in the document and focusable — and only if the
if (restoreTo && restoreTo.isConnected) restoreTo.focus(); // dialog still held the focus. Once the user has moved it elsewhere, taking it back is the
// rudest thing a closing dialog can do.
if (heldRef.current && restoreTo && restoreTo.isConnected) restoreTo.focus();
}; };
}, [restoreTo, scopeRef]); }, [restoreTo, scopeRef]);
} }