Archive/screentinker
1
0
Fork 0
mirror of https://github.com/screentinker/screentinker.git synced 2026-08-13 22:03:13 -06:00
Code Issues Actions 10 Packages Projects Releases Wiki Activity

Drop the user-agent fallback — it could never fire

Browse source 
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
ScreenTinker 2026-08-05 10:30:10 -05:00
parent 039511b988
commit 4ed7954f84
3 changed files with 23 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
frontend/js/views/device-detail.js
View file

@ -89,9 +89,8 @@ function renderDeviceClock(device) {
// covers panels paired before that existed, which registered as "Chrome 120" with a BrightSign UA. // covers panels paired before that existed, which registered as "Chrome 120" with a BrightSign UA.
function isBrightSignDevice(device) { function isBrightSignDevice(device) {
if (!device) return false; if (!device) return false;
const platform = String(device.platform || '').toLowerCase(); // platform only: `devices` has no user_agent column, so a fallback on it could never fire.
if (platform.includes('brightsign')) return true; return String(device.platform || '').toLowerCase().includes('brightsign');
return String(device.user_agent || '').toLowerCase().includes('brightsign');
} }
export function render(container, deviceId) { export function render(container, deviceId) {

20
server/lib/sync-backend.js
View file

@ -24,16 +24,22 @@
const BACKENDS = ['auto', 'screentinker', 'brightsign']; const BACKENDS = ['auto', 'screentinker', 'brightsign'];
/* /*
* A device is a BrightSign if it said so. The player sends ?platform=brightsign (autorun.brs * A device is a BrightSign if it said so: the player sends ?platform=brightsign (autorun.brs puts
* puts it there), which lands in devices.platform. The UA fallback covers players paired before * it there), which lands in devices.platform.
* the port existed — those registered a platform of "Chrome 120" with a BrightSign UA. *
* 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) { function isBrightSignDevice(device) {
if (!device) return false; if (!device) return false;
const platform = String(device.platform || '').toLowerCase(); return String(device.platform || '').toLowerCase().includes('brightsign');
if (platform.includes('brightsign')) return true;
const ua = String(device.user_agent || '').toLowerCase();
return ua.includes('brightsign');
} }
/* /*

12
server/test/sync-backend.test.js
View file

@ -66,11 +66,15 @@ test('unknown or missing settings read as auto rather than throwing', () => {
assert.equal(resolveSyncBackend('auto', null).backend, 'screentinker'); assert.equal(resolveSyncBackend('auto', null).backend, 'screentinker');
}); });
test('a player paired before the port is still recognised by its user agent', () => { test('a pre-port panel is NOT recognised until it re-registers — no phantom user-agent match', () => {
// Both of giyokun's devices registered platform "Chrome 120" with a BrightSign UA. // 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' }; const legacy = { id: 'old', platform: 'Chrome 120', user_agent: 'BrightSign/9.1.92.2 (HD1026) Chrome/120' };
assert.equal(isBrightSignDevice(legacy), true); assert.equal(isBrightSignDevice(legacy), false, 'a fabricated user_agent must not create a match');
assert.equal(resolveSyncBackend('auto', [legacy, bs(2)]).backend, 'brightsign'); 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', () => { test('a non-BrightSign device is never mistaken for one', () => {