From b0c1d0ab8ddac6ec67c0a4cf9a3aa0a85e290dbd Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Thu, 2 Apr 2026 04:46:50 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20remoteId=3D0=20treated=20as=20falsy=20?= =?UTF-8?q?=E2=80=94=20use=20null=20checks=20instead=20of=20||=20for=20int?= =?UTF-8?q?eger=20remoteId?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit remoteId is an integer index (0, 1, 2...) returned by /api/federation/sessions. In JavaScript, 0 is falsy, so all || '' patterns converted remoteId=0 to '' — making the first remote device (ALIENWARE, index 0) appear as a local session. The symptom: clicking an ALIENWARE session from spark-1 or spark-2 sent POST /api/sessions/{name}/connect ← local endpoint, session doesn't exist → 404 instead of POST /api/federation/0/connect/{name} ← correct federation proxy route Only affected the FIRST remote (index 0). spark-1 ↔ spark-2 worked because each sees the other as remoteId=1. Locations fixed in frontend/app.js: 1. buildTileHTML: session.remoteId ? → session.remoteId != null ? 2. buildSidebarHTML: session.remoteId || '' → session.remoteId != null ? ... : '' 3. _previewClickHandler: session && session.remoteId || '' → null-safe ternary 4. openSession _viewingRemoteId: opts.remoteId || '' → null-safe ternary 5. openSession _remoteId: opts.remoteId || '' → null-safe ternary 6. openSession routing: if (_remoteId) → if (_remoteId !== '') 7. closeSession DELETE guard: if (!_viewingRemoteId) → if (_viewingRemoteId === '') DOM dataset reads (tile.dataset.remoteId, item.dataset.remoteId) are unaffected because dataset values are always strings — '0' is truthy, so || '' worked there. Tests added: 5 new tests covering all four code paths (buildTileHTML, buildSidebarHTML, openSession connect routing, closeSession DELETE skip) with integer remoteId=0. Total: 291 → 296 tests, 0 failures. --- muxplex/frontend/app.js | 14 ++-- muxplex/frontend/tests/test_app.mjs | 114 ++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 7 deletions(-) diff --git a/muxplex/frontend/app.js b/muxplex/frontend/app.js index a6fb0bf..0ff0d1d 100644 --- a/muxplex/frontend/app.js +++ b/muxplex/frontend/app.js @@ -434,7 +434,7 @@ function buildTileHTML(session, index, mobile) { } const lastLines = allLines.slice(_lineCount).join('\n'); - const remoteIdAttr = session.remoteId ? ` data-remote-id="${escapeHtml(session.remoteId)}"` : ''; + const remoteIdAttr = session.remoteId != null ? ` data-remote-id="${escapeHtml(session.remoteId)}"` : ''; return ( `
` + `
` + @@ -489,7 +489,7 @@ function buildSidebarHTML(session, currentSession) { const lastLines = allLines.slice(-20).join('\n'); return ( - `
` + + `
` + `
` + `${escapedName}` + badgeHtml + @@ -873,7 +873,7 @@ function _previewClickHandler(e) { hidePreview(); if (name) { var session = _currentSessions && _currentSessions.find(function(s) { return s.name === name; }); - openSession(name, { remoteId: session && session.remoteId || '' }); + openSession(name, { remoteId: (session != null && session.remoteId != null) ? session.remoteId : '' }); } } @@ -1135,7 +1135,7 @@ function updateFaviconBadge() { async function openSession(name, opts = {}) { hidePreview(); _viewingSession = name; - _viewingRemoteId = opts.remoteId || ''; + _viewingRemoteId = opts.remoteId != null ? opts.remoteId : ''; _viewMode = 'fullscreen'; // Pre-render sidebar with current sessions before first poll tick @@ -1202,9 +1202,9 @@ async function openSession(name, opts = {}) { if (fab) fab.classList.add('hidden'); // Always spawn ttyd for this session — ensures correct session after service restart or page restore - var _remoteId = opts.remoteId || ''; + var _remoteId = opts.remoteId != null ? opts.remoteId : ''; try { - if (_remoteId) { + if (_remoteId !== '') { // Remote session: route connect POST through same-origin federation proxy await api('POST', '/api/federation/' + encodeURIComponent(_remoteId) + '/connect/' + encodeURIComponent(name)); } else { @@ -1233,7 +1233,7 @@ function closeSession() { if (window._closeTerminal) window._closeTerminal(); // Fire-and-forget DELETE — skip for remote sessions (they don't need to know we stopped watching) - if (!_viewingRemoteId) { + if (_viewingRemoteId === '') { api('DELETE', '/api/sessions/current').catch(function() {}); } _viewingRemoteId = ''; diff --git a/muxplex/frontend/tests/test_app.mjs b/muxplex/frontend/tests/test_app.mjs index 25e7695..fb44ad3 100644 --- a/muxplex/frontend/tests/test_app.mjs +++ b/muxplex/frontend/tests/test_app.mjs @@ -3956,3 +3956,117 @@ test('index.html loads all 5 xterm JS scripts from local /vendor/ paths (not CDN 'index.html must NOT reference cdn.jsdelivr.net', ); }); + +// --- remoteId=0 falsy-zero bug fixes --- +// remoteId is an integer index (0, 1, 2...). When remoteId=0, all || '' patterns +// evaluate to '' because 0 is falsy in JS — causing the first remote to be treated +// as a local session (404 on connect). + +test('buildTileHTML with integer remoteId=0 includes data-remote-id="0" attribute', () => { + const session = { name: 'alienware-session', remoteId: 0, snapshot: '' }; + const html = app.buildTileHTML(session, 0, false); + assert.ok( + html.includes('data-remote-id="0"'), + 'buildTileHTML must emit data-remote-id="0" when session.remoteId === 0 (not omit it)', + ); +}); + +test('buildSidebarHTML with integer remoteId=0 includes data-remote-id="0" (not empty)', () => { + const session = { name: 'alienware-session', remoteId: 0, snapshot: '', bell: { unseen_count: 0 } }; + const html = app.buildSidebarHTML(session, ''); + assert.ok( + html.includes('data-remote-id="0"'), + 'buildSidebarHTML must emit data-remote-id="0" when session.remoteId === 0', + ); + assert.ok( + !html.includes('data-remote-id=""'), + 'buildSidebarHTML must NOT emit data-remote-id="" when session.remoteId === 0', + ); +}); + +test('openSession with integer remoteId=0 POSTs to federation proxy URL, not local', async () => { + const fetchCalls = []; + const origFetch = globalThis.fetch; + const origGetById = globalThis.document.getElementById; + const origQS = globalThis.document.querySelector; + const origSetTimeout = globalThis.setTimeout; + globalThis.fetch = async (url, opts) => { fetchCalls.push({ url, opts }); return { ok: true }; }; + globalThis.document.getElementById = () => ({ textContent: '', style: {}, classList: { remove: () => {}, add: () => {} } }); + globalThis.document.querySelector = () => null; + globalThis.setTimeout = () => {}; + globalThis.window._openTerminal = () => {}; + + // remoteId is the integer 0 — as returned by /api/federation/sessions for the first remote + await app.openSession('muxplex-updates', { remoteId: 0 }); + + const federationCall = fetchCalls.find((c) => c.url === '/api/federation/0/connect/muxplex-updates'); + const localCall = fetchCalls.find((c) => c.url === '/api/sessions/muxplex-updates/connect'); + + assert.ok(federationCall, 'should POST to /api/federation/0/connect/muxplex-updates (not local endpoint)'); + assert.ok(!localCall, 'must NOT POST to /api/sessions/muxplex-updates/connect (local endpoint gives 404)'); + assert.strictEqual(federationCall.opts.method, 'POST'); + + globalThis.fetch = origFetch; + globalThis.document.getElementById = origGetById; + globalThis.document.querySelector = origQS; + globalThis.setTimeout = origSetTimeout; +}); + +test('openSession with integer remoteId=0 passes 0 to window._openTerminal as second arg', async () => { + let openTerminalArgs = null; + const origFetch = globalThis.fetch; + const origGetById = globalThis.document.getElementById; + const origQS = globalThis.document.querySelector; + const origSetTimeout = globalThis.setTimeout; + globalThis.fetch = async () => ({ ok: true }); + globalThis.document.getElementById = () => ({ textContent: '', style: {}, classList: { remove: () => {}, add: () => {} } }); + globalThis.document.querySelector = () => null; + globalThis.setTimeout = (fn) => { fn(); }; + globalThis.window._openTerminal = (...args) => { openTerminalArgs = args; }; + + await app.openSession('muxplex-updates', { remoteId: 0 }); + + assert.ok(openTerminalArgs !== null, '_openTerminal should have been called'); + assert.strictEqual(openTerminalArgs[0], 'muxplex-updates', '_openTerminal first arg should be session name'); + assert.ok(openTerminalArgs[1] === 0 || openTerminalArgs[1] === '0', '_openTerminal second arg should be remoteId 0'); + + globalThis.fetch = origFetch; + globalThis.document.getElementById = origGetById; + globalThis.document.querySelector = origQS; + globalThis.setTimeout = origSetTimeout; +}); + +test('closeSession after openSession with remoteId=0 does NOT fire DELETE /api/sessions/current', async () => { + const origFetch = globalThis.fetch; + const origGetById = globalThis.document.getElementById; + const origQS = globalThis.document.querySelector; + const origSetTimeout = globalThis.setTimeout; + + // Open a remote session with remoteId=0 — sets _viewingRemoteId = 0 + globalThis.fetch = async () => ({ ok: true }); + globalThis.document.getElementById = () => ({ textContent: '', style: {}, classList: { remove: () => {}, add: () => {} } }); + globalThis.document.querySelector = () => null; + globalThis.setTimeout = () => {}; + globalThis.window._openTerminal = () => {}; + globalThis.window._closeTerminal = () => {}; + + await app.openSession('muxplex-updates', { remoteId: 0 }); + + // Restore setTimeout so Promise-based yielding works + globalThis.setTimeout = origSetTimeout; + + // Reset fetch tracking + const fetchCalls = []; + globalThis.fetch = async (url, opts) => { fetchCalls.push({ url, opts }); return { ok: true }; }; + + await app.closeSession(); + await new Promise((r) => setTimeout(r, 0)); + + const deleteCall = fetchCalls.find((c) => c.url === '/api/sessions/current' && c.opts && c.opts.method === 'DELETE'); + assert.ok(!deleteCall, 'closeSession must NOT fire DELETE for remoteId=0 session (it is a remote session)'); + + globalThis.fetch = origFetch; + globalThis.document.getElementById = origGetById; + globalThis.document.querySelector = origQS; + globalThis.setTimeout = origSetTimeout; +});