fix: remoteId=0 treated as falsy — use null checks instead of || for integer remoteId

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.
This commit is contained in:
Brian Krabach
2026-04-02 04:46:50 -07:00
parent 000c71c40d
commit b0c1d0ab8d
2 changed files with 121 additions and 7 deletions
+7 -7
View File
@@ -434,7 +434,7 @@ function buildTileHTML(session, index, mobile) {
} }
const lastLines = allLines.slice(_lineCount).join('\n'); 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 ( return (
`<article class="${classes}" data-session="${escapedName}" data-session-key="${escapeHtml(session.sessionKey || name)}"${remoteIdAttr} tabindex="0" role="listitem" aria-label="${escapedName}">` + `<article class="${classes}" data-session="${escapedName}" data-session-key="${escapeHtml(session.sessionKey || name)}"${remoteIdAttr} tabindex="0" role="listitem" aria-label="${escapedName}">` +
`<div class="tile-header">` + `<div class="tile-header">` +
@@ -489,7 +489,7 @@ function buildSidebarHTML(session, currentSession) {
const lastLines = allLines.slice(-20).join('\n'); const lastLines = allLines.slice(-20).join('\n');
return ( return (
`<article class="${classes}" data-session="${escapedName}" data-remote-id="${escapeHtml(session.remoteId || '')}" tabindex="0" role="listitem">` + `<article class="${classes}" data-session="${escapedName}" data-remote-id="${escapeHtml(session.remoteId != null ? session.remoteId : '')}" tabindex="0" role="listitem">` +
`<div class="sidebar-item-header">` + `<div class="sidebar-item-header">` +
`<span class="sidebar-item-name">${escapedName}</span>` + `<span class="sidebar-item-name">${escapedName}</span>` +
badgeHtml + badgeHtml +
@@ -873,7 +873,7 @@ function _previewClickHandler(e) {
hidePreview(); hidePreview();
if (name) { if (name) {
var session = _currentSessions && _currentSessions.find(function(s) { return s.name === 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 = {}) { async function openSession(name, opts = {}) {
hidePreview(); hidePreview();
_viewingSession = name; _viewingSession = name;
_viewingRemoteId = opts.remoteId || ''; _viewingRemoteId = opts.remoteId != null ? opts.remoteId : '';
_viewMode = 'fullscreen'; _viewMode = 'fullscreen';
// Pre-render sidebar with current sessions before first poll tick // 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'); if (fab) fab.classList.add('hidden');
// Always spawn ttyd for this session — ensures correct session after service restart or page restore // 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 { try {
if (_remoteId) { if (_remoteId !== '') {
// Remote session: route connect POST through same-origin federation proxy // Remote session: route connect POST through same-origin federation proxy
await api('POST', '/api/federation/' + encodeURIComponent(_remoteId) + '/connect/' + encodeURIComponent(name)); await api('POST', '/api/federation/' + encodeURIComponent(_remoteId) + '/connect/' + encodeURIComponent(name));
} else { } else {
@@ -1233,7 +1233,7 @@ function closeSession() {
if (window._closeTerminal) window._closeTerminal(); if (window._closeTerminal) window._closeTerminal();
// Fire-and-forget DELETE — skip for remote sessions (they don't need to know we stopped watching) // 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() {}); api('DELETE', '/api/sessions/current').catch(function() {});
} }
_viewingRemoteId = ''; _viewingRemoteId = '';
+114
View File
@@ -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', '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;
});