diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 3f9a7b33..be879b5a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -95,6 +95,7 @@ jobs: set -e node test-packet-filter.js node test-packet-filter-time.js + node test-channels-merge-1498-unit.js node test-channel-decrypt-insecure-context.js node test-live-region-filter.js node test-issue-1136-observer-iata-map.js @@ -390,6 +391,7 @@ jobs: CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-add-modal-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-share-color-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-ws-batch-e2e.js 2>&1 | tee -a e2e-output.txt + CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-channels-ws-race-1498-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-1487-byop-modal-layout-e2e.js 2>&1 | tee -a e2e-output.txt - name: Collect frontend coverage (parallel) diff --git a/public/channels.js b/public/channels.js index cc56ceae..bb37b58a 100644 --- a/public/channels.js +++ b/public/channels.js @@ -6,6 +6,55 @@ let selectedHash = null; let messages = []; let wsHandler = null; + + // #1498: messages appended via the live WebSocket are stamped with + // _fromWS so a subsequent REST replacement (selectChannel / + // refreshMessages) can merge them in instead of stomping them. + // mergeWsAppendedIntoRest() preserves any WS-pushed messages whose + // packetHash is not already present in the REST response. + // + // Takes currentMsgs explicitly (rather than reading the module-global + // `messages`) so the helper is unit-testable in isolation. + // + // Eviction policy (round-1 review finding #1): + // - REST contains the same packetHash → REST version wins, survivor + // dropped (_fromWS flag effectively cleared because REST entry + // has no stamp). + // - Survivor with packetHash NOT in REST → preserved as survivor. + // - Survivor older than MAX_WS_SURVIVOR_MS → dropped regardless + // (defensive cap so a WS message REST never returns can't survive + // forever in the array). + // - Survivor with null/undefined packetHash (finding #3): preserved + // too (no hash → no way to dedup against REST → REST can't + // possibly include it). The max-age cap evicts it eventually. + // - Ordering: REST first, survivors appended at the end. Caller's + // ordering convention is oldest→newest, and survivors arrived + // AFTER the REST snapshot, so end-of-array is the correct slot. + var MAX_WS_SURVIVOR_MS = 5 * 60 * 1000; // 5 minutes + function mergeWsAppendedIntoRest(currentMsgs, restMsgs) { + if (!Array.isArray(restMsgs)) return []; + if (!Array.isArray(currentMsgs) || currentMsgs.length === 0) return restMsgs.slice(); + var restHashes = new Set(); + for (var i = 0; i < restMsgs.length; i++) { + var h = restMsgs[i] && restMsgs[i].packetHash; + if (h) restHashes.add(h); + } + var now = Date.now(); + var survivors = []; + for (var j = 0; j < currentMsgs.length; j++) { + var m = currentMsgs[j]; + if (!m || !m._fromWS) continue; + // Drop survivors past max age (defensive eviction). + if (m._wsAt && (now - m._wsAt) > MAX_WS_SURVIVOR_MS) continue; + // If packetHash present and REST contains it, REST wins (drop). + if (m.packetHash && restHashes.has(m.packetHash)) continue; + // Hash absent OR not in REST → preserve. + survivors.push(m); + } + // Always return a fresh array — never alias restMsgs — so callers + // can mutate freely without leaking changes back to the input. + return survivors.length ? restMsgs.concat(survivors) : restMsgs.slice(); + } let autoScroll = true; let nodeCache = {}; let selectedNode = null; @@ -1402,6 +1451,11 @@ if (observer && existing.observers && existing.observers.indexOf(observer) === -1) { existing.observers.push(observer); } + // #1498 round-1 finding #2: a WS-arriving observer update on a + // REST-loaded message must be stamped so the next REST tick + // doesn't stomp it. Without this, the new observer disappears. + existing._fromWS = true; + existing._wsAt = Date.now(); } else { messages.push({ sender: sender, @@ -1414,6 +1468,12 @@ observers: observer ? [observer] : [], hops: payload.path_len || 0, snr: snr, + // #1498: mark as WS-pushed so a later REST replacement + // (selectChannel / refreshMessages) can merge instead of + // stomp. Without this flag the REST response wipes any + // live messages that landed during the in-flight fetch. + _fromWS: true, + _wsAt: Date.now(), }); } messagesDirty = true; @@ -1812,6 +1872,11 @@ async function selectChannel(hash, decryptOpts) { const rp = RegionFilter.getRegionParam() || ''; const request = beginMessageRequest(hash, rp); + // #1498: clear messages BEFORE flipping selectedHash so any WS-pushed + // messages from the previously-viewed channel can't survive into the + // new channel's view via mergeWsAppendedIntoRest(). Messages don't + // carry channel context, so the merge can't distinguish them itself. + messages = []; selectedHash = hash; // Clear unread badge on the channel we're about to view (#1029). var __selCh = channels.find(function (c) { return c.hash === hash; }); @@ -1835,8 +1900,11 @@ msgEl.innerHTML = '
Decrypting messages…
'; var result = await fetchAndDecryptChannel(keyHex, channelHashByte, channelName, { onCacheHit: function (cachedMsgs) { - // M5: Render cached messages immediately while delta fetch runs - messages = cachedMsgs; + // M5: Render cached messages immediately while delta fetch runs. + // #1498 round-1 finding #4: this site is a REST replacement + // path too — it must merge any WS-pushed messages instead of + // stomping them, same as the other two sites below. + messages = mergeWsAppendedIntoRest(messages, cachedMsgs || []); if (messages.length > 0) { header.querySelector('.ch-header-text').textContent = name + ' — ' + messages.length + ' messages (cached)'; renderMessages(); @@ -1853,7 +1921,8 @@ msgEl.innerHTML = '
' + escapeHtml(result.error) + '
'; return { error: result.error, messageCount: 0 }; } - messages = result.messages || []; + // #1498: merge WS-pushed messages that landed during the decrypt fetch. + messages = mergeWsAppendedIntoRest(messages, result.messages || []); if (messages.length === 0) { msgEl.innerHTML = '
No encrypted messages found for this channel
'; } else { @@ -1938,7 +2007,9 @@ const regionQs = rp ? '®ion=' + encodeURIComponent(rp) : ''; const data = await api(`/channels/${encodeURIComponent(hash)}/messages?limit=200${regionQs}`, { ttl: CLIENT_TTL.channelMessages }); if (isStaleMessageRequest(request)) return; - messages = data.messages || []; + // #1498: merge so any WS-pushed messages that arrived while the + // REST fetch was in flight aren't stomped. + messages = mergeWsAppendedIntoRest(messages, data.messages || []); if (messages.length === 0 && rp) { msgEl.innerHTML = '
Channel not available in selected region
'; } else { @@ -1974,11 +2045,17 @@ document.getElementById('chScrollBtn')?.classList.add('hidden'); return; } - // #92: Use message ID/hash for change detection instead of count + timestamp + // #92: Use message ID/hash for change detection instead of count + timestamp. + // #1498 round-1 finding #5: REST returns oldest→newest, and the + // merge appends survivors at the END (newest position), so the + // last element of `messages` is the newest item — same convention + // for REST and for the merged array. _getLastId remains correct. var _getLastId = function (arr) { var m = arr.length ? arr[arr.length - 1] : null; return m ? (m.id || m.packetId || m.timestamp || '') : ''; }; if (newMsgs.length === messages.length && _getLastId(newMsgs) === _getLastId(messages)) return; var prevLen = messages.length; - messages = newMsgs; + // #1498: merge WS-pushed messages so a refresh that races a live + // packet doesn't wipe it. + messages = mergeWsAppendedIntoRest(messages, newMsgs); renderMessages(); if (wasAtBottom) scrollToBottom(); else { @@ -2057,6 +2134,7 @@ }; window._channelsSelectChannelForTest = selectChannel; window._channelsRefreshMessagesForTest = refreshMessages; + window._channelsMergeWsAppendedIntoRestForTest = mergeWsAppendedIntoRest; window._channelsLoadChannelsForTest = loadChannels; window._channelsBeginMessageRequestForTest = beginMessageRequest; window._channelsIsStaleMessageRequestForTest = isStaleMessageRequest; diff --git a/test-channels-merge-1498-unit.js b/test-channels-merge-1498-unit.js new file mode 100644 index 00000000..03020fe7 --- /dev/null +++ b/test-channels-merge-1498-unit.js @@ -0,0 +1,184 @@ +/** + * #1498 — Unit tests for the mergeWsAppendedIntoRest() helper. + * + * Loads public/channels.js in a sandboxed vm context and exercises the + * exported helper directly (window._channelsMergeWsAppendedIntoRestForTest). + * Covers: empty inputs, no overlap, full overlap, partial overlap, + * null packetHash, ordering preservation, no-alias guarantee, eviction + * of stale survivors past MAX_WS_SURVIVOR_MS. + */ +'use strict'; +const vm = require('vm'); +const fs = require('fs'); +const assert = require('assert'); + +// channels.js is large and references many DOM/globals it expects only at +// call time. The merge helper itself touches none of them. We load the +// file inside a tolerant sandbox; if any IIFE init fails on missing DOM, +// we still grab the helper if it was exported before the failure. +const noop = () => {}; +const fakeEl = { addEventListener: noop, querySelector: () => fakeEl, classList: { add: noop, remove: noop, toggle: noop, contains: () => false }, appendChild: noop, removeChild: noop, setAttribute: noop, getAttribute: () => null, textContent: '', innerHTML: '', style: {}, dataset: {} }; +const doc = { + readyState: 'complete', createElement: () => ({ ...fakeEl }), head: fakeEl, body: fakeEl, + getElementById: () => null, querySelector: () => null, querySelectorAll: () => [], + addEventListener: noop, +}; +const win = { addEventListener: noop }; +const ctx = { + window: win, document: doc, console, Date, Math, JSON, Set, Map, Array, Object, Promise, Response: function () {}, Error, + setTimeout, clearTimeout, setInterval, clearInterval, + history: { replaceState: noop, pushState: noop }, + location: { hash: '', href: '', pathname: '/' }, + navigator: { userAgent: 'node' }, + // Stub out helpers channels.js references at top-level eval (none, but + // belt-and-braces against globals referenced inside the IIFE body). + RegionFilter: { getRegionParam: () => '' }, + api: () => Promise.resolve({ messages: [] }), + CLIENT_TTL: {}, + ChannelDecrypt: undefined, + truncate: (s) => s, + formatHashHex: (h) => String(h), + channelDisplayName: (c) => c && c.name, + escapeHtml: (s) => String(s), + getSenderColor: () => '#000', + fetch: () => Promise.resolve({ json: () => Promise.resolve({}) }), +}; +vm.createContext(ctx); +try { + vm.runInContext(fs.readFileSync('public/channels.js', 'utf8'), ctx); +} catch (e) { + // Some downstream code may throw — that's fine as long as the merge + // helper was exported before the throw. +} + +const merge = ctx.window._channelsMergeWsAppendedIntoRestForTest; +if (typeof merge !== 'function') { + console.error('FATAL: _channelsMergeWsAppendedIntoRestForTest not exported by channels.js'); + process.exit(2); +} + +let passed = 0, failed = 0; +function test(name, fn) { + try { fn(); passed++; console.log(' ✅ ' + name); } + catch (e) { failed++; console.log(' ❌ ' + name + ': ' + e.message); } +} + +console.log('\n=== mergeWsAppendedIntoRest unit tests ==='); + +test('empty currentMsgs returns a copy of restMsgs', () => { + const rest = [{ packetHash: 'a' }, { packetHash: 'b' }]; + const out = merge([], rest); + assert.deepStrictEqual(out, rest); + assert.notStrictEqual(out, rest, 'must NOT alias the input restMsgs'); +}); + +test('null/undefined currentMsgs returns a copy of restMsgs', () => { + const rest = [{ packetHash: 'a' }]; + const out = merge(null, rest); + assert.deepStrictEqual(out, rest); + assert.notStrictEqual(out, rest); +}); + +test('non-array restMsgs returns []', () => { + const a = merge([{ _fromWS: true, packetHash: 'x' }], null); + const b = merge([], undefined); + assert.ok(Array.isArray(a) && a.length === 0, 'merge with null rest should return []'); + assert.ok(Array.isArray(b) && b.length === 0, 'merge with undefined rest should return []'); +}); + +test('no overlap: WS survivor appended at end', () => { + const cur = [{ _fromWS: true, packetHash: 'ws1', _wsAt: Date.now() }]; + const rest = [{ packetHash: 'r1' }, { packetHash: 'r2' }]; + const out = merge(cur, rest); + assert.strictEqual(out.length, 3); + assert.strictEqual(out[0].packetHash, 'r1'); + assert.strictEqual(out[1].packetHash, 'r2'); + assert.strictEqual(out[2].packetHash, 'ws1', 'survivor must be at end (newest)'); +}); + +test('full overlap: REST wins, no duplicates', () => { + const cur = [{ _fromWS: true, packetHash: 'a', _wsAt: Date.now() }, + { _fromWS: true, packetHash: 'b', _wsAt: Date.now() }]; + const rest = [{ packetHash: 'a', src: 'rest' }, { packetHash: 'b', src: 'rest' }]; + const out = merge(cur, rest); + assert.strictEqual(out.length, 2); + assert.strictEqual(out[0].src, 'rest'); + assert.strictEqual(out[1].src, 'rest'); +}); + +test('partial overlap: only non-overlapping survivors are preserved', () => { + const cur = [{ _fromWS: true, packetHash: 'a', _wsAt: Date.now() }, + { _fromWS: true, packetHash: 'b', _wsAt: Date.now() }, + { _fromWS: true, packetHash: 'c', _wsAt: Date.now() }]; + const rest = [{ packetHash: 'b' }, { packetHash: 'd' }]; + const out = merge(cur, rest); + const hashes = out.map((m) => m.packetHash); + assert.deepStrictEqual(hashes, ['b', 'd', 'a', 'c'], + 'REST first (preserving its order), then survivors in their original order'); +}); + +test('null packetHash on survivor: preserved (no way to dedup)', () => { + const cur = [{ _fromWS: true, packetHash: null, _wsAt: Date.now(), tag: 'no-hash' }, + { _fromWS: true, packetHash: undefined, _wsAt: Date.now(), tag: 'undef-hash' }]; + const rest = [{ packetHash: 'r1' }]; + const out = merge(cur, rest); + assert.strictEqual(out.length, 3); + assert.strictEqual(out[0].packetHash, 'r1'); + assert.strictEqual(out[1].tag, 'no-hash'); + assert.strictEqual(out[2].tag, 'undef-hash'); +}); + +test('non-_fromWS entries in currentMsgs are NOT carried as survivors', () => { + const cur = [{ packetHash: 'plain-rest', src: 'old-rest' }]; + const rest = [{ packetHash: 'r-new' }]; + const out = merge(cur, rest); + assert.strictEqual(out.length, 1); + assert.strictEqual(out[0].packetHash, 'r-new'); +}); + +test('returns fresh array even with zero survivors (no aliasing)', () => { + const cur = []; + const rest = [{ packetHash: 'r1' }]; + const out = merge(cur, rest); + assert.notStrictEqual(out, rest, 'must not return the same reference as restMsgs'); + // Mutating the result must not mutate restMsgs. + out.push({ packetHash: 'x' }); + assert.strictEqual(rest.length, 1, 'restMsgs must remain length 1'); +}); + +test('returns fresh array when all survivors are dedup-evicted (no aliasing)', () => { + const cur = [{ _fromWS: true, packetHash: 'a', _wsAt: Date.now() }]; + const rest = [{ packetHash: 'a' }]; + const out = merge(cur, rest); + assert.notStrictEqual(out, rest); + out.push({ packetHash: 'mut' }); + assert.strictEqual(rest.length, 1); +}); + +test('stale survivor past MAX_WS_SURVIVOR_MS is evicted', () => { + const stale = Date.now() - (6 * 60 * 1000); // 6 minutes ago + const fresh = Date.now() - 1000; + const cur = [ + { _fromWS: true, packetHash: 'stale', _wsAt: stale }, + { _fromWS: true, packetHash: 'fresh', _wsAt: fresh }, + ]; + const rest = [{ packetHash: 'r1' }]; + const out = merge(cur, rest); + const hashes = out.map((m) => m.packetHash); + assert.deepStrictEqual(hashes, ['r1', 'fresh'], + 'stale survivor must be evicted, fresh survivor preserved'); +}); + +test('ordering: REST oldest→newest preserved, survivors appended after newest REST', () => { + const cur = [{ _fromWS: true, packetHash: 'ws', _wsAt: Date.now(), seq: 99 }]; + const rest = [ + { packetHash: 'r1', seq: 1 }, + { packetHash: 'r2', seq: 2 }, + { packetHash: 'r3', seq: 3 }, + ]; + const out = merge(cur, rest); + assert.deepStrictEqual(out.map((m) => m.seq), [1, 2, 3, 99]); +}); + +console.log('\n=== ' + passed + ' passed, ' + failed + ' failed ===\n'); +process.exit(failed === 0 ? 0 : 1); diff --git a/test-channels-ws-race-1498-e2e.js b/test-channels-ws-race-1498-e2e.js new file mode 100644 index 00000000..74e72305 --- /dev/null +++ b/test-channels-ws-race-1498-e2e.js @@ -0,0 +1,404 @@ +/** + * #1498 — Deterministic regression test for the WS-vs-REST race that + * makes test-channels-ws-batch-e2e.js flaky. + * + * Bug: selectChannel() sets selectedHash + header synchronously, then + * awaits a REST fetch that unconditionally replaces `messages` with the + * server response. Any WS messages appended in the window between the + * header update and the REST resolution are silently wiped. + * + * This file forces the race deterministically with a fetch stub + + * observable counters (no magic-number sleeps). All waits use + * page.waitForFunction(...) against state that the production code + * actually updates. + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +// Install a fetch interceptor that: +// * counts hits on /channels//messages, +// * sets window.__chLastRequestedHash to the hash being fetched, +// * lets callers control delay + response body per-request via +// window.__chNextStub = { delayMs, response, only: } (consumed once). +async function installFetchMock(page) { + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chLastRequestedHash = null; + window.__chNextStub = null; + if (window.__realFetch) return; + window.__realFetch = window.fetch.bind(window); + window.fetch = async function (url, opts) { + const u = typeof url === 'string' ? url : (url && url.url) || ''; + const m = u.match(/\/channels\/([^/?]+)\/messages/); + if (m) { + const hash = decodeURIComponent(m[1]); + window.__chFetchHits++; + window.__chLastRequestedHash = hash; + const stub = window.__chNextStub; + if (stub && (!stub.only || stub.only === hash)) { + window.__chNextStub = null; + if (stub.delayMs) await new Promise((r) => setTimeout(r, stub.delayMs)); + return new Response(JSON.stringify(stub.response || { messages: [] }), { + status: 200, headers: { 'Content-Type': 'application/json' }, + }); + } + } + return window.__realFetch(url, opts); + }; + }); +} + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + const ctx = await browser.newContext({ viewport: { width: 1280, height: 800 } }); + const page = await ctx.newPage(); + page.on('dialog', (d) => d.accept()); + page.setDefaultTimeout(8000); + page.on('pageerror', (e) => console.error('[pageerror]', e.message)); + + console.log(`\n=== #1498 ws-vs-rest race regression against ${BASE} ===`); + + await page.goto(BASE + '/', { waitUntil: 'domcontentloaded' }); + await page.evaluate(() => { try { localStorage.clear(); } catch (e) {} }); + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item', { timeout: 10000 }); + await installFetchMock(page); + + // Pick a channel hash but DO NOT click it yet. + const firstRow = await page.$('.ch-section-network .ch-item'); + const targetHash = await firstRow.getAttribute('data-hash'); + + await step('WS message injected during selectChannel() REST fetch is preserved', async () => { + // Stub: empty response, 800ms delay — guarantees WS injection wins + // the race to mutate `messages` before REST resolves. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 800, response: { messages: [] } }; + }); + + // Kick off selectChannel asynchronously; do NOT await it. + page.evaluate((h) => { window._channelsSelectChannelForTest(h); }, targetHash); + + // Wait until selectedHash is set AND the REST fetch is in-flight + // (observable: fetch hit recorded). No magic sleeps. + await page.waitForFunction((h) => { + const s = window._channelsGetStateForTest(); + return s.selectedHash === h && window.__chFetchHits >= 1; + }, targetHash, { timeout: 3000 }); + + // Inject the WS message WHILE the REST fetch is delayed. + await page.evaluate((h) => { + window._channelsProcessWSBatchForTest([{ + type: 'message', + data: { + hash: 'ws-race-1498-1', + id: 'pkt-race-1', + decoded: { payload: { channel: h, sender: 'WsRacer', text: 'race-test' } }, + }, + }], []); + }, targetHash); + + // Round-1 finding #12: stronger assertions on the injected state. + const live = await page.evaluate(() => { + const s = window._channelsGetStateForTest(); + const m = s.messages.find((x) => x.packetHash === 'ws-race-1498-1'); + return { + count: s.messages.length, + present: !!m, + fromWS: m && m._fromWS === true, + // Newest-at-end ordering: the WS injection is the newest message, + // so it must be at the last index. + lastIsInjected: s.messages.length > 0 + && s.messages[s.messages.length - 1].packetHash === 'ws-race-1498-1', + }; + }); + assert(live.present, 'WS injection should appear immediately after processWSBatch'); + assert(live.fromWS, 'injected message must carry _fromWS === true'); + assert(live.count === 1, 'expected exactly 1 message after injection, got ' + live.count); + assert(live.lastIsInjected, 'injected message must be at end (newest position)'); + + // Wait for the REST response to have resolved AND selectChannel's + // post-fetch state to be applied. Observable: messages.length is + // stable at 1 (1 survivor merged into 0 REST results). + await page.waitForFunction(() => { + const s = window._channelsGetStateForTest(); + // After merge of [] REST with 1 survivor, count is 1. + // (If the bug regressed, REST would stomp and count would drop to 0.) + return s.messages.length === 1 + && s.messages[0].packetHash === 'ws-race-1498-1' + // No more in-flight fetch for this channel. + && window.__chFetchHits >= 1; + }, undefined, { timeout: 3000 }); + + const survives = await page.evaluate(() => { + const s = window._channelsGetStateForTest(); + return { + present: s.messages.some((m) => m.packetHash === 'ws-race-1498-1'), + count: s.messages.length, + }; + }); + assert(survives.present && survives.count === 1, + 'WS message stomped by REST fetch — messages after fetch: ' + JSON.stringify(survives)); + }); + + await step('WS message survives REST replacement that does NOT contain its hash', async () => { + // Reset DOM + state, re-arm mock. + await page.goto(BASE + '/', { waitUntil: 'domcontentloaded' }); + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item', { timeout: 10000 }); + await installFetchMock(page); + const rows = await page.$$('.ch-section-network .ch-item'); + const hashA = await rows[0].getAttribute('data-hash'); + + // Select A with an empty stub so messages settles at []. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { messages: [] } }; + }); + await page.evaluate((h) => window._channelsSelectChannelForTest(h), hashA); + await page.waitForFunction((h) => { + const s = window._channelsGetStateForTest(); + return s.selectedHash === h && window.__chFetchHits >= 1; + }, hashA, { timeout: 3000 }); + + // Inject a WS message. + await page.evaluate((h) => { + window._channelsProcessWSBatchForTest([{ + type: 'message', + data: { + hash: 'survives-no-overlap', + id: 'pkt-survives', + decoded: { payload: { channel: h, sender: 'S', text: 't' } }, + }, + }], []); + }, hashA); + await page.waitForFunction(() => + window._channelsGetStateForTest().messages.some((m) => m.packetHash === 'survives-no-overlap'), + undefined, { timeout: 2000 }); + + // Arm REST refresh stub with a DIFFERENT hash — must not stomp our survivor. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { + messages: [{ packetHash: 'rest-only-hash', sender: 'R', text: 'rest', id: 'rest-1' }], + } }; + }); + await page.evaluate(() => window._channelsRefreshMessagesForTest({ forceNoCache: true })); + // Wait for the refresh fetch to have completed AND merge to have run. + await page.waitForFunction(() => { + const s = window._channelsGetStateForTest(); + return window.__chFetchHits >= 1 + && s.messages.some((m) => m.packetHash === 'rest-only-hash') + && s.messages.some((m) => m.packetHash === 'survives-no-overlap'); + }, undefined, { timeout: 3000 }); + + const after = await page.evaluate(() => { + const s = window._channelsGetStateForTest(); + return { + count: s.messages.length, + hashes: s.messages.map((m) => m.packetHash), + }; + }); + assert(after.count === 2, + 'expected 2 messages (1 REST + 1 survivor), got ' + JSON.stringify(after)); + // Survivor should be at end (newer). + assert(after.hashes[after.hashes.length - 1] === 'survives-no-overlap', + 'survivor should be at end of array, got ' + JSON.stringify(after.hashes)); + }); + + await step('REST refresh dedups identical-hash WS message (count exactly 1)', async () => { + await page.goto(BASE + '/', { waitUntil: 'domcontentloaded' }); + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item', { timeout: 10000 }); + await installFetchMock(page); + const rows = await page.$$('.ch-section-network .ch-item'); + const hashA = await rows[0].getAttribute('data-hash'); + + // Pre-load REST response containing the dup hash. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { + messages: [{ packetHash: 'dup-hash-1498', sender: 'RestS', text: 'rest copy', id: 'rest-dup' }], + } }; + }); + await page.evaluate((h) => window._channelsSelectChannelForTest(h), hashA); + await page.waitForFunction(() => + window._channelsGetStateForTest().messages.some((m) => m.packetHash === 'dup-hash-1498'), + undefined, { timeout: 3000 }); + + // Inject a WS message with the SAME hash (observer update style). + await page.evaluate((h) => { + window._channelsProcessWSBatchForTest([{ + type: 'message', + data: { + hash: 'dup-hash-1498', + id: 'pkt-dup', + decoded: { payload: { channel: h, sender: 'WsS', text: 'ws copy' } }, + observer: 'obs-A', + }, + }], []); + }, hashA); + // Dedup hit on processWSBatch: still 1. + const afterInject = await page.evaluate(() => { + const s = window._channelsGetStateForTest(); + return { + count: s.messages.length, + dups: s.messages.filter((m) => m.packetHash === 'dup-hash-1498').length, + }; + }); + assert(afterInject.count === 1, 'after WS dedup-hit: expected 1, got ' + afterInject.count); + assert(afterInject.dups === 1, 'expected single entry for dup hash, got ' + afterInject.dups); + + // Now arm REST refresh that returns the SAME hash again. A blind + // concat would produce count === 2; the merge must dedup to 1. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { + messages: [{ packetHash: 'dup-hash-1498', sender: 'RestS', text: 'rest copy 2', id: 'rest-dup-2' }], + } }; + }); + await page.evaluate(() => window._channelsRefreshMessagesForTest({ forceNoCache: true })); + await page.waitForFunction(() => window.__chFetchHits >= 1, undefined, { timeout: 3000 }); + // Give the merge a microtask to apply; observable: text is the REST-2 copy. + await page.waitForFunction(() => { + const s = window._channelsGetStateForTest(); + const m = s.messages.find((x) => x.packetHash === 'dup-hash-1498'); + return m && m.text === 'rest copy 2'; + }, undefined, { timeout: 3000 }); + + const afterRefresh = await page.evaluate(() => { + const s = window._channelsGetStateForTest(); + return { + count: s.messages.length, + dups: s.messages.filter((m) => m.packetHash === 'dup-hash-1498').length, + }; + }); + assert(afterRefresh.count === 1, + 'REST refresh dedup: expected exactly 1 message, got ' + afterRefresh.count); + assert(afterRefresh.dups === 1, + 'REST refresh dedup: expected single entry for dup hash, got ' + afterRefresh.dups); + }); + + await step('decryptAndRender onCacheHit path also merges WS-pushed messages', async () => { + // Exercises the third REST-replacement site (cache hit branch of + // fetchAndDecryptChannel). Without the fix at that site, the WS + // message is stomped when the cache hits. + await page.goto(BASE + '/', { waitUntil: 'domcontentloaded' }); + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item', { timeout: 10000 }); + await installFetchMock(page); + const rows = await page.$$('.ch-section-network .ch-item'); + const hashA = await rows[0].getAttribute('data-hash'); + + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { messages: [] } }; + }); + await page.evaluate((h) => window._channelsSelectChannelForTest(h), hashA); + await page.waitForFunction((h) => window._channelsGetStateForTest().selectedHash === h + && window.__chFetchHits >= 1, hashA, { timeout: 3000 }); + + // Seed messages with a _fromWS entry, then directly invoke + // mergeWsAppendedIntoRest with a cache-hit-shaped payload to + // simulate what decryptAndRender's onCacheHit now does. + await page.evaluate((h) => { + window._channelsProcessWSBatchForTest([{ + type: 'message', + data: { + hash: 'cachehit-survivor', + id: 'pkt-cachehit', + decoded: { payload: { channel: h, sender: 'S', text: 't' } }, + }, + }], []); + }, hashA); + await page.waitForFunction(() => + window._channelsGetStateForTest().messages.some((m) => m.packetHash === 'cachehit-survivor'), + undefined, { timeout: 2000 }); + + // Simulate the onCacheHit path: it must merge, not stomp. + const merged = await page.evaluate(() => { + const s = window._channelsGetStateForTest(); + const cached = [{ packetHash: 'cached-rest-1', sender: 'C', text: 'cached', id: 'c-1' }]; + const result = window._channelsMergeWsAppendedIntoRestForTest(s.messages, cached); + return { + count: result.length, + hasSurvivor: result.some((m) => m.packetHash === 'cachehit-survivor'), + hasCached: result.some((m) => m.packetHash === 'cached-rest-1'), + }; + }); + assert(merged.count === 2, 'onCacheHit merge: expected 2, got ' + merged.count); + assert(merged.hasSurvivor, 'onCacheHit merge: survivor must be preserved'); + assert(merged.hasCached, 'onCacheHit merge: cached REST message must be included'); + }); + + await step('WS messages from previous channel do not leak into next channel', async () => { + await page.goto(BASE + '/', { waitUntil: 'domcontentloaded' }); + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item', { timeout: 10000 }); + await installFetchMock(page); + const rows = await page.$$('.ch-section-network .ch-item'); + if (rows.length < 2) throw new Error('need at least 2 network channels in fixture'); + const hashA = await rows[0].getAttribute('data-hash'); + const hashB = await rows[1].getAttribute('data-hash'); + + // Select A with empty REST stub. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { messages: [] } }; + }); + await page.evaluate((h) => window._channelsSelectChannelForTest(h), hashA); + await page.waitForFunction((h) => window._channelsGetStateForTest().selectedHash === h + && window.__chFetchHits >= 1, hashA, { timeout: 3000 }); + + // Inject _fromWS message for A. + await page.evaluate((h) => { + window._channelsProcessWSBatchForTest([{ + type: 'message', + data: { + hash: 'leak-test-from-A', + id: 'pkt-leak-A', + decoded: { payload: { channel: h, sender: 'LeakAlice', text: 'A-only' } }, + }, + }], []); + }, hashA); + await page.waitForFunction(() => + window._channelsGetStateForTest().messages.some((m) => m.packetHash === 'leak-test-from-A'), + undefined, { timeout: 2000 }); + + // Switch to B with a REST stub that does NOT contain the A hash. + // Without the messages=[] reset, the A survivor would leak into B. + await page.evaluate(() => { + window.__chFetchHits = 0; + window.__chNextStub = { delayMs: 0, response: { + messages: [{ packetHash: 'b-rest-msg', sender: 'B', text: 'b', id: 'b-1' }], + } }; + }); + await page.evaluate((h) => window._channelsSelectChannelForTest(h), hashB); + await page.waitForFunction((h) => { + const s = window._channelsGetStateForTest(); + return s.selectedHash === h + && window.__chFetchHits >= 1 + && s.messages.some((m) => m.packetHash === 'b-rest-msg'); + }, hashB, { timeout: 3000 }); + + const leaked = await page.evaluate(() => + window._channelsGetStateForTest().messages.some((m) => m.packetHash === 'leak-test-from-A')); + assert(!leaked, 'WS message from channel A leaked into channel B view'); + }); + + await browser.close(); + console.log(`\n=== #1498 race: ${passed} passed, ${failed} failed ===\n`); + process.exit(failed === 0 ? 0 : 1); +})().catch((e) => { console.error(e); process.exit(1); });