From 4ed7954f84d80a7c9547eb30e248fc8788ea2abf Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Wed, 5 Aug 2026 10:30:10 -0500 Subject: [PATCH] =?UTF-8?q?Drop=20the=20user-agent=20fallback=20=E2=80=94?= =?UTF-8?q?=20it=20could=20never=20fire?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isBrightSignDevice() fell back to device.user_agent to catch panels paired before this port existed, which registered as "Chrome 120" with a BrightSign user agent. `devices` has no user_agent column, so the field is always undefined on a row read from the database. The branch was unreachable in production and passed only in a test that fabricated the field — which is precisely how dead code survives review. Two agents flagged it independently while working on unrelated areas, and the schema confirms it: zero matches for user_agent in the devices table. Those pre-port panels are recognised the moment they re-register on a build carrying the host, which every one of them gets on its next update. Identifying them sooner would mean persisting the user agent, and a column added solely to track a population that disappears on its own is not worth carrying. The test now asserts the honest behaviour: a fabricated user_agent does NOT create a match, and a group containing such a panel reads as mixed until it re-registers. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- frontend/js/views/device-detail.js | 5 ++--- server/lib/sync-backend.js | 20 +++++++++++++------- server/test/sync-backend.test.js | 12 ++++++++---- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/frontend/js/views/device-detail.js b/frontend/js/views/device-detail.js index fa2a917..8ee9b96 100644 --- a/frontend/js/views/device-detail.js +++ b/frontend/js/views/device-detail.js @@ -89,9 +89,8 @@ function renderDeviceClock(device) { // covers panels paired before that existed, which registered as "Chrome 120" with a BrightSign UA. function isBrightSignDevice(device) { if (!device) return false; - const platform = String(device.platform || '').toLowerCase(); - if (platform.includes('brightsign')) return true; - return String(device.user_agent || '').toLowerCase().includes('brightsign'); + // platform only: `devices` has no user_agent column, so a fallback on it could never fire. + return String(device.platform || '').toLowerCase().includes('brightsign'); } export function render(container, deviceId) { diff --git a/server/lib/sync-backend.js b/server/lib/sync-backend.js index e77336b..e4858fd 100644 --- a/server/lib/sync-backend.js +++ b/server/lib/sync-backend.js @@ -24,16 +24,22 @@ const BACKENDS = ['auto', 'screentinker', 'brightsign']; /* - * A device is a BrightSign if it said so. The player sends ?platform=brightsign (autorun.brs - * puts it there), which lands in devices.platform. The UA fallback covers players paired before - * the port existed — those registered a platform of "Chrome 120" with a BrightSign UA. + * A device is a BrightSign if it said so: the player sends ?platform=brightsign (autorun.brs puts + * it there), which lands in devices.platform. + * + * There is deliberately NO user-agent fallback. An earlier version had one, to catch panels paired + * before this port existed — which registered as "Chrome 120" with a BrightSign user agent. It + * could never fire: `devices` has no user_agent column, so the field is always undefined on a row + * read from the database. It read as defensive and was dead code. + * + * Those pre-port panels are identified the moment they re-register on a build that carries the + * host, which every one of them gets on its next update. Recognising them earlier would mean + * persisting the user agent, and a column added solely to identify a population that disappears on + * its own is not worth carrying. */ function isBrightSignDevice(device) { if (!device) return false; - const platform = String(device.platform || '').toLowerCase(); - if (platform.includes('brightsign')) return true; - const ua = String(device.user_agent || '').toLowerCase(); - return ua.includes('brightsign'); + return String(device.platform || '').toLowerCase().includes('brightsign'); } /* diff --git a/server/test/sync-backend.test.js b/server/test/sync-backend.test.js index 8e12216..d171596 100644 --- a/server/test/sync-backend.test.js +++ b/server/test/sync-backend.test.js @@ -66,11 +66,15 @@ test('unknown or missing settings read as auto rather than throwing', () => { assert.equal(resolveSyncBackend('auto', null).backend, 'screentinker'); }); -test('a player paired before the port is still recognised by its user agent', () => { - // Both of giyokun's devices registered platform "Chrome 120" with a BrightSign UA. +test('a pre-port panel is NOT recognised until it re-registers — no phantom user-agent match', () => { + // Panels paired before this port registered as "Chrome 120" with a BrightSign user agent, and an + // earlier version tried to catch them that way. It could never work: `devices` has no user_agent + // column, so the field is always undefined on a row read from the database — the check passed + // only in tests that fabricated it, which is exactly how dead code survives. const legacy = { id: 'old', platform: 'Chrome 120', user_agent: 'BrightSign/9.1.92.2 (HD1026) Chrome/120' }; - assert.equal(isBrightSignDevice(legacy), true); - assert.equal(resolveSyncBackend('auto', [legacy, bs(2)]).backend, 'brightsign'); + assert.equal(isBrightSignDevice(legacy), false, 'a fabricated user_agent must not create a match'); + assert.equal(resolveSyncBackend('auto', [legacy, bs(2)]).backend, 'screentinker', + 'so a group containing one reads as mixed until that panel re-registers as brightsign'); }); test('a non-BrightSign device is never mistaken for one', () => {