diff options
Diffstat (limited to 'packages/meshbay-hub')
5 files changed, 254 insertions, 16 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js b/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js index a793527..6e9de5f 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js @@ -203,9 +203,17 @@ function ChatImage({ filename, entries, transportRef, gekRef }) { // read one answer. Empty means the group has no writable root right now — every // root is read-only, or the one drive that was writable is unplugged — and the // paperclip says so rather than producing a refusal from the node. +// +// `deviceReady` is "this connection has identified a device to the node", the +// one thing chat needs beyond being connected. It arrives as a prop, and that +// is the correction: it used to be read off the transport during render +// (`transportRef.current.devicePk`), and a ref changing re-renders nothing — so +// once a reconnect cleared it the composer stayed disabled for the rest of the +// session. Defaulting to `true` fails open: a wiring mistake here must never be +// able to leave someone with a dead textbox. function ChatPanel({ transportRef, username, userId, entries, gekRef, onRefreshIndex, onPreview, attachRoot = '', attachDir = '', - onActivity, status }) { + onActivity, status, deviceReady = true }) { const [messages, setMessages] = useState([]); const [hasMore, setHasMore] = useState(false); const [loadingOlder, setLoadingOlder] = useState(false); @@ -519,9 +527,26 @@ function ChatPanel({ transportRef, username, userId, entries, gekRef, // refuse a member claiming somebody else's key. Without it there is nothing // to send with, so the composer says so before anything is typed rather than // producing a refusal the reader cannot act on. - const transportNow = transportRef.current; - const cannotSend = !!(transportNow && transportNow.connected - && !transportNow.devicePk); + // + // Only while connected: before that the composer is enabled and `sendMessage` + // simply declines, which is what it always did — saying "this device cannot + // post" at someone who is merely still connecting names the wrong problem. + const cannotSend = status === 'connected' && !deviceReady; + + // The composer's disabled state has exactly two inputs, and neither of them + // was observable from outside the component. A "chat hangs, the textbox is + // not clickable" report arrived with a complete console dump that could not + // say which of the two it had been, nor when it started. This is what makes + // the next one answer that in one line. + useEffect(() => { + const why = sending ? 'send in flight' + : cannotSend ? 'device not identified to the node' + : null; + console.log('[MeshBay] chat composer:', why ? 'disabled (' + why + ')' : 'enabled'); + if (window.MeshBayTrace && window.MeshBayTrace.record) { + window.MeshBayTrace.record('chat_composer', { disabled: !!why, why }); + } + }, [sending, cannotSend]); const onKeyDown = useCallback((e) => { if (e.key === 'Enter' && !e.shiftKey) { diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js index c6748f2..e897646 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -26,6 +26,12 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, onRefreshAuth, onJoined, onGroupUpdated, onPresence, onLeft, onPlayQueue: parentOnPlayQueue, onStopMusic }) { const [status, setStatus] = useState('idle'); + // Whether this connection has identified a device to the node (`device_hello`). + // Held as state, not read off the transport at render time: it is settled + // inside connect() and re-settled by every reconnect, and the Chat composer + // gates on it — a value only a ref knows about leaves that composer disabled + // with no event to bring it back. Fed by transport.onDeviceIdentity below. + const [deviceReady, setDeviceReady] = useState(false); const [entries, setEntries] = useState([]); const [error, setError] = useState(''); @@ -282,6 +288,8 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, const connect = async () => { setStatus('discovering'); setError(''); + // Belongs to the connection about to be made, not the group just left. + setDeviceReady(false); gekRef.current = null; if (!session.bundleKey) session.bundleKey = await _loadBundleKey(); // Persisted (docs/auth-confirm.md §4.3) so a group joined in a later @@ -325,6 +333,12 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, // connect() call can be stale by then, since the whole point is that // some real time (screen lock, a dead NAT mapping) passed unnoticed. transport.onNeedToken = async () => (await ensureFreshToken()) || token; + // Set before connect(), because connect() is where device_hello runs — + // and again on every reconnect it makes, which is the case this exists + // for: nothing else tells the page the answer changed. + transport.onDeviceIdentity = (ok) => { + if (!cancelled) setDeviceReady(ok); + }; const ack = await transport.connect( nodeId, live, groupId, null, sessionKeys, session.bundleKey, username, @@ -683,7 +697,7 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, musicbrainzConfig]); const commonProps = { - groupId, transportRef, gekRef, status, username, + groupId, transportRef, gekRef, status, username, deviceReady, entries, availableEntries, nodeDirs, nodeRoots, setEntries, setNodeDirs, setNodeRoots, applyIndex, isNodeAdmin, operatorPaired, attachRoot, attachDir, userId, setError, onPreview, diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index 0b2fed5..d90c072 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -174,6 +174,11 @@ function trace(event, data) { window.MeshBayTrace = { enabled: traceEnabled, + // So a module that is not this one can write to the same buffer. The + // composer's own state is the half of the "chat hangs, the textbox is dead" + // report transport.js cannot see, and it belongs in the same timeline as the + // channel events it has to be read against. + record: trace, dump() { try { return JSON.parse(localStorage.getItem(TRACE_LOG_KEY) || '[]'); } catch { return []; } }, @@ -482,6 +487,12 @@ class MeshBayTransport { this._inReconnectAttempt = false; this._onReconnected = null; this._onNeedToken = null; + // Which device key THIS connection has identified itself to the node with. + // Empty means "not identified": nothing can be sealed, so nothing can be + // posted to chat. Written only through _setDevicePk, which is what makes + // the change visible to a reader — see onDeviceIdentity. + this.devicePk = ''; + this._onDeviceIdentity = null; // Cuts the backoff wait short the moment the page is foregrounded again — // found live to matter: a screen lock throttles the tab's own timers // along with everything else, so a backoff already counting down when the @@ -581,6 +592,18 @@ class MeshBayTransport { // handshake, so a consumer with something mid-flight on the old channel — // today only the video player — can pick back up rather than sit dead. set onReconnected(fn) { this._onReconnected = fn; } + + /** + * Told whenever this connection's device identity changes — including to + * *nothing*, which is the case that mattered. + * + * `devicePk` is settled inside connect(), so a reconnect can clear it long + * after the page last rendered. A reader that computed "can I post?" from the + * field itself — chat-app.js did, through a ref — had no way to learn the + * answer had changed, and the composer stayed disabled on a connection with + * nothing whatever wrong with it and not one line in the console. + */ + set onDeviceIdentity(fn) { this._onDeviceIdentity = fn; } // Reconnecting redoes the handshake, which needs a JWT that may have gone // stale while the connection was down for minutes. Without this the // reconnect resends whatever token the original connect() call captured, @@ -631,6 +654,12 @@ class MeshBayTransport { this._newNodeBundle = null; this._newNodeBundleRecovery = null; this._joinError = null; + // Per connection, for the same reason the chat keys and the roster are + // dropped further down: the device the *previous* connection identified + // itself with is not this one's, and leaving it standing is how a + // reconnect that never got as far as announcing a device still looked, to + // the composer, exactly like one that had. + this._setDevicePk('', 'new connection'); this._pc = new RTCPeerConnection({ iceServers: await iceServers() }); this._channel = this._pc.createDataChannel('mnp', { ordered: true }); @@ -1080,16 +1109,48 @@ class MeshBayTransport { } /** + * Record — and announce — the device key this connection is identified by. + * + * Every write to `devicePk` goes through here, for two reasons: it is traced, + * so a session that ends up unable to post says so and says when; and it + * tells the page, which had no other way to find out that the answer moved. + */ + _setDevicePk(pk, why) { + const next = pk || ''; + if (next === this.devicePk) return next; + this.devicePk = next; + console.log('[MeshBay] device identity:', + next ? 'identified' : 'NOT identified — chat cannot post', + '(' + why + ')'); + trace('device_identity', { identified: !!next, why }); + if (this._onDeviceIdentity) { + try { this._onDeviceIdentity(!!next); } catch (e) { + console.error('[MeshBay] onDeviceIdentity handler threw:', e); + } + } + return next; + } + + /** * "This connection is device X of account Y", signed with the device key. * * Best effort by construction: a browser that has not recovered its identity - * keys has nothing to sign with, and a node older than MNP 1.2 does not know - * the message. Neither is an error — the node simply keeps the weaker - * attribution it had before, which is what every client did until now. + * keys has nothing to sign with. What it is *not* is silent — and it had + * three exits that were. Two early returns left whatever the previous + * connection had settled on standing; and a reply that is not + * `device_hello_ack` wiped the key without a word, because an `error` reply + * does not throw and so never reached the `.catch()` at the call site. The + * result is a chat that cannot post on a connection with nothing else wrong + * with it, which is unreadable from the outside — the shape of the "chat + * hangs, the textbox is dead" report. Every exit below names itself. */ async _announceDevice() { - if (!this._sessionKeys || !this._sessionKeys.skEdB64) return; - if (!this._nonceNode || !this.nodePk || !this._userId) return; + if (!this._sessionKeys || !this._sessionKeys.skEdB64) { + return this._setDevicePk('', 'no identity key in this session'); + } + if (!this._nonceNode || !this.nodePk || !this._userId) { + return this._setDevicePk('', 'handshake state incomplete'); + } const C = window.MeshBayCrypto; // Derived from our own secret key, never read back from anywhere — the same @@ -1106,8 +1167,11 @@ class MeshBayTransport { const resp = await this._sendAndWait({ type: 'device_hello', v: '2.0', pk_ed25519: pkEdB64, ts, sig, }); - this.devicePk = (resp && resp.type === 'device_hello_ack') ? pkEdB64 : ''; - return this.devicePk; + if (!resp || resp.type !== 'device_hello_ack') { + return this._setDevicePk('', 'node answered ' + ((resp && resp.type) || 'nothing') + + ((resp && resp.detail) ? ': ' + resp.detail : '')); + } + return this._setDevicePk(pkEdB64, 'device_hello_ack'); } /** @@ -2979,7 +3043,17 @@ class MeshBayTransport { // some field of its own — and why the ones that carry no such field // reached their caller by luck. An older node ignores the extra key and // is routed by the per-type fallbacks below, exactly as before. - this._send({ ...obj, req_id: id }); + // `_send` throws synchronously when the channel is not open. Rejecting + // on that is right, but the pending entry and its 30s timer were left + // behind — so a request that never reached the wire still logged a + // "Response timeout" half a minute later, for a reply nobody was owed. + try { + this._send({ ...obj, req_id: id }); + } catch (e) { + clearTimeout(timeout); + this._pending.delete(id); + reject(e); + } }); } diff --git a/packages/meshbay-hub/tests/harness/chat_send_probe.py b/packages/meshbay-hub/tests/harness/chat_send_probe.py index 28635b0..2df12e3 100644 --- a/packages/meshbay-hub/tests/harness/chat_send_probe.py +++ b/packages/meshbay-hub/tests/harness/chat_send_probe.py @@ -36,6 +36,18 @@ Both are run with an older request already pending — the condition that turns "guess by arrival order" from usually-right into wrong — and both must come back inside a second and a half. +A third scenario, `reconnect`, is about the *other* way this tab freezes, and +the one no timeout ends: + + `reconnect` the connection is untouched, but its **device identity** is + cleared and settled again, which is what every reconnect does to + it — `connect()` drops it on the way in and `device_hello` + restores it on the way out. The composer gates on that identity, + and used to read it off the transport during render, where a ref + changing re-renders nothing: it latched shut on whatever + unrelated re-render came next (a message arriving) and had no + event that would open it again. + Since MNP 2.0 a send also has to **seal and sign for real** before it goes anywhere, so this drives `chatKeys()`, `openGroup`, `sealChat` and a genuine Ed25519 signature rather than a model of any of them. The device key is @@ -110,7 +122,7 @@ PAGE_TEMPLATE = r"""<!doctype html><html><head><meta charset=utf-8> <script src="/keyderive.js"></script> <script src="/transport.js"></script> <script type="module"> -import { html, render, useRef } from '/vendor/htm-preact.js'; +import { html, render, useRef, useState, useEffect } from '/vendor/htm-preact.js'; import { ChatPanel } from '/chat-app.js'; const log = []; @@ -192,11 +204,23 @@ function makeTransport(name, chatReply) { return tp; } +// Stands in for group-page.js the way the stub above stands in for the node, +// and for the same reason: what is under test is the seam between them. These +// are its three lines — state, the callback wired before connect() runs, and +// the prop — because the defect was that ChatPanel read `devicePk` off the +// transport during render instead, and a ref changing re-renders nothing. function Host({ tp }) { const transportRef = useRef(tp); const gekRef = useRef(null); + // Seeded from the transport because makeTransport hands over a connection + // whose handshake is already done; in the page the callback below is what + // sets it, since it is wired before connect() and connect() is where + // device_hello runs. + const [deviceReady, setDeviceReady] = useState(!!tp.devicePk); + useEffect(() => { tp.onDeviceIdentity = (ok) => setDeviceReady(ok); }, [tp]); return html`<${ChatPanel} transportRef=${transportRef} gekRef=${gekRef} - username="me" userId="user-me" entries=${[]} status="connected" />`; + username="me" userId="user-me" entries=${[]} status="connected" + deviceReady=${deviceReady} />`; } function typeInto(root, text) { @@ -208,7 +232,7 @@ function typeInto(root, text) { c.dispatchEvent(new Event('input', { bubbles: true })); } -async function runScenario(name, chatReply) { +async function runScenario(name, chatReply, duringSession) { const root = document.createElement('div'); document.getElementById('root').appendChild(root); const tp = makeTransport(name, chatReply); @@ -225,6 +249,10 @@ async function runScenario(name, chatReply) { // a send is in flight. composerDisabled: c ? c.disabled : null, composerValue: c ? c.value : null, + // Which of the composer's two reasons it is. "Disabled" alone was all + // the field report could say, and it is the half that does not identify + // the defect. + composerPlaceholder: c ? c.placeholder : null, pending: tp._pending.size, }); }; @@ -241,6 +269,8 @@ async function runScenario(name, chatReply) { await wait(100); snap('older request pending'); + if (duringSession) await duringSession(tp, snap); + typeInto(root, 'hello'); await wait(100); root.querySelector('.chat-input').dispatchEvent(new KeyboardEvent('keydown', @@ -259,6 +289,33 @@ async function runScenario(name, chatReply) { out.scenarios.push(await runScenario('ack', { type: 'ack', v: '0.14' })); out.scenarios.push(await runScenario( 'error', { type: 'error', detail: 'Request failed' })); + // The connection survives, the device identity does not — which is exactly + // what a reconnect does: connect() clears it on the way in and device_hello + // settles it again on the way out. Nothing about the channel changes, so + // nothing else in the page moves, and the composer has to follow this on its + // own or it never comes back. + out.scenarios.push(await runScenario( + 'reconnect', { type: 'ack', v: '0.14' }, + async (tp, snap) => { + tp._setDevicePk('', 'new connection'); + await wait(100); + snap('device identity cleared'); + // A live message arrives — the ordinary thing that re-renders this + // panel, and the step that made the old defect permanent. The composer + // read `devicePk` off the transport during render, so it went disabled + // *here*, on an unrelated re-render, long after the identity was + // actually lost; and since nothing re-rendered it when the identity came + // back, it stayed that way for the rest of the session. + if (tp._onChat) { + tp._onChat({ id: 'live-1', sender_id: 'someone', sender_name: 'someone', + payload: 'still there?', timestamp: now, verified: true }); + } + await wait(100); + snap('a message arrived meanwhile'); + tp._setDevicePk(DEVICE_PK_B64, 'device_hello_ack'); + await wait(100); + snap('device identity restored'); + })); fetch('/log', { method: 'POST', body: JSON.stringify(out) }); })(); </script></body></html>""" diff --git a/packages/meshbay-hub/tests/test_chat_send.py b/packages/meshbay-hub/tests/test_chat_send.py index 5db8f26..d442383 100644 --- a/packages/meshbay-hub/tests/test_chat_send.py +++ b/packages/meshbay-hub/tests/test_chat_send.py @@ -24,6 +24,13 @@ shapes of answer. None of it is visible in `chat-app.js`, where every line is correct, so this drives the real panel over the real transport in a browser rather than reading either source. + +Since 2026-09-09 it covers a **second** way the tab freezes, found from a field +report and reproduced here: a reconnect clears the connection's device identity +and settles it again, and the composer gates on that. Nothing announced the +change, so the panel latched shut on an unrelated re-render and had no event +that would open it again. Unlike the routing defect above, no timeout ends it — +only leaving the group or restarting the client does. """ import json import shutil @@ -147,6 +154,67 @@ def test_the_chat_keys_answer_is_not_handed_to_another_request(probe): "send then waits out its own 30s timeout with the composer disabled") +def test_a_reconnect_gives_the_composer_back(probe): + """ + The other way a Chat tab freezes, and the one no timeout ever ends. + + A send that goes astray holds the composer for 30s. This holds it for the + rest of the session: `devicePk` — "this connection identified a device to + the node" — is settled inside `connect()`, so every reconnect clears it and + re-settles it, and the composer gates on it. Nothing announced the change, + so the panel went disabled on whatever unrelated re-render happened next + (a message arriving) and had no event that would bring it back. The + connection stayed perfectly healthy throughout, no request ever timed out, + and nothing reached the console: a field report of exactly this arrived + with a full console dump that could not say what had happened. + + The three states below are the whole claim: it closes when the identity + goes, it stays closed while it is gone, and it **opens again** when the + identity comes back. + """ + _, steps = probe + sc = steps["reconnect"] + assert sc["older request pending"]["composerDisabled"] is False, ( + "the composer was already unusable before the reconnect") + assert sc["device identity cleared"]["composerDisabled"] is True, ( + "the composer stayed open with no device identity to seal with -- the " + "send would be refused with no reason on screen") + assert sc["a message arrived meanwhile"]["composerDisabled"] is True, ( + "an unrelated re-render changed the answer, which means the answer was " + "never being derived from anything the panel was told about") + assert sc["device identity restored"]["composerDisabled"] is False, ( + "the composer never came back after the reconnect re-identified the " + "device -- this is the freeze that no timeout ends and that only " + "leaving the group or restarting the client clears") + + +def test_the_closed_composer_says_which_of_its_two_reasons_it_is(probe): + """ + A disabled textbox is one symptom with two causes — a send in flight, or no + device identity — and telling them apart is what the field report could not + do. The placeholder is where a person reads the difference. + """ + _, steps = probe + sc = steps["reconnect"] + assert sc["device identity cleared"]["composerPlaceholder"] \ + == "chat.encrypted_cannot_send", ( + "a closed composer offered no reason for being closed") + assert sc["device identity restored"]["composerPlaceholder"] \ + == "chat.placeholder" + + +def test_sending_works_again_after_a_reconnect(probe): + """Not just enabled — actually able to seal and send under the identity + the reconnect settled on.""" + _, steps = probe + sc = steps["reconnect"] + before = sc["device identity restored"]["bubbles"] + assert sc["after send"]["bubbles"] == before + 1, ( + "the message was not added to the conversation after the reconnect") + assert sc["after send"]["composerValue"] == "", ( + "the text came back into the composer, so the send failed") + + def test_history_still_renders(probe): """ Not about sending at all, and here because it broke without a sound: |