From 391db2f2197b5f7fbdba0c918a24830a5cbe6ee4 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 9 Sep 2026 16:37:23 +0200 Subject: fix(spa): a reconnect must give the Chat composer back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Chat tab froze about every other day — the textbox stopped taking clicks — and it never recovered on its own: no timeout ends this one, only leaving the group or restarting the client. A console dump of a session it happened in ruled out everything it could and named nothing. What that dump established was almost entirely negative, and that was the useful part. No `Response timeout`, no `unsolicited`/`unrouted`/`with nothing waiting` — so the 2026-08-30 routing defect, which produces this exact symptom for thirty seconds, had not recurred. No `PC state: disconnected|failed`, no second ICE cycle, no `Reconnected after N attempt(s)` — so the connection was alive and untouched. The freeze was in the page, and no path that logs anything had run. The composer is `disabled=${sending || cannotSend}`, and `cannotSend` was `transport.connected && !transport.devicePk`, read off a **ref** during render. `devicePk` is settled inside connect(), so every reconnect clears it and settles it again; a ref changing re-renders nothing, and nothing else announced it. So the panel went disabled on whatever unrelated re-render came next — a message arriving — long after the identity was actually lost, and had no event that would open it again. group-page.js never touches `status` after 'connected', and `onReconnected` is claimed by video-player.js, so there was no second chance. It was silent as well as sticky. `_announceDevice` had three exits that wrote `devicePk` without a word: two early returns that left the *previous* connection's value standing, and a reply that is not `device_hello_ack` — an `error` reply does not throw, so the `.catch()` at the call site never saw it. Reproduced in chat_send_probe.py, which mounts the real ChatPanel over the real transport: with the old code, identity cleared leaves the composer open, an arriving message latches it shut, and restoring the identity does not reopen it. Every write to `devicePk` now goes through `_setDevicePk(pk, why)`, which logs, traces and calls `onDeviceIdentity`; group-page holds the answer as state and ChatPanel takes it as `deviceReady`. Defaulting that prop to `true` fails open — a wiring mistake here must not be able to leave anyone with a dead textbox. Two things found on the same path and fixed with it. `_send` throwing inside _sendAndWait's executor left the pending entry and its 30s timer behind, so a request that never reached the wire still logged a "Response timeout" half a minute later. And the instrumentation this was meant to be diagnosed with (3be8bd2) writes to localStorage behind ?trace=1, not to the console, so the dump could not have carried it: the two lines that decide the composer's state are now logged unconditionally, and MeshBayTrace gains `record` so the composer writes into the same timeline as the channel events. Hub suite 2264 passed, 4 skipped. chat_send_probe.py gains a `reconnect` scenario and test_chat_send.py four cases, each checked against the unfixed source. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019GXmScYB1uR29YCt74si9J --- .../meshbay-hub/src/meshbay_hub/static/chat-app.js | 33 +++++++- .../src/meshbay_hub/static/group-page.js | 16 +++- .../src/meshbay_hub/static/transport.js | 90 ++++++++++++++++++++-- 3 files changed, 126 insertions(+), 13 deletions(-) (limited to 'packages/meshbay-hub/src') 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 }); @@ -1079,17 +1108,49 @@ class MeshBayTransport { throw rejected; } + /** + * 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); + } }); } -- cgit v1.2.3