From 0a888910dc1f7a5bf943ab5e99a387507bdb9084 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Wed, 5 Aug 2026 19:34:52 -0500 Subject: [PATCH] Stop claiming offline cache on a runtime that refuses to run a service worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found on alpha after deploying rc4, by comparing what a device advertised against what it actually requested. A real BrightSign XT245 has navigator.serviceWorker, passes an `'serviceWorker' in navigator` check, and then never even fetches sw.js — its widget runtime refuses the registration. It was declaring offline.cache to the fleet while unable to cache a single byte, which is precisely the lie the capability model exists to prevent. The claim is now made on a worker that is actually IN CONTROL, and a refused registration sets a flag so the negative sticks on a runtime where it will never succeed. That failure previously went to console.warn, on a display nobody has a console for, so a panel that could cache nothing looked identical to one that could. It now reports app_error/sw_unavailable — as an allow-listed event type, since an unknown one is dropped by the server and would have been just as invisible. The cost is that the first load under-reports, before the worker claims the page. That is the right direction to be wrong in, and it self-corrects: the next register sends the true set. Also corrects docs/player-parity.md, which claimed BrightSign simply inherits the web player's service worker. The failing unit runs BSN's Supervisor rather than our brightsign/autorun.brs, and Supervisor's widget has no storage_path — the setting our own host script does configure and the precondition for a widget having persistent storage. So this is likely a widget config issue rather than a platform limit, but it is UNVERIFIED on hardware and the doc now says so. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- docs/player-parity.md | 16 +++++++- server/player/index.html | 37 ++++++++++++++++--- server/player/sw.js | 5 ++- .../test/player-declared-capabilities.test.js | 19 ++++++++-- 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/docs/player-parity.md b/docs/player-parity.md index 8df58fa..1380573 100644 --- a/docs/player-parity.md +++ b/docs/player-parity.md @@ -75,7 +75,7 @@ privilege model exists on those platforms — so the column is collapsed. |---|---|---|---|---| | `sync.clock` | ✅ | ✅ | ✅ | ✅ | | `sync.native` | ❌ no native protocol | ❌ | ❌ | ⚠️ SyncManager, BOS 8.2.10+; multicast so all members must share one L2 network | -| `offline.cache` | ✅ content downloaded to disk, **resumable** (Range + If-Range), revision-keyed | ✅ service worker, **resumable chunked prefetch**, revision-keyed | ✅ **media cached to `wgt-private`** (`js/media-cache.js`), resumable, revision-keyed — declared at runtime, since a build with no writable private storage must not claim it | ✅ inherits the web player's service worker | +| `offline.cache` | ✅ content downloaded to disk, **resumable** (Range + If-Range), revision-keyed | ✅ service worker, **resumable chunked prefetch**, revision-keyed | ✅ **media cached to `wgt-private`** (`js/media-cache.js`), resumable, revision-keyed — declared at runtime | ⚠️ **depends on the host widget's storage config — see below** | --- @@ -91,6 +91,20 @@ Ordered by how visible the failure is to an operator. across attempts instead of restarting from zero, and revision-keyed, so a replaced asset is still a miss. The capability is declared at runtime rather than assumed: a build that cannot write to private storage keeps quiet about it. +3. **BrightSign offline caching is NOT automatic — it depends on who created the widget.** A real + XT245 on alpha exposes `navigator.serviceWorker`, and then never even fetches `sw.js`: + registration is refused, so there is no worker, no content cache and no offline playback. That + unit is running **BSN's Supervisor** (`autorun.createdby = Supervisor 2.1.18.3`) rather than our + `brightsign/autorun.brs`, and Supervisor's widget has no `storage_path` — the setting our own + host script does set (`storage_path: "/cache"`, `storage_quota: "1073741824"`), and the + precondition for a widget having persistent storage at all. So this is very likely a widget + CONFIG issue rather than a platform limit, but **it is unverified on hardware**: nobody has yet + watched a player running our package register a worker. + + The player no longer lies about it either way — `offline.cache` is declared only when a worker + is genuinely in control, and a refused registration reports `app_error/sw_unavailable` to the + server instead of a `console.warn` on a display nobody has a console for. + 3. **BrightSign `remote.screenshot` needs primary storage.** Reachable today only via the canvas fallback, which cannot read the video plane, so screenshots show everything except the video. Resolves itself when a card or SSD is fitted. diff --git a/server/player/index.html b/server/player/index.html index 9f0f80b..bbf016c 100644 --- a/server/player/index.html +++ b/server/player/index.html @@ -497,6 +497,10 @@ // can't see SSID/RSSI, so we send only offline_ms + link_lost + cold_start:false. let disconnectedAt = 0; // Date.now() at the first disconnect of the current gap (0 = not in a gap) let linkLostDuringGap = false; // navigator went offline at any point during the gap + // Set when register() is REFUSED (not merely pending). A runtime can expose navigator + // .serviceWorker and still decline to run one — a real BrightSign widget does exactly that — + // and the display must stop claiming an offline capability it cannot honour. + let swRegistrationFailed = false; // feat/offline-cause-log: typed incident feed (device:event) — server inserts a device_events row. // Best-effort + auth-guarded (the reconnected socket is authenticated by the time we emit). function emitDeviceEvent(type, reason, detail) { @@ -1723,11 +1727,21 @@ } } catch (e) { /* bundle absent: hard cuts, and we do not claim the capability */ } - // Offline caching is the service worker. Reported on support rather than on an active - // controller: the first load registers it and has no controller yet, and a display that - // re-registers on every boot would otherwise flap this capability on and off. + // Offline caching is the service worker — and the API EXISTING is not the same as it + // working. A real BrightSign XT245 on alpha has `serviceWorker` in navigator, passes this + // check, and then never even fetches sw.js: registration is refused by its widget runtime. + // It declared offline.cache to the fleet and could not cache a single byte. + // + // So the claim is made on a worker that is actually IN CONTROL. The cost is that the very + // first load under-reports (registration has happened but the worker has not claimed the + // page yet) — which is the right direction to be wrong in, and self-corrects: activation + // triggers a reload, and the next register sends the true set. `swRegistrationFailed` makes + // the negative stick on a runtime where it will never succeed, rather than waiting on a + // controller that is never coming. try { - if ('serviceWorker' in navigator) caps.push('offline.cache'); + if (!swRegistrationFailed && navigator.serviceWorker && navigator.serviceWorker.controller) { + caps.push('offline.cache'); + } } catch (e) { /* locked-down browser */ } // Screenshots need somewhere to draw. Same-origin content and a 2d context are the real @@ -4071,7 +4085,20 @@ }); } }); - }, (err) => console.warn('SW registration failed:', err)); + }, (err) => { + // A registration that fails has to be VISIBLE. This one went to console.warn on a display + // nobody has a console for, so a panel that could not cache anything looked identical to + // one that could — for as long as nobody thought to compare nginx logs against the + // capability it was advertising. + swRegistrationFailed = true; + console.warn('SW registration failed:', err); + try { + // 'app_error' rather than a new type: the server allow-lists event types, and a type it + // does not know is dropped silently — which would have made this report as invisible as + // the console.warn it replaces. + emitDeviceEvent('app_error', 'sw_unavailable', String((err && err.message) || err).slice(0, 200)); + } catch (e) { /* reporting must never break the player */ } + }); } // ==================== Keyboard shortcuts ==================== diff --git a/server/player/sw.js b/server/player/sw.js index 32be80a..806f8a2 100644 --- a/server/player/sw.js +++ b/server/player/sw.js @@ -1,3 +1,6 @@ +// v23: offline.cache is claimed only when a worker is actually IN CONTROL — a real BrightSign +// widget exposes navigator.serviceWorker, refuses to register one, and was advertising the +// capability to the fleet regardless. // v22: worker scope widened to '/' (it never controlled /player before) + prune-to-playlist, so a // replaced asset's superseded copy is reclaimed rather than waiting on the quota. // v21: chunked resumable content prefetch + revision-keyed media URLs — index.html gained @@ -8,7 +11,7 @@ // — a player then ran a new index.html against a stale st-bridge.js and threw on every heartbeat. // Bump whenever a shipped /player asset changes shape; content lives in its own cache, so this // costs a small re-download and never re-fetches the playlist. -const CACHE_NAME = 'rd-player-v22'; +const CACHE_NAME = 'rd-player-v23'; // Content lives in its own cache so the shell can be re-versioned (the activate handler deletes // every cache that is not CACHE_NAME) WITHOUT throwing away megabytes of media that are still // perfectly valid. Rolling the shell used to mean a player re-downloaded its entire playlist. diff --git a/server/test/player-declared-capabilities.test.js b/server/test/player-declared-capabilities.test.js index cba2d50..1d15a1d 100644 --- a/server/test/player-declared-capabilities.test.js +++ b/server/test/player-declared-capabilities.test.js @@ -21,7 +21,7 @@ const { CAP_SET } = require('../lib/player-capabilities'); const HTML = fs.readFileSync(path.join(__dirname, '..', 'player', 'index.html'), 'utf8'); /** Pull declaredCapabilities() out of the player and run it in a controlled world. */ -function declare({ host = false, sync = false, transitions = false, canvas = true, sw = true } = {}) { +function declare({ host = false, sync = false, transitions = false, canvas = true, sw = true, swRefused = false } = {}) { const start = HTML.indexOf(' function declaredCapabilities() {'); assert.notEqual(start, -1, 'declaredCapabilities() must exist in the player'); let depth = 0, end = -1; @@ -33,7 +33,9 @@ function declare({ host = false, sync = false, transitions = false, canvas = tru const sandbox = { console: { log() {}, warn() {} }, - navigator: sw ? { serviceWorker: {} } : {}, + // `sw` now means "a worker is IN CONTROL", not merely "the API exists" — those are different + // things, and a real BrightSign widget is the difference (see the test below). + navigator: sw ? { serviceWorker: { controller: {} } } : (swRefused ? { serviceWorker: {} } : {}), document: { createElement: () => (canvas ? { width: 0, height: 0, getContext: () => ({ drawImage() {} }), toDataURL: () => 'data:,' } @@ -41,6 +43,7 @@ function declare({ host = false, sync = false, transitions = false, canvas = tru }, BS: host ? { hasHost: () => true } : null, }; + sandbox.swRegistrationFailed = swRefused; sandbox.window = sandbox; if (sync) sandbox.ScreenTinkerBSSync = { available: () => true }; // The real runtime globals the player's own transitionRuntimeReady() checks. An earlier draft @@ -101,11 +104,21 @@ test('screenshots are claimed only when there is something to draw on', () => { assert.ok(!declare({ canvas: false }).includes('remote.screenshot')); }); -test('offline cache follows service-worker support', () => { +test('offline cache follows a worker that is actually IN CONTROL', () => { assert.ok(declare({ sw: true }).includes('offline.cache')); assert.ok(!declare({ sw: false }).includes('offline.cache')); }); +test('THE BRIGHTSIGN CASE: the API exists, no worker controls the page, so no claim', () => { + // Found on real hardware. A BrightSign XT245 on alpha has navigator.serviceWorker, passes an + // `'serviceWorker' in navigator` check, and then never even fetches sw.js — its widget runtime + // refuses to register one. It was advertising offline.cache to the fleet while being unable to + // cache a single byte, which is precisely the lie this whole capability model exists to stop. + const caps = declare({ swRefused: true }); + assert.ok(!caps.includes('offline.cache'), 'a runtime that will not run a worker must not claim to cache'); + assert.ok(caps.includes('playback.video'), 'and it must still declare what it genuinely can do'); +}); + test('transitions are declared only when the bundle actually loaded', () => { // It is a progressive enhancement — a failed load means hard cuts, not a broken player. assert.ok(!declare({ transitions: false }).includes('playback.transitions'));