From bd5a607075f3579618d21d49ac5179cbdbe1c6de Mon Sep 17 00:00:00 2001 From: Inch-high Date: Tue, 2 Jun 2026 18:57:53 +0100 Subject: [PATCH 1/2] fix(docker): stop blank page and stale update badge during container updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two distinct failures on /Docker while a container update is in flight, both rooted in how boot.ts handles the snapshot fetch. Blank page on refresh: docker-state.php returns `containers: []` (still HTTP 200) during the window the stock update_container flow rewrites the webui-info docker.json that getAllInfo() reads. resync()'s try/catch never fired (it's a 200), so it overwrote the populated store with the empty list AND poisoned the sessionStorage SWR cache via writeCachedSnapshot — leaving the page blank across refreshes until the update finished. Guard with isTransientEmptySnapshot(): when a snapshot is empty but the store already holds rows, skip the overwrite and wait for the next resync. First load (store empty) still renders a genuinely-empty server. Stale "update available" badge after a successful update: reconcileUpdating() already force-clears the badge when a container's id rotates (recreated = pull succeeded), trusting the rotation over Unraid's lagging digest-status cache — but only while the update probe still exists. onBatchComplete cleared all probes BEFORE resyncing, so the rotated-id snapshot had no probe to match and the badge fell back to the snapshot's stale updateAvailable=true. Resync first, then clearAllUpdating() for any probe that didn't self-reconcile. Tests: new boot.test.ts for isTransientEmptySnapshot; store.test.ts pins the onBatchComplete ordering contract. Full docker suite green (119). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/ts/docker/boot.ts | 42 +++++++++++++++++++++++++----- tests/unit-ts/docker/boot.test.ts | 40 ++++++++++++++++++++++++++++ tests/unit-ts/docker/store.test.ts | 24 +++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 tests/unit-ts/docker/boot.test.ts diff --git a/src/ts/docker/boot.ts b/src/ts/docker/boot.ts index 65cf004..cb02170 100644 --- a/src/ts/docker/boot.ts +++ b/src/ts/docker/boot.ts @@ -19,6 +19,17 @@ export function isDockerPageEnabled(doc: Document): boolean { return doc.documentElement.dataset.modernuiDocker !== 'off'; } +// True when an incoming snapshot is an empty container list while the store +// already holds containers. docker-state.php returns `containers: []` (still +// HTTP 200) during the window the stock update_container flow rewrites the +// webui-info docker.json that getAllInfo() reads. Accepting it would blank the +// page and poison the SWR cache; resync() skips the overwrite in that case and +// waits for the next snapshot. First load (currentCount === 0) falls through so +// a genuinely empty server still renders the empty state. +export function isTransientEmptySnapshot(incomingCount: number, currentCount: number): boolean { + return incomingCount === 0 && currentCount > 0; +} + // Parse a docker-style size string like "25.34MiB" or "1.5GiB" into bytes. // Matches the format `docker stats --format='{{.MemUsage}}'` produces; MiB/GiB // use 1024 base, MB/GB use 1000 base per Docker CLI conventions. @@ -149,6 +160,19 @@ export async function boot(): Promise { // fresh for running containers regardless. const snapshot = await fetchSnapshot({ withStats: store.getShowStats() }); lastServerUptime = snapshot.serverUptime; + // Guard against a transient empty snapshot clobbering good state. While the + // stock update_container flow recreates a container, it rewrites the + // webui-info docker.json that getAllInfo() reads; for that window + // docker-state.php returns `containers: []` — still HTTP 200, so the catch + // below never fires. Accepting it would blank the page AND poison the SWR + // cache (writeCachedSnapshot), leaving it blank across refreshes until the + // update finished. Keep current state and wait for the next resync (nchan + // delta / update-complete callback) to deliver the real list. + if ( + isTransientEmptySnapshot(snapshot.containers.length, store.getState().containers.length) + ) { + return; + } store.setState({ containers: snapshot.containers, folders: snapshot.folders, @@ -167,11 +191,16 @@ export async function boot(): Promise { // snapshot of the in-flight pull/recreate sequence. The page mounts a panel // that consumes this. // - // The onBatchComplete callback fires on the script's `_DONE_` marker — we - // force-clear the docker store's updating set + refetch so the panel and - // per-row "update available" badges don't sit on stale state if the digest - // cache hasn't caught up yet. Without this fallback, "Working…" would - // linger until the 5-min watchdog fired. + // The onBatchComplete callback fires on the script's `_DONE_` marker. We + // resync THEN clear the updating set — order matters. reconcileUpdating() + // force-clears a recreated container's stale "update available" badge (the + // Unraid digest cache lags after a pull; see store.ts), but only while that + // container's update probe still exists. Clearing probes first — as this used + // to — left reconcile nothing to match, so the freshly-updated badge persisted + // on the lagging cache until a manual page reload. So: resync first (the + // rotated-id snapshot reconciles and clears the badge), then clearAllUpdating + // mops up any probe that didn't self-reconcile (a no-op/failed update) so the + // panel doesn't sit on "Working…" until the 5-min watchdog. const progressStore = createUpdateProgressStore( (image) => { // image string ("linuxserver/plex:latest") → container.name. Match @@ -185,8 +214,7 @@ export async function boot(): Promise { return null; }, () => { - store.clearAllUpdating(); - void resync(); + void resync().then(() => store.clearAllUpdating()); }, ); diff --git a/tests/unit-ts/docker/boot.test.ts b/tests/unit-ts/docker/boot.test.ts new file mode 100644 index 0000000..14004c9 --- /dev/null +++ b/tests/unit-ts/docker/boot.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from 'vitest'; +import { isDockerPageEnabled, isTransientEmptySnapshot } from '../../../src/ts/docker/boot'; + +// Regression: refreshing /Docker while a container update is in flight rendered +// a blank page. docker-state.php returns `containers: []` (HTTP 200) during the +// window the stock update_container flow rewrites webui-info docker.json, and +// resync() overwrote the populated store + SWR cache with that empty list — so +// the page blanked and stayed blank across refreshes until the update finished. +// isTransientEmptySnapshot() is the guard that makes resync() skip that overwrite. +describe('isTransientEmptySnapshot', () => { + it('treats an empty snapshot as transient when the store already has rows', () => { + expect(isTransientEmptySnapshot(0, 43)).toBe(true); + }); + + it('accepts an empty snapshot on first load (store still empty)', () => { + // A genuinely empty server must still render the empty state, not be skipped. + expect(isTransientEmptySnapshot(0, 0)).toBe(false); + }); + + it('always accepts a non-empty snapshot', () => { + expect(isTransientEmptySnapshot(43, 43)).toBe(false); + expect(isTransientEmptySnapshot(1, 0)).toBe(false); + expect(isTransientEmptySnapshot(42, 43)).toBe(false); // a real removal, not empty + }); +}); + +describe('isDockerPageEnabled gate', () => { + it('defaults ON when the attribute is absent', () => { + delete document.documentElement.dataset.modernuiDocker; + expect(isDockerPageEnabled(document)).toBe(true); + }); + + it('returns false only when explicitly "off"', () => { + document.documentElement.dataset.modernuiDocker = 'off'; + expect(isDockerPageEnabled(document)).toBe(false); + document.documentElement.dataset.modernuiDocker = 'on'; + expect(isDockerPageEnabled(document)).toBe(true); + delete document.documentElement.dataset.modernuiDocker; + }); +}); diff --git a/tests/unit-ts/docker/store.test.ts b/tests/unit-ts/docker/store.test.ts index e07c1c2..46eac0e 100644 --- a/tests/unit-ts/docker/store.test.ts +++ b/tests/unit-ts/docker/store.test.ts @@ -408,6 +408,30 @@ describe('updating state', () => { expect(store.getState().containers[0].updateAvailable).toBe(false); }); + it('loses the badge force-clear if probes are cleared BEFORE the reconciling snapshot', () => { + // Ordering contract for boot.ts onBatchComplete (the stuck-badge bug): the + // id-rotation force-clear only runs while the update probe still exists. + // clearAllUpdating() before the resync's setState() deletes the probe, so + // reconcileUpdating() has nothing to match and the badge falls back to the + // snapshot's stale updateAvailable=true. onBatchComplete must therefore + // resync FIRST, then clear. This test pins that dependency so a future + // reorder back to clear-then-resync fails loudly. + const store = createDockerStore(); + const s = sampleState(); + s.containers[0] = mkContainer({ name: 'plex', id: 'old-id', updateAvailable: true }); + store.setState(s); + store.markUpdating(['plex']); + + // Wrong order: wipe probes before the recreated snapshot arrives. + store.clearAllUpdating(); + const next = sampleState(); + next.containers[0] = mkContainer({ name: 'plex', id: 'new-id', updateAvailable: true }); + store.setState(next); + + // No probe survived, so the lagging-cache badge persists — the bug. + expect(store.getState().containers[0].updateAvailable).toBe(true); + }); + it('clears entries when updateAvailable flips true→false (id unchanged path)', () => { const store = createDockerStore(); const s = sampleState(); From 3900ed8848f9f58c95037a6de9e8730b0812a4f7 Mon Sep 17 00:00:00 2001 From: Inch-high Date: Tue, 2 Jun 2026 19:15:04 +0100 Subject: [PATCH 2/2] fix(docker): move transient-empty guard server-side; fix delete-all ghost The blank-page guard from the prior commit lived in boot.ts as isTransientEmptySnapshot(): an empty snapshot was suppressed whenever the store already held rows. That heuristic couldn't tell a transient mid-update empty from a genuinely empty server, so deleting every container left ghost rows on screen until a manual reload. Move the disambiguation server-side, where the authoritative signal exists. docker-state.php now compares getAllInfo() (reads the webui-info docker.json the stock update_container flow rewrites) against getDockerContainers() (hits the docker socket directly, unaffected by that rewrite). When info is empty but the daemon still lists containers, it's a mid-rewrite window -> respond 503. fetchSnapshot() throws on the non-200 and resync()'s existing catch keeps current state + skips the SWR cache write, then retries on the next resync. A genuinely empty server (daemon also empty) returns 200 with [] and renders the empty state. This is trigger-agnostic (stock UI, CA Auto Update, or the modernui page) because it keys off the daemon, not the modernui update probes. Removed isTransientEmptySnapshot() + its boot.test.ts cases; added modernui_is_transient_empty() with docker-state.test.php coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- package/include/docker-state.php | 50 +++++++++++++++++++++++--- src/ts/docker/boot.ts | 31 ++++------------ tests/unit-php/docker-state.test.php | 53 ++++++++++++++++++++++++++++ tests/unit-ts/docker/boot.test.ts | 31 +++++----------- 4 files changed, 114 insertions(+), 51 deletions(-) create mode 100644 tests/unit-php/docker-state.test.php diff --git a/package/include/docker-state.php b/package/include/docker-state.php index 7d90115..3771450 100644 --- a/package/include/docker-state.php +++ b/package/include/docker-state.php @@ -27,13 +27,33 @@ require_once MODERNUI_DOCKER_CLIENT; // populates $dockerManPaths + $driver } -function modernui_docker_state(bool $withStats = false): array +// True when getAllInfo() returned no rows while the docker daemon still lists +// containers — i.e. the stock update_container flow is mid-rewrite of the +// webui-info docker.json that getAllInfo() reads. getDockerContainers() hits +// the docker socket directly and is unaffected by that rewrite, so it's the +// source of truth: an empty info list alongside a non-empty daemon means +// "transient rewrite", not "genuinely empty server". The caller responds 503 +// for the transient case so the client keeps its current state and retries, +// instead of blanking the page on a momentarily-empty snapshot. +function modernui_is_transient_empty(int $infoCount, int $daemonCount): bool +{ + return $infoCount === 0 && $daemonCount > 0; +} + +// $transient is set true (by-ref) when the snapshot is a transient mid-rewrite +// empty (see modernui_is_transient_empty). On that path the returned array is +// empty and the caller should respond 503 rather than serve it. +function modernui_docker_state(bool $withStats = false, bool &$transient = false): array { global $dockerManPaths, $driver; // bound from DockerClient.php's top-level scope + $transient = false; + modernui_maybe_migrate_folder_view2(); $containers = []; + $infoCount = 0; + $daemonCount = 0; if (class_exists('DockerTemplates') && class_exists('DockerClient')) { try { // Two data sources to merge: @@ -41,7 +61,8 @@ function modernui_docker_state(bool $withStats = false): array // getDockerContainers() → docker socket data (Image, Status, Id, Ports, NetworkMode) // Keyed by container Name in both. $templates = new DockerTemplates(); - $info = $templates->getAllInfo(false, true, false); + $info = (array)$templates->getAllInfo(false, true, false); + $infoCount = count($info); $client = new DockerClient(); $rawContainers = $client->getDockerContainers(); @@ -51,6 +72,7 @@ function modernui_docker_state(bool $withStats = false): array $byName[$ct['Name']] = $ct; } } + $daemonCount = count($byName); // Sizes + live CPU/RAM are both expensive (sizes walks each // container's RW layer; `docker stats --no-stream` blocks ~1s for @@ -61,7 +83,7 @@ function modernui_docker_state(bool $withStats = false): array $sizes = $withStats ? modernui_fetch_sizes($client) : []; $stats = $withStats ? modernui_fetch_docker_stats() : []; - foreach ((array)$info as $name => $row) { + foreach ($info as $name => $row) { $container = $byName[$name] ?? []; $shortId = substr((string)($container['Id'] ?? ''), 0, 12); $vdisk = $sizes[$shortId] ?? null; @@ -74,6 +96,15 @@ function modernui_docker_state(bool $withStats = false): array } } + // Mid-rewrite of the webui-info docker.json: bail before reading folders/tags + // and let the caller serve a 503. The client treats that as "keep current + // state, retry on the next resync" rather than accepting an empty list. A + // genuinely empty server (daemon also empty) falls through and returns []. + if (modernui_is_transient_empty($infoCount, $daemonCount)) { + $transient = true; + return []; + } + $folders = modernui_read_folders(); $tags = modernui_read_tags(); @@ -269,5 +300,16 @@ function modernui_normalize_ports(array $rawPorts, string $url): array // ?stats=1 opts into the expensive size + docker-stats fetches. The // front-end only sends it when the Stats pill is on. $withStats = isset($_GET['stats']) && $_GET['stats'] === '1'; - modernui_json_response(modernui_docker_state($withStats)); + $transient = false; + $snapshot = modernui_docker_state($withStats, $transient); + if ($transient) { + // Mid-rewrite of the webui-info docker.json by the stock update flow: + // the container list is momentarily empty though the daemon still has + // them. 503 tells the client to keep its current state and retry — + // fetchSnapshot() throws on the non-200 and resync()'s catch swallows + // it. See modernui_is_transient_empty. + modernui_json_response(['error' => 'transient', 'retry' => true], 503); + } else { + modernui_json_response($snapshot); + } } diff --git a/src/ts/docker/boot.ts b/src/ts/docker/boot.ts index cb02170..2345987 100644 --- a/src/ts/docker/boot.ts +++ b/src/ts/docker/boot.ts @@ -19,17 +19,6 @@ export function isDockerPageEnabled(doc: Document): boolean { return doc.documentElement.dataset.modernuiDocker !== 'off'; } -// True when an incoming snapshot is an empty container list while the store -// already holds containers. docker-state.php returns `containers: []` (still -// HTTP 200) during the window the stock update_container flow rewrites the -// webui-info docker.json that getAllInfo() reads. Accepting it would blank the -// page and poison the SWR cache; resync() skips the overwrite in that case and -// waits for the next snapshot. First load (currentCount === 0) falls through so -// a genuinely empty server still renders the empty state. -export function isTransientEmptySnapshot(incomingCount: number, currentCount: number): boolean { - return incomingCount === 0 && currentCount > 0; -} - // Parse a docker-style size string like "25.34MiB" or "1.5GiB" into bytes. // Matches the format `docker stats --format='{{.MemUsage}}'` produces; MiB/GiB // use 1024 base, MB/GB use 1000 base per Docker CLI conventions. @@ -158,21 +147,15 @@ export async function boot(): Promise { // Only request stats when the toggle is on — saves a second+ on // every fetch when stats aren't shown. nchan deltas keep CPU/RAM // fresh for running containers regardless. + // docker-state.php returns 503 during the window the stock update_container + // flow rewrites the webui-info docker.json that getAllInfo() reads (the + // container list is momentarily empty though the daemon still has them). + // fetchSnapshot() throws on that non-200, so the catch below keeps the + // current state and we retry on the next resync — instead of blanking the + // page and poisoning the SWR cache with an empty list. A genuinely empty + // server returns 200 with an empty list and renders the empty state. const snapshot = await fetchSnapshot({ withStats: store.getShowStats() }); lastServerUptime = snapshot.serverUptime; - // Guard against a transient empty snapshot clobbering good state. While the - // stock update_container flow recreates a container, it rewrites the - // webui-info docker.json that getAllInfo() reads; for that window - // docker-state.php returns `containers: []` — still HTTP 200, so the catch - // below never fires. Accepting it would blank the page AND poison the SWR - // cache (writeCachedSnapshot), leaving it blank across refreshes until the - // update finished. Keep current state and wait for the next resync (nchan - // delta / update-complete callback) to deliver the real list. - if ( - isTransientEmptySnapshot(snapshot.containers.length, store.getState().containers.length) - ) { - return; - } store.setState({ containers: snapshot.containers, folders: snapshot.folders, diff --git a/tests/unit-php/docker-state.test.php b/tests/unit-php/docker-state.test.php new file mode 100644 index 0000000..628a51e --- /dev/null +++ b/tests/unit-php/docker-state.test.php @@ -0,0 +1,53 @@ + { - it('treats an empty snapshot as transient when the store already has rows', () => { - expect(isTransientEmptySnapshot(0, 43)).toBe(true); - }); - - it('accepts an empty snapshot on first load (store still empty)', () => { - // A genuinely empty server must still render the empty state, not be skipped. - expect(isTransientEmptySnapshot(0, 0)).toBe(false); - }); - - it('always accepts a non-empty snapshot', () => { - expect(isTransientEmptySnapshot(43, 43)).toBe(false); - expect(isTransientEmptySnapshot(1, 0)).toBe(false); - expect(isTransientEmptySnapshot(42, 43)).toBe(false); // a real removal, not empty - }); -}); +// The transient-empty-snapshot guard that used to live here moved server-side: +// docker-state.php now returns 503 (not 200 + `containers: []`) while the stock +// update_container flow rewrites webui-info docker.json, so resync()'s catch +// keeps current state. See modernui_is_transient_empty in docker-state.php and +// docker-state.test.php. Removing the client heuristic also fixes the inverse +// bug — a genuinely empty server (all containers deleted) now renders the empty +// state instead of being suppressed as "transient". describe('isDockerPageEnabled gate', () => { it('defaults ON when the attribute is absent', () => {