From 4d5a07299c792de1251622a41ce7f94870b363fa Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 9 Sep 2026 16:55:53 +0200 Subject: test(spa): drive the composer's recovery through the real reconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reconnect scenario added with the fix proved the composer follows `_setDevicePk`, and it poked that method itself at both ends. That is a narrower claim than it reads as: it says nothing about whether a reconnect *reaches* it, and the harness's own `Host` stands in for group-page.js, so a green probe did not mean the page joins the two. Both halves are real now. The scenario calls `connect()` with the arguments `_reconnectLoop` calls it with; it stops at signaling, because there is no hub in the harness, and the identity has to be gone by then — connect() drops it before it touches the network. The restore is the shipped `_announceDevice`, answered by the stand-in node with a `device_hello_ack` as `_do_device_hello` answers it, and the key it settles on is the one the following send seals and signs with. Three assertions check the scenario went that way rather than through a variable set by the test. The seam the harness cannot drive gets its own check: the wiring exists, the prop is in `commonProps`, and the callback is set *before* `connect()` — after it, device_hello's answer is missed and the composer starts closed. That check first passed with the wiring deleted, on the strength of a comment naming the callback; it matches the assignment now. Two things the harness turned up. `do_POST` answered every path, so the offer connect() posts to the hub was swallowed as the measurement and put the machine's own SDP, public address included, into the probe's output — it answers `/log` and nothing else now. And a connect() that gives up before `await channelReady` left that promise rejected with nobody attached, so closing the peer connection printed "Uncaught (in promise) DataChannel closed" on every failed reconnect attempt — noise in exactly the log a freeze is read from. Checked against the unfixed source both ways: with the clear removed from connect() and the wiring removed from group-page.js, three cases fail; with them back, 15 pass. Hub and node suites 2266 passed, 4 skipped. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019GXmScYB1uR29YCt74si9J --- .../src/meshbay_hub/static/transport.js | 7 ++++ .../meshbay-hub/tests/harness/chat_send_probe.py | 45 +++++++++++++++++++- packages/meshbay-hub/tests/test_chat_send.py | 48 ++++++++++++++++++++++ 3 files changed, 98 insertions(+), 2 deletions(-) (limited to 'packages') diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index d90c072..03eb919 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -676,6 +676,13 @@ class MeshBayTransport { resolve(); }; }); + // Marks it handled, and nothing else — `await channelReady` below still + // sees the rejection. Without it, a connect() that gives up earlier (at + // signaling, say) leaves this promise with nobody attached, and closing the + // peer connection then rejects it into an "Uncaught (in promise) + // DataChannel closed" on the console. Every failed reconnect attempt + // printed one, which is noise in exactly the log a freeze gets read from. + channelReady.catch(() => { /* the awaiter below reports it */ }); this._channel.onmessage = (event) => this._onMessage(event.data); this._channel.onclose = (ev) => { diff --git a/packages/meshbay-hub/tests/harness/chat_send_probe.py b/packages/meshbay-hub/tests/harness/chat_send_probe.py index 2df12e3..b6dd957 100644 --- a/packages/meshbay-hub/tests/harness/chat_send_probe.py +++ b/packages/meshbay-hub/tests/harness/chat_send_probe.py @@ -178,6 +178,12 @@ function makeTransport(name, chatReply) { // Sealed under the group key, as `_do_chat_keys_req` sends it. answer({ type: 'chat_keys_resp', v: '2.0', group_id: tp._groupId, nonce: hex('__KEYS_NONCE_HEX__'), ct: hex('__KEYS_CT_HEX__') }); + } else if (obj.type === 'device_hello') { + // What `_do_device_hello` answers once the signature checks out. Driving + // the real `_announceDevice` through it is the point: the composer has to + // come back from the code that actually re-identifies the device after a + // reconnect, not from a test poking `devicePk`. + answer({ type: 'device_hello_ack', v: '2.0' }); } else if (obj.type === 'chat_msg') { // Recorded so the test can assert what actually left the browser, rather // than trusting that a composer which accepted the text sealed it. @@ -297,8 +303,27 @@ async function runScenario(name, chatReply, duringSession) { out.scenarios.push(await runScenario( 'reconnect', { type: 'ack', v: '0.14' }, async (tp, snap) => { - tp._setDevicePk('', 'new connection'); + // The real `connect()`, called the way `_reconnectLoop` calls it. It + // fails — there is no hub here to sign an offer with — and that is what + // makes the point: the identity is already gone by then, because + // connect() drops it before it touches the network. Nothing about this + // step is simulated, and the clear is not poked in by the test. + await tp.connect('node-1', 'token', tp._groupId, tp._gekRaw, + tp._sessionKeys, null, 'me', 'user-me').then( + () => log.push('reconnect: connect() unexpectedly succeeded'), + (e) => log.push('reconnect: connect() stopped at signaling, as expected: ' + + (e && e.message || e))); + // The dead peer connection would otherwise reach "failed" and start a + // real reconnect loop of its own, against a hub that is not there. + tp._closed = true; + try { if (tp._pc) tp._pc.close(); } catch (e) { /* already gone */ } + // After the close event, not before: the real channel's `onclose` is + // delivered as its own task and sets `_connected` back to false, so a + // stand-in installed ahead of it is undone a tick later. await wait(100); + tp._channel = { readyState: 'open', send() {} }; + tp._connected = true; + await wait(50); 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 @@ -312,7 +337,13 @@ async function runScenario(name, chatReply, duringSession) { } await wait(100); snap('a message arrived meanwhile'); - tp._setDevicePk(DEVICE_PK_B64, 'device_hello_ack'); + // What the handshake leaves behind, and all `_announceDevice` needs of + // it: connect() above already set `_userId`. + tp._nonceNode = hex('00'.repeat(32)); + tp.nodePk = 'a-node-public-key'; + const pk = await tp._announceDevice(); + log.push('reconnect: _announceDevice settled on ' + + (pk === DEVICE_PK_B64 ? 'the device key' : JSON.stringify(pk))); await wait(100); snap('device identity restored'); })); @@ -326,6 +357,16 @@ class H(http.server.BaseHTTPRequestHandler): pass def do_POST(self): + # Only the measurement. The `reconnect` scenario drives the real + # `connect()`, which POSTs its offer to the hub's signaling endpoint -- + # there is no hub here, and answering that with 204 both swallowed the + # measurement and put the machine's own SDP (public address included) + # into the probe's output. It gets a 404, which is what makes connect() + # stop where the scenario needs it to. + if self.path != "/log": + self.send_response(404) + self.end_headers() + return RECORDS.append(json.loads( self.rfile.read(int(self.headers["Content-Length"])).decode())) self.send_response(204) diff --git a/packages/meshbay-hub/tests/test_chat_send.py b/packages/meshbay-hub/tests/test_chat_send.py index d442383..c1a7f6f 100644 --- a/packages/meshbay-hub/tests/test_chat_send.py +++ b/packages/meshbay-hub/tests/test_chat_send.py @@ -42,6 +42,7 @@ import pytest HARNESS = Path(__file__).parent / "harness" / "chat_send_probe.py" STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" +GROUP_PAGE = STATIC / "group-page.js" pytestmark = pytest.mark.skipif( shutil.which("google-chrome") is None or not (STATIC / "chat-app.js").exists(), @@ -215,6 +216,53 @@ def test_sending_works_again_after_a_reconnect(probe): "the text came back into the composer, so the send failed") +def test_the_reconnect_scenario_drives_the_shipped_path(probe): + """The scenario has to be worth what it claims. + + It would be easy to write one that sets `devicePk` itself at both ends and + proves only that the composer follows a variable. These three lines say the + clear came out of the real `connect()`, that the restore came out of the + real `_announceDevice`, and that the re-identification actually went over + the wire as a `device_hello`. + """ + data, _ = probe + log = data["log"] + assert any("connect() stopped at signaling" in line for line in log), ( + "the scenario never ran the real connect(), so it did not test the " + "reconnect path at all") + assert any("sent device_hello" == line for line in log), ( + "no device_hello reached the stand-in node -- the identity was not " + "re-announced, it was assigned") + assert any("_announceDevice settled on the device key" in line for line in log), ( + "_announceDevice did not settle on the key it signed with") + + +def test_the_page_tells_the_composer_when_the_identity_moves(): + """The one seam the probe stands in for. + + `Host` in the harness plays group-page.js, so a green probe proves that + ChatPanel and the transport agree — not that the page joins them. These are + the lines that do, and the order matters: the callback has to be wired + before `connect()`, because `connect()` is where `device_hello` runs, and a + callback set after it misses the first answer and starts the composer shut. + """ + src = GROUP_PAGE.read_text(encoding="utf-8") + # The assignment, not a mention of it. Matching the bare name passed with + # the wiring deleted, on the strength of a comment that named it. + assert "transport.onDeviceIdentity = " in src, ( + "nothing tells the page that the device identity moved, so the " + "composer has no event to open back up on -- the freeze this file's " + "reconnect scenario is about") + assert "deviceReady," in src, ( + "the answer never reaches the apps: ChatPanel defaults the prop to " + "true, so a composer wired this way is merely never closed rather " + "than correct") + assert src.index("transport.onDeviceIdentity = ") \ + < src.index("await transport.connect("), ( + "the callback is wired after connect(), which is where device_hello " + "runs -- its answer is missed and the composer starts closed") + + def test_history_still_renders(probe): """ Not about sending at all, and here because it broke without a sound: -- cgit v1.2.3