fix(plex): the reclaim window rolls, and a refused finish is not performed through a third route

Two ways round 6's own fixes did not bound what their comments said they bounded.

The reclaim budget's window anchor was written only where the window RESET, so it marked the window's
start rather than the last loss — a tumbling window, not the rolling one the constant is named for.
Any loss cadence slower than half the window satisfied the reset on every second loss, so the budget
never reached its bound: measured over 200 loss cycles, 61/70/90/119s apart all gave 200 reclaims and
never an error, while 5s and 30s apart correctly gave two and then the honest answer. The failing
band is exactly the case the ERROR handler names — a transcode that falls behind realtime — and both
committed tests spaced their losses 0s and 10min apart, i.e. outside it. The anchor now moves with
every loss.

And `finish`'s refusal handler lowered the play-through latch, which is the only gate on the 10s
interval, the page-hide beacon and `settle`. All three post `absRef`, which on the play-to-the-end
path IS the end of the media, and the backend reads a position within `_FINISH_MARGIN_S` of the
duration as a finish — so within ten seconds of the toast saying the mark had failed, the item was
marked watched, its position cleared and a scrobble sent to Plex. Round 6 closed the explicit
rollback write and left the other three open; the test it added set `upNext: false`, which tears the
effect down before any of them can fire. The refusal now takes back the CLAIM rather than the latch:
a second flag stops `settle` re-asserting watched, and playing again clears both.

`FINISH_MARGIN_S` duplicated the backend's constant in a second language with nothing relating them,
and its comment had the asymmetry backwards — if the BACKEND's margin grows, this guard starts
admitting a rollback the backend reads as a finish, i.e. re-opens the defect above. Pinned by a
contract test that reads the Python, and the comment corrected.

frontend/src/components/PlexPlayer.hls.dom.test.tsx

@@ -220,6 +220,34 @@ describe("an HLS session the server no longer has", () => {
     }
   });
 
+  it("gives up on a session lost over and over on a SLOW cadence too", async () => {
+    // The window's anchor used to be written only where the window RESET, so it marked the start of
+    // the window instead of the last loss — a tumbling window, not the rolling one the constant is
+    // named for. Any cadence slower than half the window then cleared the budget on every second
+    // loss and the bound never bit: a transcode falling ~70s behind realtime — the case the ERROR
+    // handler itself names — reclaimed forever (measured: 200 losses 70s apart → 200 reclaims,
+    // never an error). The two cases above space their losses 0s and 10min apart, i.e. both sit
+    // outside the 60–120s band where the defect lived (deep-review round 7).
+    vi.useFakeTimers({ shouldAdvanceTime: true, toFake: ["Date"] });
+    try {
+      const session = mount();
+      const v = () => document.querySelector("video") as Media;
+      for (let i = 0; i < 6 && !screen.queryByText("plex.player.errGeneric"); i++) {
+        await waitFor(() => expect(hlsInstances).toHaveLength(i + 1));
+        act(() => hlsInstances[i]!.emit(Hls.Events.MANIFEST_PARSED));
+        v()._t = 700;
+        act(() => void v().dispatchEvent(new Event("timeupdate")));
+        act(() => hlsInstances[i]!.emit(Hls.Events.ERROR, lost404));
+        act(() => void vi.setSystemTime(Date.now() + 70_000));
+      }
+      await screen.findByText("plex.player.errGeneric");
+      // Two reclaims, each within two minutes of the loss before it — then the honest answer.
+      expect(session).toHaveBeenCalledTimes(3);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it("is a failure, not a restart, when the session never played", async () => {
     // Otherwise a session that 404s from the start would restart itself in a loop.
     const session = mount();

frontend/src/components/PlexPlayer.session.dom.test.tsx

@@ -382,6 +382,44 @@ describe("leaving an item that was played through", () => {
     expect(progress.mock.calls.filter(([id, , , final]) => id === "A" && final)).toEqual([]);
   });
 
+  it("performs the refused mark through no OTHER route while it stays on the finished item", async () => {
+    // Round 6 guarded the explicit rollback write, but the same catch lowered the play-through
+    // latch — and that latch is the only gate on the 10s interval, the page-hide beacon and
+    // `settle`. All three post `absRef`, which on this path IS the end of the media, and the
+    // backend reads a position within `_FINISH_MARGIN_S` of the duration as a finish: within ten
+    // seconds of the toast saying the mark had failed, the item was watched, its position cleared
+    // and a scrobble sent to Plex (deep-review round 7). `upNext` is ON here — the default — so the
+    // player STAYS on the finished item and those routes are live; the round-6 test above turns it
+    // off, which tears the effect down before any of them can fire.
+    prefs({ upNext: true });
+    const beacon = vi.spyOn(api, "plexProgressBeacon").mockImplementation(() => {});
+    const { qc, progress, unmount } = mount({
+      details: { A: item("A"), B: item("B") },
+      sessions: { A: () => Promise.resolve(direct("A")), B: () => new Promise(() => {}) },
+      queue: ["A", "B"],
+    });
+    vi.spyOn(api, "plexSetState").mockRejectedValue(new Error("offline"));
+    await playingAt("/media/A", 2995);
+    endMedia(); // the up-next card is up, the item is finished, and the mark comes back refused
+    await tick();
+    progress.mockClear();
+
+    // The viewer closes the tab while the failed-mark toast is still up.
+    act(() => void window.dispatchEvent(new Event("pagehide")));
+    expect(beacon).not.toHaveBeenCalled();
+
+    // The refusal dropped the cached detail — and the open player's own observer refetches it, so
+    // by the time the viewer leaves there is an entry again. `settle` must not re-assert watched
+    // onto it: only the server is honest about this item now.
+    act(() => void qc.setQueryData(qk.plexItem("A", "en"), item("A")));
+
+    // And leaving the player: `settle` must neither post the end position nor re-assert watched.
+    unmount();
+    await tick();
+    expect(progress.mock.calls.filter(([id]) => id === "A")).toEqual([]);
+    expect(qc.getQueryData<PlexItemDetail>(qk.plexItem("A", "en"))?.status).not.toBe("watched");
+  });
+
   it("does restore the position when the refused finish came from the credits skip", async () => {
     // The other caller of the same helper captures a MID-episode position, and there the rollback
     // is the point: without it the viewer loses the stretch they watched. One helper, two callers,

frontend/src/components/PlexPlayer.tsx

@@ -435,9 +435,15 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) {
           if (parsed && data.type === Hls.ErrorTypes.NETWORK_ERROR && data.response?.code === 404) {
             const now = Date.now();
             // A new window: the last loss was long enough ago that this is a fresh incident, not the
-            // same session being lost over and over.
-            if (now - restartsRef.current.at > RECLAIM_WINDOW_MS)
-              restartsRef.current = { n: 0, at: now };
+            // same session being lost over and over. The anchor moves with EVERY loss, because with
+            // it left at the window's start the window tumbled instead of rolling: any loss period
+            // longer than half the window satisfied the reset on every second loss, so the budget
+            // never reached its bound and the reclaim loop it exists to end ran forever (measured:
+            // 200 losses 70s apart → 200 reclaims, never an error; 5s apart → 2 and an error, which
+            // is why the committed test, spacing its cases 0s and 10min apart, could not see it —
+            // deep-review round 7).
+            if (now - restartsRef.current.at > RECLAIM_WINDOW_MS) restartsRef.current.n = 0;
+            restartsRef.current.at = now;
             if (restartsRef.current.n < MAX_SESSION_RECLAIMS) {
               restartsRef.current.n += 1;
               loadSession(playhead(absRef, loadStartRef), !video.paused);

frontend/src/lib/plexFinishMargin.contract.test.ts

@@ -0,0 +1,39 @@
+import { readFileSync } from "node:fs";
+import { expect, it } from "vitest";
+
+// One invariant, two languages, no shared wire format — so a test is the only thing that can hold
+// them together.
+//
+// The backend finishes an item when a progress checkpoint lands within `_FINISH_MARGIN_S` of the
+// duration: watched, position cleared, scrobbled to Plex. The player's refused-finish rollback must
+// therefore never post a position inside that window, or the branch named "take the finish back"
+// performs, through the backend, the very write that was just refused (rounds 6 and 7 both landed
+// on this). The frontend mirrors the number so it knows where the window starts.
+//
+// Both were 30 and exactly complementary, but nothing related them, so a backend-only change would
+// have re-opened the defect silently — a rule stated in a comment and not enforced in code, the one
+// thing this repo's house rules single out. The direction matters: the frontend's margin must be at
+// LEAST the backend's. Larger is merely conservative (a rollback skipped where it could have run);
+// smaller is the bug.
+//
+// Both values are read from source rather than imported: `plexPlayerHooks` pulls in the i18n module
+// (and with it `localStorage`), and booting half the app to read one integer would make this test
+// about the module graph instead of about the two numbers.
+function constantIn(path: string, pattern: RegExp): number {
+  const source = readFileSync(new URL(path, import.meta.url), "utf8");
+  const m = pattern.exec(source);
+  // If this fails the constant was renamed or moved — find it and re-point this test, because the
+  // invariant it guards did not go away.
+  expect(m, `${pattern} not found in ${path}`).not.toBeNull();
+  return Number(m![1]);
+}
+
+it("never lets the player's finish margin fall below the backend's", () => {
+  const backend = constantIn(
+    "../../../backend/app/routes/plex/playback.py",
+    /^_FINISH_MARGIN_S\s*=\s*(\d+)/m,
+  );
+  const frontend = constantIn("./plexPlayerHooks.ts", /^const FINISH_MARGIN_S\s*=\s*(\d+)/m);
+  expect(backend).toBeGreaterThan(0);
+  expect(frontend).toBeGreaterThanOrEqual(backend);
+});

frontend/src/lib/plexPlayerHooks.ts

@@ -23,9 +23,16 @@ import { PLEX_SCALE, normalizeVolumeIn, stepVolumeIn } from "./playerVolume";
 
 // How close to the end the backend counts as "finished" (`_FINISH_MARGIN_S` in routes/plex/playback):
 // a progress checkpoint at or past `duration - this` marks the item watched and clears the position.
-// Mirrored here because `finish`'s rollback must not accidentally drive that branch. It is a
-// one-way safety margin, not a shared contract — if the two ever diverge, the worst case is a
-// rollback that is skipped or a resume point set a few seconds from the end.
+// Mirrored here because `finish`'s rollback must not accidentally drive that branch.
+//
+// Drift is NOT symmetric, which an earlier version of this comment had backwards. If THIS value
+// grows, the rollback is skipped where it need not be — a lost resume point, annoying. If the
+// BACKEND's grows (its own comment invites that tuning by contrasting it with the feed's 10 s),
+// this guard starts admitting a rollback the backend reads as a finish, which marks the item
+// watched, clears the position and scrobbles to Plex — re-performing the very write that was just
+// refused, i.e. exactly the defect this constant was introduced to close. So the invariant is
+// one-directional and it is PINNED, not described: `plexFinishMargin.contract.test.ts` reads the
+// Python and fails if this value ever drops below it (round 7).
 const FINISH_MARGIN_S = 30;
 
 // A live wall clock for the top bar, re-rendered every 15s (minute resolution is enough for the
@@ -111,6 +118,16 @@ export function usePlexProgress(opts: {
   const finishRef = useRef<(confirmed?: Promise<unknown>) => void>(() => {});
   useEffect(() => {
     let finished = false;
+    // Raised when the caller's watched write came back REFUSED. The play-through latch itself stays
+    // up, because lowering it re-armed three other routes to the same write: the 10s interval, the
+    // page-hide beacon and `settle` all post `absRef`, which on this path IS the end of the media,
+    // and the backend reads a position within `_FINISH_MARGIN_S` of the duration as a finish. So the
+    // branch named "take the finish back" marked the item watched, cleared the position and
+    // scrobbled to Plex within ten seconds — the refused write performed through a third route,
+    // under an error toast saying it had failed. Round 6 closed only the explicit rollback write;
+    // this closes the other three (deep-review round 7). What the refusal does take back is the
+    // CLAIM: the cache must not say watched, and `settle` must not re-assert it on the way out.
+    let finishRefused = false;
     // `final` = a settled checkpoint (pause / page-hide / leaving the player) vs. the periodic tick.
     // Only final checkpoints get mirrored to a linked Plex account (backend-side debounce), so Plex
     // sees the resume position when the user actually stops — not a timeline call every 10 seconds.
@@ -157,6 +174,7 @@ export function usePlexProgress(opts: {
     window.addEventListener("pagehide", onHide);
     const onPlayAgain = () => {
       finished = false;
+      finishRefused = false;
     };
     video?.addEventListener("play", onPlayAgain);
     // Back (unmount) persists the position server-side via flush(), but reopening the same item in
@@ -170,9 +188,14 @@ export function usePlexProgress(opts: {
       // Match the language-keyed detail query (E5 S6) so the optimistic resume patch lands.
       const key = qk.plexItem(id, plexLang());
       if (finished) {
-        qc.setQueryData(key, (old) =>
-          old ? { ...old, status: "watched", position_seconds: 0 } : old,
-        );
+        // A refused finish claims nothing: the server has the item unwatched at the position the
+        // rollback restored, and the catch below has already dropped this entry, so the next open
+        // fetches the truth. Re-patching "watched / 0" here would put back the very lie the
+        // refusal handler removed.
+        if (!finishRefused)
+          qc.setQueryData(key, (old) =>
+            old ? { ...old, status: "watched", position_seconds: 0 } : old,
+          );
         return;
       }
       flush(true);
@@ -185,14 +208,15 @@ export function usePlexProgress(opts: {
     finishRef.current = (confirmed?: Promise<unknown>) => {
       finished = true;
       // The latch has to go up BEFORE the media pauses, so it cannot wait for the server. If the
-      // watched write is then refused, the latch is taken back: the cached detail said watched /
-      // position 0 while the server still had the item unwatched at the last checkpoint, and the
-      // once-per-id start guard meant the corrected detail arriving later was never used — the
-      // episode reopened at 0:00 with a Watched badge the server disagreed with (round 5).
+      // watched write is then refused, what is taken back is the CLAIM, not the latch (see
+      // `finishRefused`): the cached detail said watched / position 0 while the server still had
+      // the item unwatched at the last checkpoint, and the once-per-id start guard meant the
+      // corrected detail arriving later was never used — the episode reopened at 0:00 with a
+      // Watched badge the server disagreed with (round 5).
       const pos = Math.floor(absRef.current);
       const dur = durationRef.current;
       void confirmed?.catch(() => {
-        finished = false;
+        finishRefused = true;
         // Restore the resume point the finish replaced — but ONLY where there is one to restore.
         // On the play-to-the-end path `pos` IS the end of the media, and the backend reads a
         // position within `_FINISH_MARGIN_S` of the duration as a finish: it would set watched,