From add9e037185357d027f91c054e08d6ec952507d7 Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Mon, 6 Apr 2026 18:08:19 -0700 Subject: [PATCH] fix: persist active_remote_id in state for remote session restore Remote sessions failed to reconnect after page refresh because restoreState() and renderSheetList() didn't pass remoteId to openSession(). Fix: - state.py: add active_remote_id field to empty_state() - main.py: extend StatePatch model to accept active_session and active_remote_id; update PATCH /api/state to selectively update only fields explicitly provided (using model_fields_set) - app.js restoreState(): read state.active_remote_id and pass it as remoteId option so page-refresh reconnects remote sessions - app.js openSession(): fire-and-forget PATCH /api/state to persist active_session + active_remote_id after successful connect - app.js closeSession(): fire-and-forget PATCH /api/state to clear both fields so refresh doesn't try to reopen a closed session - app.js renderSheetList(): add data-remote-id to sheet items and pass remoteId in click handler (previously called openSession(name) with no remoteId, routing remote sessions to local 404 endpoint) Fixes: POST /api/sessions/paperclip-adapter/connect 404 when clicking a remote session from the mobile bottom sheet or after page refresh. --- muxplex/frontend/app.js | 16 +++- muxplex/frontend/tests/test_app.mjs | 112 ++++++++++++++++++++++++++++ muxplex/main.py | 19 ++++- muxplex/state.py | 1 + muxplex/tests/test_api.py | 60 +++++++++++++++ muxplex/tests/test_state.py | 10 +++ 6 files changed, 212 insertions(+), 6 deletions(-) diff --git a/muxplex/frontend/app.js b/muxplex/frontend/app.js index 6d2dd03..8d89919 100644 --- a/muxplex/frontend/app.js +++ b/muxplex/frontend/app.js @@ -219,7 +219,10 @@ async function restoreState() { const res = await api('GET', '/api/state'); const state = await res.json(); if (state.active_session) { - await openSession(state.active_session, { skipAnimation: true }); + await openSession(state.active_session, { + skipAnimation: true, + remoteId: state.active_remote_id || '', + }); } } catch (err) { console.warn('[restoreState] could not restore previous session:', err); @@ -1241,6 +1244,9 @@ async function openSession(name, opts = {}) { return closeSession(); } + // Persist active_remote_id so restoreState() can reopen remote sessions after page refresh + api('PATCH', '/api/state', { active_session: name, active_remote_id: _remoteId || null }).catch(function() {}); + // Fire-and-forget bell-clear for remote sessions — acknowledge bells on the remote server if (_remoteId !== '') { api('POST', '/api/federation/' + encodeURIComponent(_remoteId) + '/sessions/' + encodeURIComponent(name) + '/bell/clear').catch(function() {}); @@ -1267,6 +1273,8 @@ function closeSession() { if (_viewingRemoteId === '') { api('DELETE', '/api/sessions/current').catch(function() {}); } + // Clear active_remote_id so a page refresh does not attempt to reopen the remote session + api('PATCH', '/api/state', { active_session: null, active_remote_id: null }).catch(function() {}); _viewingRemoteId = ''; const expanded = $('view-expanded'); @@ -1857,8 +1865,9 @@ function renderSheetList() { (s.bell.seen_at === null || s.bell.last_fired_at > s.bell.seen_at); var isActive = s.name === _viewingSession; var escapedName = escapeHtml(s.name || ''); + var remoteIdAttr = s.remoteId ? ' data-remote-id="' + escapeHtml(s.remoteId) + '"' : ''; return '
  • ' + + ' data-session="' + escapedName + '"' + remoteIdAttr + ' role="option">' + '' + escapedName + '' + (hasBell ? '\uD83D\uDD14' : '') + '' + formatTimestamp(s.bell && s.bell.last_fired_at) + '' + @@ -1869,7 +1878,8 @@ function renderSheetList() { item.addEventListener('click', function() { closeBottomSheet(); var name = item.dataset.session; - if (name !== _viewingSession) openSession(name); + var remoteId = item.dataset.remoteId || ''; + if (name !== _viewingSession) openSession(name, { remoteId: remoteId }); }); }); } diff --git a/muxplex/frontend/tests/test_app.mjs b/muxplex/frontend/tests/test_app.mjs index 147549e..922279f 100644 --- a/muxplex/frontend/tests/test_app.mjs +++ b/muxplex/frontend/tests/test_app.mjs @@ -4381,3 +4381,115 @@ test('showNewSessionInput device select has keydown handler for Escape', () => { const selectKeydownIdx = fnBody.indexOf("select.addEventListener('keydown'"); assert.ok(selectKeydownIdx !== -1, 'select element must have a keydown handler in showNewSessionInput'); }); + +// --- Fix: remote sessions fail to connect because openSession call sites don't pass remoteId --- + +test('restoreState reads active_remote_id from state and passes it to openSession', () => { + const source = fs.readFileSync(new URL('../app.js', import.meta.url), 'utf8'); + const restoreIdx = source.indexOf('async function restoreState'); + assert.ok(restoreIdx >= 0, 'restoreState must exist'); + const restoreEnd = source.indexOf('\n}', restoreIdx) + 2; + const restoreBody = source.slice(restoreIdx, restoreEnd); + assert.ok( + restoreBody.includes('active_remote_id'), + 'restoreState must read active_remote_id from persisted state', + ); + assert.ok( + restoreBody.includes('remoteId'), + 'restoreState must pass remoteId to openSession so remote sessions restore correctly', + ); +}); + +test('renderSheetList click handler passes remoteId to openSession', () => { + const source = fs.readFileSync(new URL('../app.js', import.meta.url), 'utf8'); + const sheetIdx = source.indexOf('function renderSheetList'); + assert.ok(sheetIdx >= 0, 'renderSheetList must exist'); + // Extract the full function body (up to the closing brace) + const sheetEnd = source.indexOf('\n}', sheetIdx) + 2; + const sheetBody = source.slice(sheetIdx, sheetEnd); + assert.ok( + sheetBody.includes('remoteId'), + 'renderSheetList click handler must pass remoteId to openSession', + ); +}); + +test('renderSheetList item HTML includes data-remote-id attribute for remote sessions', () => { + let capturedHTML = ''; + const mockList = { + get innerHTML() { return capturedHTML; }, + set innerHTML(v) { capturedHTML = v; }, + querySelectorAll: () => [], + }; + const origGetById = globalThis.document.getElementById; + globalThis.document.getElementById = (id) => (id === 'sheet-list' ? mockList : null); + app._setCurrentSessions([{ name: 'remote-sess', remoteId: 'fed-abc123', bell: null }]); + + app.renderSheetList(); + + assert.ok( + capturedHTML.includes('data-remote-id="fed-abc123"'), + 'sheet item should have data-remote-id attribute for remote sessions', + ); + globalThis.document.getElementById = origGetById; +}); + +test('openSession PATCHes /api/state with active_remote_id after successful connect', 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 = (fn) => { fn(); }; // invoke immediately so animation resolves + globalThis.window._openTerminal = () => {}; + + await app.openSession('remote-session', { remoteId: 'fed-abc123' }); + + const patchCall = fetchCalls.find((c) => c.url === '/api/state' && c.opts && c.opts.method === 'PATCH'); + assert.ok(patchCall, 'openSession should PATCH /api/state after successful connect'); + const body = JSON.parse(patchCall.opts.body); + assert.strictEqual(body.active_remote_id, 'fed-abc123', 'PATCH body should include active_remote_id'); + + globalThis.fetch = origFetch; + globalThis.document.getElementById = origGetById; + globalThis.document.querySelector = origQS; + globalThis.setTimeout = origSetTimeout; +}); + +test('closeSession PATCHes /api/state to clear active_remote_id', async () => { + const origFetch = globalThis.fetch; + const origGetById = globalThis.document.getElementById; + const origQS = globalThis.document.querySelector; + const origSetTimeout = globalThis.setTimeout; + + // Open a remote session first so _viewingRemoteId is set + globalThis.fetch = async () => ({ ok: true }); + globalThis.document.getElementById = () => ({ textContent: '', style: {}, classList: { remove: () => {}, add: () => {} } }); + globalThis.document.querySelector = () => null; + globalThis.setTimeout = (fn) => { fn(); }; + globalThis.window._openTerminal = () => {}; + globalThis.window._closeTerminal = () => {}; + + await app.openSession('remote-sess', { remoteId: 'fed-abc123' }); + + 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 patchCall = fetchCalls.find((c) => c.url === '/api/state' && c.opts && c.opts.method === 'PATCH'); + assert.ok(patchCall, 'closeSession should PATCH /api/state to clear remote session state'); + const body = JSON.parse(patchCall.opts.body); + assert.strictEqual(body.active_remote_id, null, 'PATCH should clear active_remote_id to null'); + + globalThis.fetch = origFetch; + globalThis.document.getElementById = origGetById; + globalThis.document.querySelector = origQS; + globalThis.setTimeout = origSetTimeout; +}); diff --git a/muxplex/main.py b/muxplex/main.py index 8b5cecf..b3f2623 100644 --- a/muxplex/main.py +++ b/muxplex/main.py @@ -279,7 +279,9 @@ app.add_middleware( class StatePatch(BaseModel): - session_order: list[str] + session_order: list[str] | None = None + active_session: str | None = None + active_remote_id: str | None = None class HeartbeatPayload(BaseModel): @@ -332,10 +334,21 @@ async def get_state() -> dict: @app.patch("/api/state") async def patch_state(patch: StatePatch) -> dict: - """Update session_order in the persistent state and return the updated state.""" + """Update fields in the persistent state and return the updated state. + + Only fields explicitly included in the request body are updated; + omitted fields are left unchanged. Supports: session_order, + active_session, active_remote_id. + """ async with state_lock: state = load_state() - state["session_order"] = patch.session_order + changed = patch.model_fields_set + if "session_order" in changed: + state["session_order"] = patch.session_order + if "active_session" in changed: + state["active_session"] = patch.active_session + if "active_remote_id" in changed: + state["active_remote_id"] = patch.active_remote_id save_state(state) return state diff --git a/muxplex/state.py b/muxplex/state.py index 56ca97b..a08bff1 100644 --- a/muxplex/state.py +++ b/muxplex/state.py @@ -59,6 +59,7 @@ def empty_state() -> dict: """ return { "active_session": None, + "active_remote_id": None, "session_order": [], "sessions": {}, "devices": {}, diff --git a/muxplex/tests/test_api.py b/muxplex/tests/test_api.py index fd4714f..eae1575 100644 --- a/muxplex/tests/test_api.py +++ b/muxplex/tests/test_api.py @@ -139,6 +139,66 @@ def test_patch_state_rejects_non_list_session_order(client): assert response.status_code == 422 +def test_patch_state_updates_active_remote_id(client): + """PATCH /api/state with active_remote_id persists it in state.""" + response = client.patch( + "/api/state", + json={"active_session": "remote-sess", "active_remote_id": "fed-abc123"}, + ) + assert response.status_code == 200 + data = response.json() + assert data["active_remote_id"] == "fed-abc123" + assert data["active_session"] == "remote-sess" + + +def test_patch_state_clears_active_remote_id(client): + """PATCH /api/state with active_remote_id: null clears it in state.""" + from muxplex.state import save_state + + # Set up initial state with active_remote_id + initial = { + "active_session": "remote-sess", + "active_remote_id": "fed-abc123", + "session_order": [], + "sessions": {}, + "devices": {}, + } + save_state(initial) + + response = client.patch( + "/api/state", + json={"active_session": None, "active_remote_id": None}, + ) + assert response.status_code == 200 + data = response.json() + assert data["active_remote_id"] is None + assert data["active_session"] is None + + +def test_patch_state_without_session_order_updates_active_remote_id_only(client): + """PATCH /api/state without session_order only updates active_remote_id.""" + from muxplex.state import save_state + + initial = { + "active_session": None, + "active_remote_id": None, + "session_order": ["alpha", "beta"], + "sessions": {}, + "devices": {}, + } + save_state(initial) + + response = client.patch( + "/api/state", + json={"active_remote_id": "fed-xyz"}, + ) + assert response.status_code == 200 + data = response.json() + assert data["active_remote_id"] == "fed-xyz" + # session_order should be unchanged + assert data["session_order"] == ["alpha", "beta"] + + def test_patch_state_ignores_unknown_fields(client): """PATCH /api/state ignores unknown fields in the request body.""" response = client.patch( diff --git a/muxplex/tests/test_state.py b/muxplex/tests/test_state.py index e5c502a..dba6b3d 100644 --- a/muxplex/tests/test_state.py +++ b/muxplex/tests/test_state.py @@ -68,6 +68,16 @@ def test_empty_state_devices_is_empty_dict(): assert state["devices"] == {} +def test_empty_state_has_active_remote_id_key(): + state = empty_state() + assert "active_remote_id" in state + + +def test_empty_state_active_remote_id_is_none(): + state = empty_state() + assert state["active_remote_id"] is None + + def test_empty_state_returns_independent_dicts(): """Mutating one state must not affect another.""" s1 = empty_state()