From 1a9d043e9f76529bf0587b8a0ae34530dc78af26 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 30 Sep 2026 13:54:32 +0000 Subject: [PATCH 01/23] test(channels): reproduce client-only state loss on channel-list refresh Relates to #152. loadChannels() replaces the channel list with the server snapshot and carries nothing across. These tests drive the real loadChannels(), the real WS path and the real init() handlers (region change, show-encrypted toggle, shared-channel approval) through the existing test hooks, and a Playwright check adds a PSK through the Add Channel modal and switches region at desktop and mobile width. On master, 11 of 15 unit tests and 4 of 6 E2E steps fail with the reported symptoms: the PSK selection is cleared and the URL rewritten to #/channels, My Channels and labels vanish, unread and the user:* preview reset, and a WS update that lands during an in-flight request is lost. The four passing unit tests are guards for the fix (omitted rows are not resurrected, a stale client row does not beat a newer snapshot). Registered in the existing unit and Playwright steps of deploy.yml and in test-all.sh. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DxHDRtHRCQxS6AzzDXXSJi --- .github/workflows/deploy.yml | 2 + test-all.sh | 1 + test-channels-client-state-152-e2e.js | 127 +++++++ test-channels-client-state-152.js | 497 ++++++++++++++++++++++++++ 4 files changed, 627 insertions(+) create mode 100644 test-channels-client-state-152-e2e.js create mode 100644 test-channels-client-state-152.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index d3262947b..c29ba9470 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -146,6 +146,7 @@ jobs: node test-app-api-inflight-cleanup-rejection.js node test-channels-merge-1498-unit.js node test-channels-observed-path-hash-size.js + node test-channels-client-state-152.js node test-issue-1518-home-url.js node test-channel-decrypt-insecure-context.js node test-live-region-filter.js @@ -480,6 +481,7 @@ jobs: BASE_URL=http://localhost:13581 node test-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-channel-issue-1087-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-channel-issue-1111-e2e.js 2>&1 | tee -a e2e-output.txt + BASE_URL=http://localhost:13581 node test-channels-client-state-152-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-map-modal-fluid-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-map-nodes-pagination-e2e.js 2>&1 | tee -a e2e-output.txt CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-123-map-lifecycle-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/test-all.sh b/test-all.sh index a46a53639..d9c06474a 100755 --- a/test-all.sh +++ b/test-all.sh @@ -102,6 +102,7 @@ node test-node-analytics-hop-chart.js node test-analytics-hop-depth-ui.js node test-channels-ping-bot-reply.js node test-channels-observed-path-hash-size.js +node test-channels-client-state-152.js node test-packet-path-map.js node test-area-nodes-map.js node test-ping-scores.js diff --git a/test-channels-client-state-152-e2e.js b/test-channels-client-state-152-e2e.js new file mode 100644 index 000000000..ae0ede296 --- /dev/null +++ b/test-channels-client-state-152-e2e.js @@ -0,0 +1,127 @@ +/** + * #152 — E2E: an open PSK conversation and My Channels survive a region + * change on the Channels page. + * + * Adds a test PSK through the Add Channel modal (the real UI path), which + * selects the new user:* channel, then switches region with the region + * pills. Before the fix the refresh evicted the user:* selection: the + * conversation closed ("Choose a channel from the sidebar"), the URL was + * rewritten to #/channels and the My Channels section disappeared. + * + * Runs at desktop and mobile width. + * + * Usage: BASE_URL=http://localhost:13581 node test-channels-client-state-152-e2e.js + */ +'use strict'; + +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const PSK_KEY = '0152c0de0152c0de0152c0de0152c0de'; +const PSK_HASH = 'user:psk:0152c0de'; +const PSK_LABEL = 'Issue152 Team'; + +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'); } + +async function conversationState(page) { + return page.evaluate((hash) => { + const header = document.querySelector('#chHeader .ch-header-text'); + const msgs = document.getElementById('chMessages'); + const row = document.querySelector('#chList [data-hash="' + hash + '"]'); + const mine = document.querySelector('#chList .ch-section-mychannels'); + return { + urlHash: location.hash, + header: header ? header.textContent : '', + messagesText: msgs ? msgs.textContent : '', + rowPresent: !!row, + rowSelected: !!row && (row.getAttribute('aria-selected') === 'true' || row.classList.contains('selected')), + rowInMyChannels: !!(mine && mine.querySelector('[data-hash="' + hash + '"]')), + myChannelsText: mine ? mine.textContent : '', + }; + }, PSK_HASH); +} + +async function switchRegion(page, code) { + const pill = page.locator('#chRegionFilter [data-region="' + code + '"]'); + await pill.waitFor({ state: 'attached', timeout: 8000 }); + // Each step selects a region set not requested before, so api()'s TTL + // cache cannot answer it and a real /api/channels request goes out. + const response = page.waitForResponse((r) => { + const u = decodeURIComponent(r.url()); + return u.indexOf('/api/channels?') !== -1 && u.indexOf('/messages') === -1 && u.indexOf(code) !== -1; + }, { timeout: 8000 }); + // The mobile detail view hides or covers the sidebar's region pills; fall + // back to a DOM click, which still goes through the real RegionFilter + // handler. + const click = pill.click({ timeout: 2000 }).catch(() => pill.evaluate((el) => el.click())); + await Promise.all([response, click]); + // loadChannels() renders and reconciles right after the response resolves. + await page.waitForTimeout(300); +} + +async function runViewport(browser, vp) { + const ctx = await browser.newContext({ viewport: { width: vp.width, height: vp.height } }); + const page = await ctx.newPage(); + page.setDefaultTimeout(8000); + page.on('pageerror', (e) => console.error('[pageerror ' + vp.name + ']', e.message)); + + await page.goto(BASE + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.evaluate(() => { try { localStorage.clear(); } catch (e) {} }); + await page.reload({ waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item, #chList .ch-row', { timeout: 10000 }); + + await step(vp.name + ': adding a PSK in the UI opens its conversation', async () => { + await page.click('#chAddChannelBtn'); + await page.fill('#chPskKey', PSK_KEY); + await page.fill('#chPskName', PSK_LABEL); + await page.click('#chPskAddBtn'); + await page.waitForFunction((hash) => location.hash === '#/channels/' + encodeURIComponent(hash), PSK_HASH, { timeout: 10000 }); + await page.waitForFunction((label) => { + const h = document.querySelector('#chHeader .ch-header-text'); + return h && h.textContent.indexOf(label) !== -1; + }, PSK_LABEL, { timeout: 10000 }); + const s = await conversationState(page); + assert(s.rowPresent && s.rowSelected, 'PSK row must be listed and selected, got ' + JSON.stringify(s)); + if (vp.mobile === false) assert(s.rowInMyChannels, 'PSK row must be in My Channels'); + }); + + const before = await conversationState(page); + + for (const code of ['SJC', 'SFO']) { + await step(vp.name + ': region change to ' + code + ' keeps the PSK conversation and My Channels', async () => { + await switchRegion(page, code); + const s = await conversationState(page); + assert(s.urlHash === before.urlHash, 'URL must stay on the PSK channel, got ' + s.urlHash); + assert(s.header.indexOf(PSK_LABEL) !== -1, 'header must still name the PSK channel, got ' + JSON.stringify(s.header)); + assert(s.messagesText.indexOf('Choose a channel') === -1, 'conversation must not close, got ' + JSON.stringify(s.messagesText.slice(0, 120))); + assert(s.messagesText === before.messagesText, 'message pane must be unchanged'); + assert(s.rowPresent && s.rowSelected, 'PSK row must stay listed and selected, got ' + JSON.stringify(s)); + if (vp.mobile === false) { + assert(s.rowInMyChannels, 'My Channels must still contain the PSK row'); + assert(s.myChannelsText.indexOf(PSK_LABEL) !== -1, 'My Channels must still show the label'); + } + }); + } + + await page.evaluate(() => { try { localStorage.clear(); } catch (e) {} }); + await ctx.close(); +} + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + console.log('\n=== #152 E2E against ' + BASE + ' ==='); + await runViewport(browser, { name: 'desktop', width: 1280, height: 800, mobile: false }); + await runViewport(browser, { name: 'mobile', width: 390, height: 844, mobile: true }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); + await browser.close(); + process.exit(failed > 0 ? 1 : 0); +})().catch((e) => { console.error('FATAL:', e); process.exit(1); }); diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js new file mode 100644 index 000000000..7712808b8 --- /dev/null +++ b/test-channels-client-state-152.js @@ -0,0 +1,497 @@ +/** + * #152 — loadChannels() must keep client-only channel state across a + * channel-list refresh. + * + * loadChannels() replaced `channels` with the server snapshot and carried + * nothing across. Every refresh (region change, show-encrypted toggle, + * shared-channel approval) therefore: + * - closed an open PSK conversation (a user:* hash is never in the server + * snapshot, so reconcileSelectionAfterChannelRefresh() evicted it, + * emptied `messages` and rewrote the URL to #/channels); + * - dropped the My Channels section and the user's labels; + * - reset unread badges and the sidebar preview of user:* rows. + * + * These tests load the real channels.js (plus channel-decrypt.js and + * channel-proposals.js) in a vm sandbox and drive the real loadChannels(), + * the real WS path and the real init() handlers through the existing test + * hooks. Nothing here re-implements production logic. + * + * Usage: node test-channels-client-state-152.js + */ +'use strict'; + +const vm = require('vm'); +const fs = require('fs'); +const path = require('path'); +const assert = require('assert'); +const { webcrypto } = require('crypto'); + +const RealDate = Date; +const HOUR = 60 * 60 * 1000; +const PSK_NAME = 'psk:0badc0de'; +const PSK_HASH = 'user:' + PSK_NAME; +const PSK_KEY = '0badc0de0badc0de0badc0de0badc0de'; + +// A Date whose "now" is shifted by skewMs, to simulate a browser clock that +// runs ahead of (skewMs > 0) or behind (skewMs < 0) the server clock. +function makeSkewedDate(skewMs) { + function SkewedDate() { + const args = Array.prototype.slice.call(arguments); + if (!(this instanceof SkewedDate)) return new RealDate(RealDate.now() + skewMs).toString(); + if (args.length === 0) return new RealDate(RealDate.now() + skewMs); + return new (Function.prototype.bind.apply(RealDate, [null].concat(args)))(); + } + SkewedDate.now = () => RealDate.now() + skewMs; + SkewedDate.parse = RealDate.parse; + SkewedDate.UTC = RealDate.UTC; + SkewedDate.prototype = RealDate.prototype; + return SkewedDate; +} + +function deferred() { + let resolve; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +} + +async function flush(n) { + for (let i = 0; i < (n || 20); i++) await new Promise((r) => setImmediate(r)); +} + +function makeHarness(opts) { + opts = opts || {}; + const storage = {}; + const elements = {}; + function makeFakeEl(id) { + return { + id: id || '', innerHTML: '', textContent: '', value: '', scrollTop: 0, + scrollHeight: 0, clientHeight: 0, + style: {}, dataset: {}, + classList: { add() {}, remove() {}, toggle() {}, contains() { return false; } }, + addEventListener() {}, removeEventListener() {}, + querySelector() { return makeFakeEl(); }, + querySelectorAll() { return []; }, + getAttribute() { return null; }, setAttribute() {}, removeAttribute() {}, + getBoundingClientRect() { return { width: 240, height: 0, top: 0, left: 0, right: 0, bottom: 0 }; }, + appendChild() {}, removeChild() {}, remove() {}, + focus() {}, blur() {}, + checked: false, + }; + } + function el(id) { + if (!elements[id]) elements[id] = makeFakeEl(id); + return elements[id]; + } + + const historyCalls = []; + const windowListeners = {}; + const h = { + elements, + historyCalls, + regionChange: null, + approvedCallback: null, + // Each /channels request is answered by h.respondChannels(path); tests + // replace it to return a deferred promise when they need a request in + // flight. + respondChannels: () => Promise.resolve({ channels: [] }), + channelRequests: [], + }; + + const ctx = { + window: { + addEventListener(type, fn) { (windowListeners[type] = windowListeners[type] || []).push(fn); }, + removeEventListener() {}, + matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }), + }, + document: { + readyState: 'complete', + documentElement: { getAttribute: () => null, setAttribute() {}, classList: { add() {}, remove() {}, toggle() {}, contains() { return false; } } }, + createElement: () => makeFakeEl(), + head: { appendChild() {} }, + body: { appendChild() {}, removeChild() {}, contains() { return false; } }, + getElementById: el, + addEventListener() {}, removeEventListener() {}, + querySelector: () => null, + querySelectorAll: () => [], + }, + console, + Date: makeSkewedDate(opts.skewMs || 0), + Math, Array, Object, String, Number, JSON, RegExp, Error, TypeError, Set, Map, Promise, + parseInt, parseFloat, isNaN, isFinite, + encodeURIComponent, decodeURIComponent, + setTimeout: (fn) => { Promise.resolve().then(fn); return 0; }, + clearTimeout: () => {}, + setInterval: () => 0, + clearInterval: () => {}, + fetch: () => Promise.resolve({ ok: true, json: () => Promise.resolve({}) }), + performance: { now: () => RealDate.now() }, + localStorage: { + getItem: (k) => Object.prototype.hasOwnProperty.call(storage, k) ? storage[k] : null, + setItem: (k, v) => { storage[k] = String(v); }, + removeItem: (k) => { delete storage[k]; }, + }, + location: { hash: '' }, + history: { replaceState(_s, _t, url) { historyCalls.push(url); }, pushState() {} }, + crypto: webcrypto, + TextEncoder, TextDecoder, + Uint8Array, Uint16Array, Uint32Array, Int8Array, Int16Array, Int32Array, ArrayBuffer, + URLSearchParams, + CustomEvent: class CustomEvent {}, + MutationObserver: class MutationObserver { observe() {} disconnect() {} }, + requestAnimationFrame: (cb) => { Promise.resolve().then(cb); return 0; }, + matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }), + addEventListener() {}, dispatchEvent() {}, + getHashParams: () => new URLSearchParams(), + btoa: (s) => Buffer.from(String(s), 'binary').toString('base64'), + atob: (s) => Buffer.from(String(s), 'base64').toString('binary'), + }; + ctx.self = ctx; + ctx.globalThis = ctx; + + // app.js / roles.js stubs. + ctx.onWS = () => {}; + ctx.offWS = () => {}; + ctx.debouncedOnWS = (fn) => fn; + ctx.debounce = (fn) => fn; + ctx.invalidateApiCache = () => {}; + ctx.api = (p) => { + if (p.indexOf('/channels') === 0 && p.indexOf('/messages') === -1) { + h.channelRequests.push(p); + return h.respondChannels(p); + } + if (p.indexOf('/observers') === 0) return Promise.resolve({ observers: [] }); + return Promise.resolve({ messages: [], packets: [], channels: [] }); + }; + ctx.CLIENT_TTL = { channels: 15000, observers: 120000, channelMessages: 10000, nodeDetail: 10000 }; + ctx.escapeHtml = (s) => String(s == null ? '' : s); + ctx.truncate = (s, n) => { s = String(s || ''); return s.length > n ? s.slice(0, n) : s; }; + ctx.formatHashHex = (x) => String(x); + ctx.formatSecondsAgo = () => ''; + ctx.timeAgo = () => ''; + ctx.payloadTypeName = () => 'GRP_TXT'; + ctx.ROLE_EMOJI = {}; + ctx.ROLE_LABELS = {}; + ctx.RegionFilter = { + init() {}, + onChange(fn) { h.regionChange = fn; return fn; }, + offChange() {}, + getRegionParam() { return h.regionParam || ''; }, + getSelected() { return null; }, + }; + ctx.ChannelColors = { get() { return null; }, remove() {} }; + ctx.ChannelColorPicker = { open() {} }; + let pageMod = null; + ctx.registerPage = (name, mod) => { if (name === 'channels') pageMod = mod; }; + + vm.createContext(ctx); + function load(file) { + const src = fs.readFileSync(path.join(__dirname, file), 'utf8'); + vm.runInContext(src, ctx, { filename: file }); + for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k]; + } + load('public/vendor/aes-ecb.js'); + load('public/channel-decrypt.js'); + load('public/channel-proposals.js'); + // Capture the onApproved callback init() hands to ChannelProposals.mount + // without letting mount() touch the DOM or start polling. The pure + // mergeApprovedChannels() stays the real one. + ctx.window.ChannelProposals.mount = (mountOpts) => { h.approvedCallback = mountOpts && mountOpts.onApproved; }; + ctx.window.ChannelProposals.unmount = () => {}; + load('public/channels.js'); + + h.ctx = ctx; + h.w = ctx.window; + h.page = pageMod; + h.showEncryptedChanged = () => (windowListeners['mc-channels-show-encrypted-changed'] || []).forEach((fn) => fn({})); + h.state = () => ctx.window._channelsGetStateForTest(); + h.setState = (s) => ctx.window._channelsSetStateForTest(s); + h.row = (hash) => h.state().channels.find((c) => c.hash === hash); + h.storeKey = (name, key, label) => ctx.window.ChannelDecrypt.storeKey(name, key, label); + h.removeKey = (name) => ctx.window.ChannelDecrypt.removeKey(name); + h.init = async () => { + await pageMod.init(el('page'), null); + await flush(); + historyCalls.length = 0; + }; + // A live CHAN message through the real WS path. + h.liveMessage = (channel, sender, text, hash) => { + ctx.window._channelsProcessWSBatchForTest([{ + type: 'message', + data: { hash: hash || ('pkt-' + channel + '-' + text), decoded: { header: { payloadTypeName: 'GRP_TXT' }, payload: { channel, sender, text: sender + ': ' + text } } }, + }], null); + }; + return h; +} + +function serverChannel(hash, extra) { + return Object.assign({ + hash, + name: hash, + messageCount: 10, + lastActivity: new RealDate(RealDate.now() - 60000).toISOString(), + lastSender: 'Server', + lastMessage: 'from server', + }, extra || {}); +} + +function pskRow(extra) { + return Object.assign({ + hash: PSK_HASH, + name: PSK_NAME, + userLabel: 'Team', + messageCount: 3, + lastActivityMs: RealDate.now() - 5000, + lastSender: 'Bob', + lastMessage: 'decrypted preview', + encrypted: true, + userAdded: true, + }, extra || {}); +} + +// A harness with the page initialised, a stored PSK key and an open PSK +// conversation — the state a user is in when they hit the bug. +async function openPskConversation(opts) { + const h = makeHarness(opts); + h.storeKey(PSK_NAME, PSK_KEY, 'Team'); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public')] }); + await h.init(); + const messages = [ + { sender: 'Alice', text: 'first', timestamp: '2026-01-01T00:00:00Z', packetHash: 'm1' }, + { sender: 'Bob', text: 'second', timestamp: '2026-01-01T00:01:00Z', packetHash: 'm2' }, + ]; + h.setState({ + channels: [Object.assign({}, h.row('public')), pskRow()], + messages, + selectedHash: PSK_HASH, + }); + return { h, messages }; +} + +function assertConversationOpen(h, messages, label) { + const s = h.state(); + assert.strictEqual(s.selectedHash, PSK_HASH, label + ': PSK selection must survive'); + assert.strictEqual(s.messages.length, messages.length, label + ': messages must survive'); + assert.strictEqual(s.messages[0].text, 'first', label + ': messages must be the same conversation'); + assert.ok(!h.historyCalls.includes('#/channels'), label + ': URL must not be rewritten to #/channels (got ' + JSON.stringify(h.historyCalls) + ')'); + const row = h.row(PSK_HASH); + assert.ok(row && row.userAdded === true, label + ': PSK row must stay in My Channels'); + assert.ok(/ch-section-mychannels/.test(h.elements.chList.innerHTML), label + ': rendered list must contain My Channels'); +} + +let passed = 0, failed = 0; +async function test(name, fn) { + try { await fn(); passed++; console.log(' ✅ ' + name); } + catch (e) { failed++; console.log(' ❌ ' + name + ': ' + (e && e.message)); } +} + +(async () => { + console.log('\n=== #152 loadChannels keeps client-only state ==='); + + await test('open PSK conversation survives a silent loadChannels() refresh', async () => { + const { h, messages } = await openPskConversation(); + await h.w._channelsLoadChannelsForTest(true); + assertConversationOpen(h, messages, 'loadChannels'); + }); + + await test('open PSK conversation survives a region change (RegionFilter.onChange handler)', async () => { + const { h, messages } = await openPskConversation(); + h.regionParam = 'CPH'; + assert.strictEqual(typeof h.regionChange, 'function', 'init() must register a region handler'); + h.regionChange(); + await flush(); + assert.ok(h.channelRequests[h.channelRequests.length - 1].indexOf('region=CPH') !== -1, 'region change must refetch with the region'); + assertConversationOpen(h, messages, 'region change'); + }); + + await test('open PSK conversation survives the show-encrypted toggle', async () => { + const { h, messages } = await openPskConversation(); + h.ctx.localStorage.setItem('channels-show-encrypted', 'true'); + h.showEncryptedChanged(); + await flush(); + assert.ok(h.channelRequests[h.channelRequests.length - 1].indexOf('includeEncrypted=true') !== -1, 'toggle must refetch with includeEncrypted'); + assertConversationOpen(h, messages, 'show-encrypted'); + }); + + await test('shared-channel approval (onApproved) keeps the open PSK conversation, one PSK row', async () => { + const { h, messages } = await openPskConversation(); + h.respondChannels = () => Promise.resolve({ + channels: [serverChannel('public')], + approvedChannels: [{ hash: '#shared', name: '#shared' }], + }); + assert.strictEqual(typeof h.approvedCallback, 'function', 'init() must mount ChannelProposals with onApproved'); + const before = h.channelRequests.length; + h.approvedCallback(); + await flush(); + assert.strictEqual(h.channelRequests.length, before + 1, 'approval must refetch the channel list exactly once'); + assertConversationOpen(h, messages, 'onApproved'); + assert.ok(h.row('#shared') && h.row('#shared').shared === true, 'approved channel must be listed'); + assert.strictEqual(h.state().channels.filter((c) => c.hash === PSK_HASH).length, 1, 'PSK row must appear exactly once'); + }); + + await test('My Channels rows and labels survive a refresh (user:* row and key-matched server row)', async () => { + const h = makeHarness(); + h.storeKey(PSK_NAME, PSK_KEY, 'Team'); + h.storeKey('#ops', 'aa'.repeat(16), 'Ops room'); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public'), serverChannel('#ops', { name: '#ops' })] }); + await h.init(); + await h.w._channelsLoadChannelsForTest(true); + const psk = h.row(PSK_HASH); + const ops = h.row('#ops'); + assert.ok(psk && psk.userAdded === true && psk.userLabel === 'Team', 'user:* row keeps userAdded + label'); + assert.ok(ops && ops.userAdded === true && ops.userLabel === 'Ops room', 'server row matched by a stored key keeps userAdded + label'); + assert.ok(!h.row('public').userAdded, 'unrelated server row is not marked userAdded'); + assert.ok(/ch-section-mychannels/.test(h.elements.chList.innerHTML), 'My Channels rendered'); + assert.ok(h.elements.chList.innerHTML.indexOf('Team') !== -1, 'PSK label rendered'); + }); + + await test('unread and preview on a user:* row survive a refresh (explicit 0 on a server row kept)', async () => { + const { h } = await openPskConversation(); + h.setState({ + channels: [Object.assign({}, h.row('public'), { unread: 0 }), pskRow({ unread: 4 })], + selectedHash: null, + messages: [], + }); + await h.w._channelsLoadChannelsForTest(true); + const psk = h.row(PSK_HASH); + assert.strictEqual(psk.unread, 4, 'user:* unread kept (got ' + psk.unread + ')'); + assert.strictEqual(psk.lastMessage, 'decrypted preview', 'user:* preview kept (got ' + psk.lastMessage + ')'); + assert.strictEqual(psk.lastSender, 'Bob', 'user:* preview sender kept'); + assert.strictEqual(psk.messageCount, 3, 'user:* messageCount kept'); + const pub = h.row('public'); + assert.ok(Object.prototype.hasOwnProperty.call(pub, 'unread') && pub.unread === 0, 'explicit unread: 0 kept on a server row'); + assert.ok(/data-unread-channel="user:psk:0badc0de"/.test(h.elements.chList.innerHTML), 'unread badge rendered for the user:* row'); + }); + + await test('unread on a server row survives a refresh', async () => { + const h = makeHarness(); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public')] }); + await h.init(); + h.setState({ channels: [Object.assign({}, h.row('public'), { unread: 7 })] }); + await h.w._channelsLoadChannelsForTest(true); + assert.strictEqual(h.row('public').unread, 7); + }); + + await test('a server channel the new snapshot omits (region filter) does not come back', async () => { + const h = makeHarness(); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public'), serverChannel('#far')] }); + await h.init(); + h.setState({ channels: h.state().channels.map((c) => Object.assign({}, c, { unread: 2 })) }); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public')] }); + await h.w._channelsLoadChannelsForTest(true); + assert.strictEqual(h.row('#far'), undefined, 'omitted row must not be resurrected'); + assert.deepStrictEqual(h.state().channels.map((c) => c.hash), ['public']); + }); + + await test('a user:* row whose key was removed does not come back', async () => { + const { h } = await openPskConversation(); + h.setState({ selectedHash: null, messages: [] }); + h.removeKey(PSK_NAME); + await h.w._channelsLoadChannelsForTest(true); + assert.strictEqual(h.row(PSK_HASH), undefined, 'removed PSK row must not be resurrected from the previous list'); + }); + + for (const skew of [HOUR, -HOUR]) { + const label = skew > 0 ? 'browser clock 1h ahead' : 'browser clock 1h behind'; + + await test('WS update during an in-flight request wins (' + label + ')', async () => { + const h = makeHarness({ skewMs: skew }); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public', { messageCount: 10, lastMessage: 'old snapshot' })] }); + await h.init(); + const pending = deferred(); + h.respondChannels = () => pending.promise; + const load = h.w._channelsLoadChannelsForTest(true); + await flush(); + h.liveMessage('public', 'Carol', 'live during flight'); + const liveRow = Object.assign({}, h.row('public')); + // The snapshot was taken on the server before the live packet arrived. + pending.resolve({ channels: [serverChannel('public', { + messageCount: 10, + lastActivity: new RealDate(RealDate.now() - 1000).toISOString(), + lastSender: 'Server', + lastMessage: 'snapshot', + })] }); + await load; + const row = h.row('public'); + assert.strictEqual(row.lastMessage, 'live during flight', 'live message kept (got ' + row.lastMessage + ')'); + assert.strictEqual(row.lastSender, 'Carol', 'live sender kept with its message'); + assert.strictEqual(row.lastActivityMs, liveRow.lastActivityMs, 'live activity time kept with its message'); + assert.strictEqual(row.messageCount, 11, 'live messageCount kept with its message'); + }); + + await test('a stale client row does not override a newer snapshot (' + label + ')', async () => { + const h = makeHarness({ skewMs: skew }); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public', { messageCount: 10 })] }); + await h.init(); + // Live update BEFORE the refresh starts: the snapshot already has it + // plus a newer message. + h.liveMessage('public', 'Carol', 'older live'); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public', { + messageCount: 12, + lastActivity: new RealDate(RealDate.now() - 1000).toISOString(), + lastSender: 'Dave', + lastMessage: 'newer snapshot', + })] }); + await h.w._channelsLoadChannelsForTest(true); + const row = h.row('public'); + assert.strictEqual(row.lastMessage, 'newer snapshot', 'snapshot message wins (got ' + row.lastMessage + ')'); + assert.strictEqual(row.lastSender, 'Dave', 'snapshot sender wins'); + assert.strictEqual(row.messageCount, 12, 'snapshot count wins'); + }); + } + + await test('loadChannels() does not mutate the api() payload (shared with the api cache)', async () => { + const h = makeHarness(); + h.storeKey('#ops', 'aa'.repeat(16), 'Ops room'); + const payload = { channels: [serverChannel('#ops', { name: '#ops' })] }; + const snapshot = JSON.stringify(payload); + h.respondChannels = () => Promise.resolve(payload); + await h.init(); + await h.w._channelsLoadChannelsForTest(true); + assert.strictEqual(JSON.stringify(payload), snapshot, 'api payload must be left untouched'); + assert.notStrictEqual(h.row('#ops'), payload.channels[0], 'channels must not alias the payload rows'); + }); + + await test('mergeClientChannelState: pure, carries by hash, never resurrects, never mutates inputs', async () => { + const h = makeHarness(); + const merge = h.w._channelsMergeClientChannelStateForTest; + assert.strictEqual(typeof merge, 'function', '_channelsMergeClientChannelStateForTest must be exported'); + const deepFreeze = (o) => { Object.values(o).forEach((v) => { if (v && typeof v === 'object') deepFreeze(v); }); return Object.freeze(o); }; + const fresh = deepFreeze([ + { hash: 'public', name: 'public', lastActivityMs: 100, lastMessage: 'fresh', lastSender: 'S', messageCount: 5 }, + { hash: PSK_HASH, name: PSK_NAME, lastActivityMs: 0, lastMessage: 'Encrypted — click to decrypt', lastSender: '', messageCount: 0, userAdded: true, userLabel: '' }, + ]); + const prev = deepFreeze([ + { hash: 'public', unread: 0, userAdded: true, userLabel: 'Pub', lastActivityMs: 999999, lastMessage: 'stale', lastSender: 'X', messageCount: 99, _wsSeq: 3 }, + { hash: PSK_HASH, unread: 2, userAdded: true, userLabel: 'Team', lastActivityMs: 50, lastMessage: 'preview', lastSender: 'B', messageCount: 1 }, + { hash: 'gone', unread: 9 }, + ]); + const freshCopy = JSON.stringify(fresh); + const prevCopy = JSON.stringify(prev); + const out = merge(fresh, prev, 3); + assert.strictEqual(JSON.stringify(fresh), freshCopy, 'fresh input untouched'); + assert.strictEqual(JSON.stringify(prev), prevCopy, 'prev input untouched'); + assert.notStrictEqual(out, fresh); + out.forEach((o, i) => { assert.notStrictEqual(o, fresh[i], 'rows must be copies'); assert.notStrictEqual(o, prev[i]); }); + assert.deepStrictEqual(out.map((o) => o.hash), ['public', PSK_HASH], 'no resurrected rows, fresh order kept'); + assert.strictEqual(out[0].unread, 0, 'explicit 0 carried'); + assert.strictEqual(out[0].userAdded, true); + assert.strictEqual(out[0].userLabel, 'Pub'); + assert.strictEqual(out[0].lastMessage, 'fresh', 'stamp not newer than request start → snapshot activity wins'); + assert.strictEqual(out[0].messageCount, 5); + assert.strictEqual(out[1].unread, 2); + assert.strictEqual(out[1].userLabel, 'Team'); + assert.strictEqual(out[1].lastMessage, 'preview', 'client-only row keeps its preview'); + const newer = merge(fresh, prev, 2); + assert.strictEqual(newer[0].lastMessage, 'stale', 'stamp newer than request start → live activity wins'); + assert.strictEqual(newer[0].messageCount, 99, 'moved together with its message'); + assert.strictEqual(newer[0].lastSender, 'X'); + assert.strictEqual(newer[0].lastActivityMs, 999999); + assert.deepStrictEqual(merge(null, prev, 0), []); + const noPrev = merge(fresh, null, 0); + assert.deepStrictEqual(JSON.parse(JSON.stringify(noPrev)), JSON.parse(freshCopy)); + assert.notStrictEqual(noPrev[0], fresh[0]); + }); + + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); + process.exit(failed > 0 ? 1 : 0); +})().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From b6521196cd48172134a1ed8f56abac449e21c079 Mon Sep 17 00:00:00 2001 From: dborup Date: Wed, 30 Sep 2026 14:00:23 +0000 Subject: [PATCH 02/23] fix(channels): keep client-only state across a channel-list refresh Relates to #152. loadChannels() replaced the channel list with the server snapshot and carried nothing across, so a region change, the show-encrypted toggle or a shared-channel approval closed an open PSK conversation, dropped My Channels and reset unread badges and user:* previews. loadChannels() now runs: fresh snapshot (copied, so the api() cache is not mutated) -> mergeApprovedChannels -> mergeUserChannels -> mergeClientChannelState -> sort -> render -> reconcile. The new pure helper carries unread (explicit 0 included), userAdded and userLabel by hash for server rows and user:* rows alike, and never re-creates a row the fresh list lacks, so the region filter keeps working. A label just re-read from storage wins over the old one. Live activity (lastActivityMs, lastSender, lastMessage, messageCount, moved together) is carried only when the WS path stamped the row with a local sequence number newer than the one loadChannels() sampled when its request started. No browser/server time comparison, so clock skew cannot decide it. user:* rows exist only in the tab and always keep theirs. onApproved now just calls loadChannels(): its trailing mergeUserChannels()/render ran after the reconcile had already closed the PSK conversation, and is redundant now. init() keeps its own mergeUserChannels()/render: a no-op after a successful load, but the only thing that lists My Channels when /channels fails (covered by a test). Test follow-ups: wait for the decrypt pass before the E2E pane snapshot, a cross-realm array assertion, and tests for init()'s failure path and for a stored label winning over the previous list's. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DxHDRtHRCQxS6AzzDXXSJi --- public/channels.js | 96 +++++++++++++++++++++++---- test-channels-client-state-152-e2e.js | 8 ++- test-channels-client-state-152.js | 30 ++++++++- 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/public/channels.js b/public/channels.js index 10b9e2411..e6c8e6b93 100644 --- a/public/channels.js +++ b/public/channels.js @@ -171,6 +171,63 @@ // can mutate freely without leaking changes back to the input. return survivors.length ? mergedRest.concat(survivors) : mergedRest; } + + // #152: loadChannels() replaces `channels` with the server snapshot, which + // knows nothing about state that only lives in this tab: unread counts, + // the user's PSK marks and labels, the preview of user:* rows, and live + // activity the WS path applied while the request was in flight. + // mergeWsAppendedIntoRest() above does this job for `messages` (#1498); + // this is its counterpart for `channels`. + // + // Which activity is newer is decided by a local sequence, never by + // comparing times: the WS path stamps a row with ++wsActivitySeq, and + // loadChannels() samples wsActivitySeq when its request starts. A + // browser Date.now() and the server's lastActivity come from different + // clocks, so comparing them depends on clock skew. + var wsActivitySeq = 0; + + // user:* rows exist only in this browser (mergeUserChannels() creates them + // from stored keys with a placeholder preview), so their activity always + // comes from the previous list. + function isClientOnlyChannel(ch) { + return typeof ch.hash === 'string' && ch.hash.indexOf('user:') === 0; + } + + // Carries client-only state from prevChannels onto freshChannels by hash. + // Only enriches rows freshChannels already contains: carrying a missing + // row over would resurrect channels the region filter just excluded. + // Returns a fresh array of fresh objects; never aliases or mutates input. + function mergeClientChannelState(freshChannels, prevChannels, seqAtRequestStart) { + if (!Array.isArray(freshChannels)) return []; + var prevByHash = new Map(); + if (Array.isArray(prevChannels)) { + for (var i = 0; i < prevChannels.length; i++) { + var p = prevChannels[i]; + if (p && p.hash != null) prevByHash.set(p.hash, p); + } + } + return freshChannels.map(function (c) { + var out = Object.assign({}, c); + var prev = out.hash != null ? prevByHash.get(out.hash) : undefined; + if (!prev) return out; + // Carried when present, including an explicit 0: undefined is not "read". + if (Object.prototype.hasOwnProperty.call(prev, 'unread')) out.unread = prev.unread; + if (prev.userAdded === true) out.userAdded = true; + // A label mergeUserChannels() just read from storage wins over the old one. + if (prev.userLabel && !out.userLabel) out.userLabel = prev.userLabel; + var newerLive = typeof prev._wsSeq === 'number' && prev._wsSeq > seqAtRequestStart; + if (newerLive || isClientOnlyChannel(out)) { + // Moved together: a sender without its message reads as another message. + out.lastActivityMs = prev.lastActivityMs; + out.lastSender = prev.lastSender; + out.lastMessage = prev.lastMessage; + out.messageCount = prev.messageCount; + if (typeof prev._wsSeq === 'number') out._wsSeq = prev._wsSeq; + } + return out; + }); + } + let autoScroll = true; let nodeCache = {}; let selectedNode = null; @@ -1046,10 +1103,9 @@ view: _initUrlParams.get('view'), onApproved: function () { invalidateApiCache('/channels'); - loadChannels(true).then(function () { - mergeUserChannels(); - renderChannelList(); - }); + // loadChannels() merges the user's PSK rows itself (#152), before + // it reconciles the selection. + loadChannels(true); } }); } @@ -1311,11 +1367,11 @@ loadObserverRegions(); loadChannels().then(async function () { // Also load user-added encrypted channels into the sidebar. - // mergeUserChannels() mutates `channels` (marks userAdded, appends - // PSK-only entries) AFTER loadChannels() already rendered — so we - // MUST re-render here, otherwise the My Channels section never - // appears on first load when the route has no specific channel - // hash (regression caught by test-channel-issue-1111-e2e.js, case 2). + // Since #152 a successful loadChannels() has already merged and + // rendered them, so this is an idempotent re-render there. It stays + // for the failure path: when /channels fails, loadChannels() merges + // nothing, and this is what still lists My Channels (the section + // test-channel-issue-1111-e2e.js case 2 requires on first load). mergeUserChannels(); renderChannelList(); if (routeParam) await selectChannel(routeParam); @@ -1636,6 +1692,7 @@ ch.lastActivityMs = Date.now(); ch.lastSender = sender; ch.lastMessage = truncate(displayText, 100); + ch._wsSeq = ++wsActivitySeq; // #152: see mergeClientChannelState() channelListDirty = true; } else if (isFirstObservation) { // New channel we haven't seen @@ -1646,6 +1703,7 @@ lastActivityMs: Date.now(), lastSender: sender, lastMessage: truncate(displayText, 100), + _wsSeq: ++wsActivitySeq, }); channelListDirty = true; } @@ -1854,6 +1912,8 @@ } async function loadChannels(silent) { + // #152: WS activity stamped after this point is newer than the snapshot. + const seqAtRequestStart = wsActivitySeq; try { const rp = RegionFilter.getRegionParam(); var showEnc = localStorage.getItem('channels-show-encrypted') === 'true'; @@ -1862,15 +1922,24 @@ if (showEnc) params.push('includeEncrypted=true'); const qs = params.length ? '?' + params.join('&') : ''; const data = await api('/channels' + qs, { ttl: CLIENT_TTL.channels }); - channels = (data.channels || []).map(ch => { - ch.lastActivityMs = ch.lastActivity ? new Date(ch.lastActivity).getTime() : 0; - return ch; - }); + const prevChannels = channels; + // Copies: api() hands the same cached objects back on a TTL hit, and + // mergeUserChannels() below mutates rows. + channels = (data.channels || []).map(ch => Object.assign({}, ch, { + lastActivityMs: ch.lastActivity ? new Date(ch.lastActivity).getTime() : 0 + })); // Approved shared channels are listed for everyone, even before they // carry traffic. if (window.ChannelProposals) { channels = window.ChannelProposals.mergeApprovedChannels(channels, data.approvedChannels); } + // #152: re-derive the user's PSK rows from storage, then carry the + // tab's own state over from the previous list. mergeUserChannels() + // must run first so its user:* rows get their unread and preview back, + // and both must run before reconcileSelectionAfterChannelRefresh(), + // which closes the conversation when selectedHash is not listed. + if (typeof ChannelDecrypt !== 'undefined' && ChannelDecrypt) mergeUserChannels(); + channels = mergeClientChannelState(channels, prevChannels, seqAtRequestStart); channels.sort((a, b) => (b.lastActivityMs || 0) - (a.lastActivityMs || 0)); renderChannelList(); reconcileSelectionAfterChannelRefresh(); @@ -2595,6 +2664,7 @@ window._channelsSelectChannelForTest = selectChannel; window._channelsRefreshMessagesForTest = refreshMessages; window._channelsMergeWsAppendedIntoRestForTest = mergeWsAppendedIntoRest; + window._channelsMergeClientChannelStateForTest = mergeClientChannelState; window._channelsNormalizeObservedPathHashSizesForTest = normalizeObservedPathHashSizes; window._channelsUnionObservedPathHashSizesForTest = unionObservedPathHashSizes; window._channelsRenderObservedPathHashBadgeForTest = renderObservedPathHashBadge; diff --git a/test-channels-client-state-152-e2e.js b/test-channels-client-state-152-e2e.js index ae0ede296..6f968d835 100644 --- a/test-channels-client-state-152-e2e.js +++ b/test-channels-client-state-152-e2e.js @@ -85,6 +85,12 @@ async function runViewport(browser, vp) { const h = document.querySelector('#chHeader .ch-header-text'); return h && h.textContent.indexOf(label) !== -1; }, PSK_LABEL, { timeout: 10000 }); + // Let the decrypt pass finish so the pane snapshot below is final. + await page.waitForFunction(() => { + const m = document.getElementById('chMessages'); + return m && !m.querySelector('.ch-loading') && m.textContent.indexOf('Decrypting') === -1; + }, null, { timeout: 15000 }); + await page.waitForTimeout(500); const s = await conversationState(page); assert(s.rowPresent && s.rowSelected, 'PSK row must be listed and selected, got ' + JSON.stringify(s)); if (vp.mobile === false) assert(s.rowInMyChannels, 'PSK row must be in My Channels'); @@ -99,7 +105,7 @@ async function runViewport(browser, vp) { assert(s.urlHash === before.urlHash, 'URL must stay on the PSK channel, got ' + s.urlHash); assert(s.header.indexOf(PSK_LABEL) !== -1, 'header must still name the PSK channel, got ' + JSON.stringify(s.header)); assert(s.messagesText.indexOf('Choose a channel') === -1, 'conversation must not close, got ' + JSON.stringify(s.messagesText.slice(0, 120))); - assert(s.messagesText === before.messagesText, 'message pane must be unchanged'); + assert(s.messagesText === before.messagesText, 'message pane must be unchanged: ' + JSON.stringify([before.messagesText.slice(0,120), s.messagesText.slice(0,120)])); assert(s.rowPresent && s.rowSelected, 'PSK row must stay listed and selected, got ' + JSON.stringify(s)); if (vp.mobile === false) { assert(s.rowInMyChannels, 'My Channels must still contain the PSK row'); diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 7712808b8..fc7fc63be 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -344,6 +344,13 @@ async function test(name, fn) { assert.ok(h.elements.chList.innerHTML.indexOf('Team') !== -1, 'PSK label rendered'); }); + await test('a label re-read from storage wins over the previous list\'s label', async () => { + const { h } = await openPskConversation(); + h.setState({ channels: [Object.assign({}, h.row('public')), pskRow({ userLabel: 'Old name' })] }); + await h.w._channelsLoadChannelsForTest(true); + assert.strictEqual(h.row(PSK_HASH).userLabel, 'Team', 'storage label must win (got ' + h.row(PSK_HASH).userLabel + ')'); + }); + await test('unread and preview on a user:* row survive a refresh (explicit 0 on a server row kept)', async () => { const { h } = await openPskConversation(); h.setState({ @@ -486,12 +493,33 @@ async function test(name, fn) { assert.strictEqual(newer[0].messageCount, 99, 'moved together with its message'); assert.strictEqual(newer[0].lastSender, 'X'); assert.strictEqual(newer[0].lastActivityMs, 999999); - assert.deepStrictEqual(merge(null, prev, 0), []); + const none = merge(null, prev, 0); + assert.ok(Array.isArray(none) && none.length === 0, 'non-array fresh → []'); const noPrev = merge(fresh, null, 0); assert.deepStrictEqual(JSON.parse(JSON.stringify(noPrev)), JSON.parse(freshCopy)); assert.notStrictEqual(noPrev[0], fresh[0]); }); + // init() keeps its own mergeUserChannels()/render after loadChannels(): + // a no-op after a successful load, but the only thing that lists My + // Channels when /channels fails. + await test('init(): My Channels still listed when /channels fails', async () => { + const h = makeHarness(); + h.storeKey(PSK_NAME, PSK_KEY, 'Team'); + h.respondChannels = () => Promise.reject(new Error('offline')); + await h.init(); + assert.ok(h.row(PSK_HASH) && h.row(PSK_HASH).userAdded === true, 'PSK row listed'); + assert.ok(/ch-section-mychannels/.test(h.elements.chList.innerHTML), 'My Channels rendered'); + }); + + await test('init(): a successful load lists each PSK row exactly once', async () => { + const h = makeHarness(); + h.storeKey(PSK_NAME, PSK_KEY, 'Team'); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public')] }); + await h.init(); + assert.strictEqual(h.state().channels.filter((c) => c.hash === PSK_HASH).length, 1); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From 78bc3a7edcdc4ef5a00b9bbd17db0e47038b9804 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 03:17:14 +0000 Subject: [PATCH 03/23] =?UTF-8?q?fix(channels):=20F1=20=E2=80=94=20restart?= =?UTF-8?q?=20decryption=20on=20region=20switch=20mid-decrypt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A region change while a PSK (user:* or key-matched) channel's decrypt fetch is still in flight left the message pane stuck on "Decrypting messages…": the in-flight decrypt's own staleness check discards its result once the region changes under it, and the region handler's refreshMessages({regionSwitch:true}) has no REST messages to refetch for an encrypted row, so it just returned without ever restarting the decrypt. The same gap could also leave a finished decrypt showing the previous region's messages. regionChangeHandler now re-runs selectChannel(selectedHash) for an encrypted selection instead of refreshMessages() — it redoes the key lookup and decrypt fetch for the new region without closing the conversation (selectedHash/URL stay put). Test: new unit test drives selectChannel() with a real AES-128-ECB + HMAC-SHA256 encrypted packet (Node's crypto, matching the scheme documented in channel-decrypt.js) and an in-flight /packets fetch, switches region mid-flight, and asserts the pane ends up with the new-region message, not stuck on "Decrypting…" and not showing the old region's message. Confirmed red by reverting this fix (pane stays stuck on "Decrypting…"). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- public/channels.js | 14 ++++ test-channels-client-state-152.js | 103 +++++++++++++++++++++++++++++- 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/public/channels.js b/public/channels.js index e6c8e6b93..7aee75c44 100644 --- a/public/channels.js +++ b/public/channels.js @@ -1071,6 +1071,20 @@ regionChangeHandler = RegionFilter.onChange(function () { loadChannels(true).then(async function () { if (!selectedHash) return; + var selCh = channels.find(function (c) { return c.hash === selectedHash; }); + if (selCh && selCh.encrypted) { + // F1 (#152 follow-up): an encrypted (user:* or key-matched) row has + // no REST messages for refreshMessages() to refetch — it used to + // just return, which left an in-flight decrypt stuck on + // "Decrypting messages…" once its own staleness check discarded + // the result (the region changed under it), and left a finished + // decrypt showing the previous region's messages. Re-run the + // selection instead: it redoes the key lookup and decrypt fetch + // for the new region without closing the conversation + // (selectedHash/URL stay put). + await selectChannel(selectedHash); + return; + } await refreshMessages({ regionSwitch: true, forceNoCache: true }); }); }); diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index fc7fc63be..1551ae44c 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -24,7 +24,36 @@ const vm = require('vm'); const fs = require('fs'); const path = require('path'); const assert = require('assert'); -const { webcrypto } = require('crypto'); +const nodeCrypto = require('crypto'); +const { webcrypto } = nodeCrypto; + +// Encrypts a channel message exactly the way internal/channel/channel.go / +// public/channel-decrypt.js expect it (see that file's header comment): +// AES-128-ECB(timestamp(4 LE) + flags(1) + "sender: message" + 0x00, padded +// to a block boundary) plus an HMAC-SHA256(key + 16 zero bytes) MAC +// truncated to 2 bytes. Node's built-in `crypto` implements plain AES-128 +// and HMAC-SHA256, so this round-trips through the real (decrypt-only) +// public/vendor/aes-ecb.js the way a real radio-encrypted packet would. +function encryptChannelMessage(keyHex, sender, text) { + const keyBytes = Buffer.from(keyHex, 'hex'); + const body = Buffer.from(sender + ': ' + text, 'utf8'); + const header = Buffer.alloc(5); + header.writeUInt32LE(Math.floor(Date.now() / 1000) >>> 0, 0); + let plaintext = Buffer.concat([header, body, Buffer.from([0])]); + const pad = (16 - (plaintext.length % 16)) % 16; + if (pad) plaintext = Buffer.concat([plaintext, Buffer.alloc(pad)]); + const cipher = nodeCrypto.createCipheriv('aes-128-ecb', keyBytes, null); + cipher.setAutoPadding(false); + const ciphertext = Buffer.concat([cipher.update(plaintext), cipher.final()]); + const secret = Buffer.concat([keyBytes, Buffer.alloc(16)]); + const mac = nodeCrypto.createHmac('sha256', secret).update(ciphertext).digest().slice(0, 2).toString('hex'); + const channelHash = nodeCrypto.createHash('sha256').update(keyBytes).digest()[0]; + return { encryptedData: ciphertext.toString('hex'), mac, channelHash }; +} + +function encryptedPacket(hash, firstSeen, enc) { + return { hash, first_seen: firstSeen, decoded_json: { type: 'GRP_TXT', encryptedData: enc.encryptedData, mac: enc.mac, channelHash: enc.channelHash } }; +} const RealDate = Date; const HOUR = 60 * 60 * 1000; @@ -95,6 +124,10 @@ function makeHarness(opts) { // flight. respondChannels: () => Promise.resolve({ channels: [] }), channelRequests: [], + // Each /packets request (the client-side decrypt fetch) is answered by + // h.respondPackets(path); tests replace it to control decrypt timing. + respondPackets: () => Promise.resolve({ packets: [] }), + packetsRequests: [], }; const ctx = { @@ -159,6 +192,10 @@ function makeHarness(opts) { h.channelRequests.push(p); return h.respondChannels(p); } + if (p.indexOf('/packets') === 0) { + h.packetsRequests.push(p); + return h.respondPackets(p); + } if (p.indexOf('/observers') === 0) return Promise.resolve({ observers: [] }); return Promise.resolve({ messages: [], packets: [], channels: [] }); }; @@ -294,13 +331,73 @@ async function test(name, fn) { }); await test('open PSK conversation survives a region change (RegionFilter.onChange handler)', async () => { - const { h, messages } = await openPskConversation(); + const { h } = await openPskConversation(); h.regionParam = 'CPH'; assert.strictEqual(typeof h.regionChange, 'function', 'init() must register a region handler'); h.regionChange(); await flush(); assert.ok(h.channelRequests[h.channelRequests.length - 1].indexOf('region=CPH') !== -1, 'region change must refetch with the region'); - assertConversationOpen(h, messages, 'region change'); + // F1 (#152 follow-up): an encrypted/PSK selection now re-runs + // selectChannel() for the new region (see below) instead of just + // refreshMessages() — which has no REST messages to refetch for it — so + // unlike a plain server channel its messages are freshly re-decrypted + // for the new region (empty here, since the harness's default /packets + // response has none), not carried over verbatim. The conversation + // itself — selection, URL, My Channels membership — still survives. + const s = h.state(); + assert.strictEqual(s.selectedHash, PSK_HASH, 'region change: PSK selection must survive'); + assert.ok(!h.historyCalls.includes('#/channels'), 'region change: URL must not be rewritten to #/channels (got ' + JSON.stringify(h.historyCalls) + ')'); + const row = h.row(PSK_HASH); + assert.ok(row && row.userAdded === true, 'region change: PSK row must stay in My Channels'); + assert.ok(/ch-section-mychannels/.test(h.elements.chList.innerHTML), 'region change: rendered list must contain My Channels'); + }); + + // ── F1: a region switch mid-decrypt must restart decryption, not strand + // the pane on "Decrypting messages…" or leave it showing the previous + // region's messages. refreshMessages() has nothing to refetch for an + // encrypted row (no REST messages exist), so it must hand off to + // selectChannel() instead of silently returning. + await test('F1: region switch mid-decrypt restarts decryption for the new region', async () => { + const h = makeHarness(); + const KEY = 'f1f1f1f1f1f1f1f1f1f1f1f1f1f1f1f1'; + const NAME = 'psk:f1test'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'F1 Team'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + + const oldMsg = encryptChannelMessage(KEY, 'Alice', 'old region message'); + const newMsg = encryptChannelMessage(KEY, 'Bob', 'new region message'); + + const pending = deferred(); + h.respondPackets = () => pending.promise; + const select = h.w._channelsSelectChannelForTest(HASH); + // selectChannel() awaits ChannelDecrypt.computeChannelHash() (a real + // crypto.subtle.digest call) before it even reaches the decrypt-and-set + // "Decrypting…" step, which takes more ticks than a plain microtask + // flush — give it a generous margin. + await flush(300); + assert.ok(/Decrypting/.test(h.elements.chMessages.innerHTML), 'pane must show "Decrypting…" while the fetch is in flight'); + + // Region changes while the first decrypt's /packets fetch is still + // pending. The region handler's loadChannels() + its own /packets + // fetch (for the new region) resolve before the stale one does. + h.regionParam = 'CPH'; + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-new', '2026-01-01T00:01:00Z', newMsg)] }); + h.regionChange(); + await flush(300); + + // The original (stale) fetch finally resolves, for the OLD region. + pending.resolve({ packets: [encryptedPacket('pkt-old', '2026-01-01T00:00:00Z', oldMsg)] }); + await select; + await flush(300); + + const msgEl = h.elements.chMessages; + assert.ok(!/Decrypting/.test(msgEl.innerHTML), 'pane must not be stuck on "Decrypting…" (got ' + msgEl.innerHTML.slice(0, 200) + ')'); + const texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('new region message') !== -1, 'new-region message must be shown (got ' + JSON.stringify(texts) + ')'); + assert.ok(texts.indexOf('old region message') === -1, 'stale old-region message must not be shown (got ' + JSON.stringify(texts) + ')'); + assert.strictEqual(h.state().selectedHash, HASH, 'conversation must stay open on the same PSK'); }); await test('open PSK conversation survives the show-encrypted toggle', async () => { From 8a3b18206d201e25f03dacb43eaaaf0b26a1dfdd Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 03:18:49 +0000 Subject: [PATCH 04/23] =?UTF-8?q?fix(channels):=20F2=20=E2=80=94=20storage?= =?UTF-8?q?=20stays=20authoritative=20for=20userAdded/userLabel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergeClientChannelState() carried prev.userAdded/prev.userLabel forward from the in-memory previous channel list, and the remove-key handler's server-known-channel branch unmarked userAdded but never cleared userLabel. Removing a key from a server-known channel therefore left a stale userLabel on the in-memory row; the next refresh's merge carried it back onto the fresh row (mergeUserChannels() only (re)derives it for keys still in storage, so a removed key's label was never otherwise overwritten), making the removed label resurface indefinitely. mergeUserChannels() already re-derives both userAdded and userLabel from storage on every load, before mergeClientChannelState() runs — so the merge never needed to carry them from prev at all. Drop that carry-over and make storage the single source of truth; also clear userLabel in the remove handler itself so the UI doesn't show it for one extra render. Extracted the remove-button logic into removeUserChannelKey() so it's directly testable. Tests: a new regression test adds a key+label to a server-known channel, removes it through the real handler, and asserts the label and My Channels membership don't resurrect across two loadChannels() refreshes. Updated the existing mergeClientChannelState pure-helper test, whose assertions encoded the old (buggy) carry-over contract. Confirmed red against two separate mutants: reverting the merge-helper change alone (caught by the pure-helper test) and reverting the remove-handler's label clear alone (caught by the new regression test). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- public/channels.js | 87 +++++++++++++++++++------------ test-channels-client-state-152.js | 41 +++++++++++++-- 2 files changed, 92 insertions(+), 36 deletions(-) diff --git a/public/channels.js b/public/channels.js index 7aee75c44..b5154a3ac 100644 --- a/public/channels.js +++ b/public/channels.js @@ -212,9 +212,11 @@ if (!prev) return out; // Carried when present, including an explicit 0: undefined is not "read". if (Object.prototype.hasOwnProperty.call(prev, 'unread')) out.unread = prev.unread; - if (prev.userAdded === true) out.userAdded = true; - // A label mergeUserChannels() just read from storage wins over the old one. - if (prev.userLabel && !out.userLabel) out.userLabel = prev.userLabel; + // userAdded/userLabel are NOT carried from prev (F2, #152 follow-up): + // mergeUserChannels() runs before this, on the same fresh list, and + // re-derives both from storage on every load, so storage stays the + // single source of truth. Carrying a stale prev value let a removed + // key's label (and a stale userAdded) resurrect across a refresh. var newerLive = typeof prev._wsSeq === 'number' && prev._wsSeq > seqAtRequestStart; if (newerLive || isClientOnlyChannel(out)) { // Moved together: a sender without its message reads as another message. @@ -728,6 +730,49 @@ } } + // Removes a user-stored channel key (and its label) and updates the + // channel list/selection to match. Extracted from the remove-button click + // handler so it's directly testable; the handler still owns the confirm() + // gate. + function removeUserChannelKey(channelHash) { + if (!channelHash) return; + var ch = channels.find(function (c) { return c.hash === channelHash; }); + var chName = channelHash.startsWith('user:') + ? channelHash.substring(5) + : (ch && ch.name) || channelHash; + ChannelDecrypt.removeKey(chName); + if (channelHash.startsWith('user:')) { + // Pure user-added channel — drop from the list entirely. + channels = channels.filter(function (c) { return c.hash !== channelHash; }); + if (selectedHash === channelHash) { + selectedHash = null; + messages = []; + history.replaceState(null, '', '#/channels'); + var msgEl2 = document.getElementById('chMessages'); + if (msgEl2) msgEl2.innerHTML = '
Choose a channel from the sidebar to view messages
'; + var header2 = document.getElementById('chHeader'); + if (header2) header2.querySelector('.ch-header-text').textContent = 'Select a channel'; + } + } else if (ch) { + // Server-known channel: keep the row, just unmark as user-added so + // the close button disappears until they re-add a key. // EMOJI-OK: prior glyph reference + ch.userAdded = false; + // F2 (#152 follow-up): also clear the label now. mergeClientChannelState() + // no longer carries userLabel from the previous list either way, but + // clearing it here means the UI stops showing it immediately instead + // of waiting for the next channel-list refresh. + delete ch.userLabel; + // If this was the selected channel, clear decrypted messages since + // the key is gone — they can't be re-decrypted without re-adding it. + if (selectedHash === channelHash) { + messages = []; + var msgEl2 = document.getElementById('chMessages'); + if (msgEl2) msgEl2.innerHTML = '
Key removed — add a key to decrypt messages
'; + } + } + renderChannelList(); + } + // Fetch and decrypt GRP_TXT packets client-side (M5: delta fetch + cache) async function fetchAndDecryptChannel(keyHex, channelHashByte, channelName, opts) { opts = opts || {}; @@ -1498,37 +1543,12 @@ // The localStorage key is the channel name. For user:-prefixed entries // strip the prefix; for server-known channels look up the channel // object so we use its display name (the hash itself isn't the key). - var ch = channels.find(function (c) { return c.hash === channelHash; }); - var chName = channelHash.startsWith('user:') + var confirmCh = channels.find(function (c) { return c.hash === channelHash; }); + var confirmName = channelHash.startsWith('user:') ? channelHash.substring(5) - : (ch && ch.name) || channelHash; - if (!confirm('Remove channel "' + chName + '"?\n\nThis will permanently remove the key from this browser and clear cached messages. You will need to re-enter the key to decrypt this channel again.')) return; - ChannelDecrypt.removeKey(chName); - if (channelHash.startsWith('user:')) { - // Pure user-added channel — drop from the list entirely. - channels = channels.filter(function (c) { return c.hash !== channelHash; }); - if (selectedHash === channelHash) { - selectedHash = null; - messages = []; - history.replaceState(null, '', '#/channels'); - var msgEl2 = document.getElementById('chMessages'); - if (msgEl2) msgEl2.innerHTML = '
Choose a channel from the sidebar to view messages
'; - var header2 = document.getElementById('chHeader'); - if (header2) header2.querySelector('.ch-header-text').textContent = 'Select a channel'; - } - } else if (ch) { - // Server-known channel: keep the row, just unmark as user-added so - // the close button disappears until they re-add a key. // EMOJI-OK: prior glyph reference - ch.userAdded = false; - // If this was the selected channel, clear decrypted messages since - // the key is gone — they can't be re-decrypted without re-adding it. - if (selectedHash === channelHash) { - messages = []; - var msgEl2 = document.getElementById('chMessages'); - if (msgEl2) msgEl2.innerHTML = '
Key removed — add a key to decrypt messages
'; - } - } - renderChannelList(); + : (confirmCh && confirmCh.name) || channelHash; + if (!confirm('Remove channel "' + confirmName + '"?\n\nThis will permanently remove the key from this browser and clear cached messages. You will need to re-enter the key to decrypt this channel again.')) return; + removeUserChannelKey(channelHash); return; } // Color clear button — remove color without opening picker (#681) @@ -2690,6 +2710,7 @@ window._channelsBeginMessageRequestForTest = beginMessageRequest; window._channelsIsStaleMessageRequestForTest = isStaleMessageRequest; window._channelsReconcileSelectionForTest = reconcileSelectionAfterChannelRefresh; + window._channelsRemoveKeyHandlerForTest = removeUserChannelKey; window._channelsGetStateForTest = function () { return { channels: channels, messages: messages, selectedHash: selectedHash }; }; diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 1551ae44c..e47e4eff4 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -578,12 +578,17 @@ async function test(name, fn) { out.forEach((o, i) => { assert.notStrictEqual(o, fresh[i], 'rows must be copies'); assert.notStrictEqual(o, prev[i]); }); assert.deepStrictEqual(out.map((o) => o.hash), ['public', PSK_HASH], 'no resurrected rows, fresh order kept'); assert.strictEqual(out[0].unread, 0, 'explicit 0 carried'); - assert.strictEqual(out[0].userAdded, true); - assert.strictEqual(out[0].userLabel, 'Pub'); + // F2 (#152 follow-up): userAdded/userLabel are NOT carried from prev — + // mergeUserChannels() (run before this, on the same fresh list) is the + // sole source now, so storage stays authoritative. A stale prev value + // (here simulating an in-memory row a remove-handler forgot to clear) + // must never leak onto the merged row. + assert.strictEqual(out[0].userAdded, undefined, 'userAdded must not be carried from prev'); + assert.strictEqual(out[0].userLabel, undefined, 'userLabel must not be carried from prev'); assert.strictEqual(out[0].lastMessage, 'fresh', 'stamp not newer than request start → snapshot activity wins'); assert.strictEqual(out[0].messageCount, 5); assert.strictEqual(out[1].unread, 2); - assert.strictEqual(out[1].userLabel, 'Team'); + assert.strictEqual(out[1].userLabel, '', 'fresh already set this (by mergeUserChannels in the real pipeline); merge must not override it from prev'); assert.strictEqual(out[1].lastMessage, 'preview', 'client-only row keeps its preview'); const newer = merge(fresh, prev, 2); assert.strictEqual(newer[0].lastMessage, 'stale', 'stamp newer than request start → live activity wins'); @@ -597,6 +602,36 @@ async function test(name, fn) { assert.notStrictEqual(noPrev[0], fresh[0]); }); + // ── F2: storage must be authoritative for userAdded/userLabel. A removed + // key's label must not resurrect across refreshes (regression for the + // remove-handler leaving ch.userLabel set, and mergeClientChannelState + // carrying it forward from the in-memory previous list). + await test('F2: a removed key\'s label and My Channels row do not resurrect across two refreshes', async () => { + const h = makeHarness(); + h.storeKey('#ops', 'aa'.repeat(16), 'Ops room'); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('#ops', { name: '#ops' })] }); + await h.init(); + const ops = h.row('#ops'); + assert.ok(ops && ops.userAdded === true && ops.userLabel === 'Ops room', 'precondition: key+label present'); + + // Remove the key through the same handler the UI uses (server-known + // channel branch: unmark userAdded, keep the row). removeUserChannelKey + // itself calls ChannelDecrypt.removeKey(), so don't double-remove. + h.w._channelsRemoveKeyHandlerForTest('#ops'); + + const afterRemove = h.row('#ops'); + assert.strictEqual(afterRemove.userAdded, false, 'userAdded cleared immediately by the remove handler'); + assert.ok(!afterRemove.userLabel, 'userLabel must be cleared immediately by the remove handler (got ' + JSON.stringify(afterRemove.userLabel) + ')'); + + for (let i = 0; i < 2; i++) { + await h.w._channelsLoadChannelsForTest(true); + const row = h.row('#ops'); + assert.ok(!row.userLabel, 'refresh ' + (i + 1) + ': label must not resurrect (got ' + JSON.stringify(row.userLabel) + ')'); + assert.notStrictEqual(row.userAdded, true, 'refresh ' + (i + 1) + ': My Channels membership must not resurrect'); + } + assert.ok(!/ch-section-mychannels/.test(h.elements.chList.innerHTML) || !/Ops room/.test(h.elements.chList.innerHTML), 'My Channels must not show the removed label'); + }); + // init() keeps its own mergeUserChannels()/render after loadChannels(): // a no-op after a successful load, but the only thing that lists My // Channels when /channels fails. From 28419a4828c4ae1e16cf60da7b1bf51907b11980 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 03:19:37 +0000 Subject: [PATCH 05/23] =?UTF-8?q?fix(channels):=20F3=20=E2=80=94=20remap?= =?UTF-8?q?=20a=20name-colliding=20PSK=20instead=20of=20closing=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mergeUserChannels() matches a stored PSK key against a server-known or newly-approved shared channel by name (not just by hash), annotating the existing row with userAdded instead of creating a separate user:* row. If that PSK's conversation was already open under its user:* hash, the hash simply disappears from the fresh channel list on the next refresh (a region change, or a shared-channel approval), even though the conversation is still live — reconcileSelectionAfterChannelRefresh() then treats the missing hash as "channel gone" and closes it: clears the selection, empties messages, and rewrites the URL to #/channels. reconcileSelectionAfterChannelRefresh() now checks, before closing a missing user:* selection, whether a userAdded row with the same PSK name exists in the fresh list; if so it remaps selectedHash to that row and updates the URL via replaceState, without touching messages. Tests: two new regression tests cover both triggers — a region switch that reveals a same-named server channel, and approving a shared channel with the same name as the open PSK — each asserting the selection remaps (not closes), the URL updates, and messages are left alone. Confirmed red by reverting the reconcile change (both tests fail: selection closes to null instead of remapping). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- public/channels.js | 16 +++++++++ test-channels-client-state-152.js | 59 +++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/public/channels.js b/public/channels.js index b5154a3ac..89dc2be8e 100644 --- a/public/channels.js +++ b/public/channels.js @@ -307,6 +307,22 @@ function reconcileSelectionAfterChannelRefresh() { if (!selectedHash || channels.some(ch => ch.hash === selectedHash)) return false; + // F3 (#152 follow-up): a PSK whose name collides with a server-known or + // newly-approved shared channel is matched by name in mergeUserChannels() + // instead of getting its own user:* row, so the user:* hash this + // conversation opened under disappears from the fresh list even though + // the conversation is still live. Remap to the row mergeUserChannels() + // annotated instead of closing the conversation. + if (selectedHash.indexOf('user:') === 0) { + var pskName = selectedHash.substring(5); + var remapped = channels.find(function (ch) { return ch.userAdded === true && ch.name === pskName; }); + if (remapped) { + selectedHash = remapped.hash; + history.replaceState(null, '', `#/channels/${encodeURIComponent(selectedHash)}`); + renderChannelList(); + return false; + } + } selectedHash = null; messages = []; history.replaceState(null, '', '#/channels'); diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index e47e4eff4..715f9ba98 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -632,6 +632,65 @@ async function test(name, fn) { assert.ok(!/ch-section-mychannels/.test(h.elements.chList.innerHTML) || !/Ops room/.test(h.elements.chList.innerHTML), 'My Channels must not show the removed label'); }); + // ── F3: a PSK whose name collides with a server-known or newly-approved + // shared channel is matched by name in mergeUserChannels() instead of + // getting its own user:* row, so the user:* hash the conversation opened + // under disappears from the fresh list even though the conversation is + // still live. reconcileSelectionAfterChannelRefresh() must remap to the + // row mergeUserChannels() annotated instead of closing the conversation. + await test('F3 (region variant): a PSK is remapped, not closed, when a region switch reveals a same-named server channel', async () => { + const h = makeHarness(); + const NAME = '#chat'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, 'aa'.repeat(16), 'Chat'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + h.setState({ selectedHash: HASH, messages: [{ sender: 'Alice', text: 'hello', timestamp: '2026-01-01T00:00:00Z', packetHash: 'm1' }] }); + assert.ok(h.row(HASH), 'precondition: user:* row exists before the refresh'); + + // The new region's snapshot already knows about #chat as a real server + // channel — mergeUserChannels() will name-match it instead of creating + // a user:#chat row. Drive loadChannels() directly (what the region + // handler calls first) rather than the full region-change handler: a + // same-named match isn't `encrypted`, so the handler's own follow-up + // would fall through to refreshMessages(), which legitimately clears + // messages when the new region has none over REST — a separate, + // unrelated behavior this test isn't about. + h.respondChannels = () => Promise.resolve({ channels: [serverChannel(NAME, { name: NAME })] }); + await h.w._channelsLoadChannelsForTest(true); + + assert.strictEqual(h.row(HASH), undefined, 'the user:* hash must no longer exist (name-matched instead)'); + const remapped = h.row(NAME); + assert.ok(remapped && remapped.userAdded === true, 'the name-matched row must carry userAdded'); + assert.strictEqual(h.state().selectedHash, NAME, 'selection must remap to the matched row, not close (got ' + h.state().selectedHash + ')'); + assert.strictEqual(h.state().messages.length, 1, 'messages must not be cleared by the remap'); + assert.ok(!h.historyCalls.includes('#/channels'), 'must not route back to #/channels (got ' + JSON.stringify(h.historyCalls) + ')'); + assert.ok(h.historyCalls[h.historyCalls.length - 1].indexOf(encodeURIComponent(NAME)) !== -1, 'URL must be updated to the matched row (got ' + JSON.stringify(h.historyCalls) + ')'); + }); + + await test('F3 (approval variant): a PSK is remapped, not closed, when approving a shared channel with the same name', async () => { + const h = makeHarness(); + const NAME = '#chat'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, 'bb'.repeat(16), 'Chat'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + h.setState({ messages: [{ sender: 'Alice', text: 'hello', timestamp: '2026-01-01T00:00:00Z', packetHash: 'm1' }] }); + + h.respondChannels = () => Promise.resolve({ channels: [], approvedChannels: [{ hash: NAME, name: NAME }] }); + assert.strictEqual(typeof h.approvedCallback, 'function', 'init() must mount ChannelProposals with onApproved'); + h.approvedCallback(); + await flush(300); + + assert.strictEqual(h.row(HASH), undefined, 'the user:* hash must no longer exist (name-matched instead)'); + const remapped = h.row(NAME); + assert.ok(remapped && remapped.userAdded === true, 'the name-matched row must carry userAdded'); + assert.strictEqual(h.state().selectedHash, NAME, 'selection must remap on approval too (got ' + h.state().selectedHash + ')'); + assert.strictEqual(h.state().messages.length, 1, 'messages must not be cleared by the remap'); + }); + // init() keeps its own mergeUserChannels()/render after loadChannels(): // a no-op after a successful load, but the only thing that lists My // Channels when /channels fails. From 9d21416a870f3474bd01821de353982eed957e1d Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 03:20:04 +0000 Subject: [PATCH 06/23] test(channels): pin the new-WS-row _wsSeq stamp against a surviving mutant The mutant "drop _wsSeq from the object literal when processWSBatch() creates a brand-new channel row" survived the existing suite: every _wsSeq regression test exercised a row that already existed in channels (the ch._wsSeq = ++wsActivitySeq branch), never the push() branch for a channel seen for the first time over WS. Add a test: a live message for a channel with no existing row creates one via the WS path while a concurrent loadChannels() is in flight, then an older snapshot for that same channel resolves. Without the new row's _wsSeq stamp, mergeClientChannelState() has no way to tell the freshly live row is newer than the request, so the older snapshot would win. Confirmed red by removing the field from the push() literal. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- test-channels-client-state-152.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 715f9ba98..987ac2fe9 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -711,6 +711,34 @@ async function test(name, fn) { assert.strictEqual(h.state().channels.filter((c) => c.hash === PSK_HASH).length, 1); }); + // ── Nit: a brand-new WS-pushed channel row must be _wsSeq-stamped like an + // existing row is, or its freshly-live activity loses to a concurrent, + // older snapshot once mergeClientChannelState() can no longer tell it's + // newer than the request. + await test('a live message for a brand-new channel is _wsSeq-stamped and wins over a concurrent older snapshot', async () => { + const h = makeHarness(); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + const pending = deferred(); + h.respondChannels = () => pending.promise; + const load = h.w._channelsLoadChannelsForTest(true); + await flush(); + assert.strictEqual(h.row('brandnew'), undefined, 'precondition: channel does not exist yet'); + h.liveMessage('brandnew', 'Carol', 'hello'); + assert.ok(h.row('brandnew'), 'WS path must create the row immediately'); + pending.resolve({ channels: [serverChannel('brandnew', { + messageCount: 1, + lastActivity: new RealDate(RealDate.now() - 100000).toISOString(), + lastSender: 'Old', + lastMessage: 'stale snapshot', + })] }); + await load; + const row = h.row('brandnew'); + assert.strictEqual(row.lastMessage, 'hello', 'live message must win over an older concurrent snapshot (got ' + row.lastMessage + ')'); + assert.strictEqual(row.lastSender, 'Carol'); + assert.strictEqual(row.messageCount, 1); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From f61a13a68b607a254219b26f7773c26a434ca593 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 03:38:43 +0000 Subject: [PATCH 07/23] =?UTF-8?q?test(channels):=20F1=20E2E=20=E2=80=94=20?= =?UTF-8?q?seed=20a=20real=20client-decrypted=20packet=20for=20the=20regio?= =?UTF-8?q?n-switch=20race?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing #152 E2E (test-channels-client-state-152-e2e.js) uses a PSK key that matches no fixture packets, so it never exercises an in-flight client-side decrypt — it can't catch F1 (region switch while decryptAndRender()'s /api/packets fetch is still pending). Add a dedicated E2E test that seeds two genuinely client-decryptable GRP_TXT packets (real AES-128-ECB + HMAC-SHA256, matching the scheme in channel-decrypt.js) directly into a temp copy of the fixture DB, under a PSK key the server never sees — one observed only from an SJC observer, one only from SFO, so a region-scoped /api/packets?region= fetch returns exactly one. It delays the first /packets response ~3s, narrows the region filter mid-decrypt, and asserts the pane ends up showing the new region's message (not stuck on "Decrypting…", not the stale one), with the conversation still open. Runs at desktop and mobile. Since the server refuses to start against an unmigrated DB and writes must not go through the read-only server connection, the test briefly runs the real ingestor (against an in-process fake MQTT broker, no real traffic) to apply schema migrations, stops it, seeds the packets, then starts the read-only server — consistent with the read/write separation invariant. Confirmed red against the pre-F1 code (10 passed, 4 failed — pane stuck on "Decrypting…" and/or showing it instead of the new-region message) and green against the current (fixed) code (14 passed, 0 failed), both independently re-run. Registered in .github/workflows/deploy.yml next to test-channel-proposals-e2e.js (same own-server-and-ingestor pattern). Not added to test-all.sh, which only runs unit tests. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- .github/workflows/deploy.yml | 2 + test-channels-client-state-152-decrypt-e2e.js | 627 ++++++++++++++++++ 2 files changed, 629 insertions(+) create mode 100644 test-channels-client-state-152-decrypt-e2e.js diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index c29ba9470..48de6bbac 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -546,6 +546,8 @@ jobs: CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-2052-touch-target-e2e.js 2>&1 | tee -a e2e-output.txt # Starts its own ingestor + server on a temp copy of the fixture. CHROMIUM_REQUIRE=1 CORESCOPE_SERVER_BIN=./corescope-server CORESCOPE_INGESTOR_BIN=./corescope-ingestor FIXTURE_DB=test-fixtures/e2e-fixture.db node test-channel-proposals-e2e.js 2>&1 | tee -a e2e-output.txt + # Starts its own ingestor + server on a temp copy of the fixture; seeds client-decryptable packets itself. + CHROMIUM_REQUIRE=1 CORESCOPE_SERVER_BIN=./corescope-server CORESCOPE_INGESTOR_BIN=./corescope-ingestor FIXTURE_DB=test-fixtures/e2e-fixture.db node test-channels-client-state-152-decrypt-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1236-map-mobile-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1329-map-controls-accordion-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1273-qr-overlay-height-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/test-channels-client-state-152-decrypt-e2e.js b/test-channels-client-state-152-decrypt-e2e.js new file mode 100644 index 000000000..b62953f76 --- /dev/null +++ b/test-channels-client-state-152-decrypt-e2e.js @@ -0,0 +1,627 @@ +#!/usr/bin/env node +/** + * #152 (decrypt race) — E2E: a PSK channel's decrypt doesn't get stuck, or + * show stale-region messages, when the region filter changes while the + * client-side decrypt fetch is still in flight. + * + * Bug (F1): opening a PSK channel calls selectChannel() -> decryptAndRender() + * -> fetchAndDecryptChannel(), which fetches /api/packets?...®ion= and + * decrypts matching GRP_TXT packets client-side (public/channel-decrypt.js). + * If the region changes while that fetch is still in flight: + * 1. the in-flight fetch's own staleness check (isStaleMessageRequest(), + * which compares RegionFilter.getRegionParam() against the region the + * request captured) discards its own result once it resolves, leaving + * the pane stuck on "Decrypting messages..." forever, AND + * 2. refreshMessages()'s `if (selCh && selCh.encrypted) return;` early + * return means nothing re-fetches for the new region either, so even + * after the dust settles the pane never shows the new region's + * messages. + * + * This test seeds two genuinely-encrypted GRP_TXT packets directly into a + * temp copy of the E2E fixture DB (one observed only from an SJC observer, + * one only from an SFO observer) under a PSK key the server does NOT know + * (so the ingestor/server never auto-decrypts it — this must go through the + * client-side Web Crypto / pure-JS AES-ECB path in channel-decrypt.js). + * It then: opens that PSK channel with no region filter selected (decrypt + * fetch — covering both packets — delayed 3s via page.route), narrows the + * region filter to SFO only mid-decrypt, and asserts the pane ends up + * showing just the SFO message (not stuck "Decrypting...", not the stale + * SJC-region message) with the conversation still open on the same hash. + * + * Runs at desktop (1280x800) and mobile (390x844) viewports. + * + * Seeding: no sqlite3 CLI is available in this environment, so this file + * uses Node's built-in (experimental) `node:sqlite` module directly against + * a temp copy of the fixture DB, before the (read-only, mode=ro) server is + * started — consistent with AGENTS.md's read/write separation invariant. + * The raw fixture ships in a schema the ingestor migrates on startup (the + * server refuses to start against an unmigrated DB — "schema not migrated + * by ingestor"), so this file briefly runs the real ingestor binary first + * (against a minimal in-process fake MQTT broker, same trick as + * test-channel-proposals-e2e.js, with zero real MQTT traffic), stops it once + * migrations are applied, THEN seeds the encrypted packets, THEN starts the + * read-only server — all writes happen before the read-only server exists. + * + * Usage: + * cd cmd/ingestor && go build -o ../../corescope-ingestor . + * cd cmd/server && go build -o ../../corescope-server . + * node test-channels-client-state-152-decrypt-e2e.js + * + * Env: + * CORESCOPE_SERVER_BIN (default ./corescope-server) + * CORESCOPE_INGESTOR_BIN (default ./corescope-ingestor) + * FIXTURE_DB (default test-fixtures/e2e-fixture.db) + * PUBLIC_DIR (default public) + * CHROMIUM_PATH, CHROMIUM_REQUIRE=1 (fail instead of skip without Chromium) + */ +'use strict'; + +const assert = require('assert'); +const crypto = require('crypto'); +const { spawn } = require('child_process'); +const fs = require('fs'); +const net = require('net'); +const os = require('os'); +const path = require('path'); +const { DatabaseSync } = require('node:sqlite'); +const { chromium } = require('playwright'); + +const ROOT = __dirname; +const SERVER_BIN = path.resolve(process.env.CORESCOPE_SERVER_BIN || path.join(ROOT, 'corescope-server')); +const INGESTOR_BIN = path.resolve(process.env.CORESCOPE_INGESTOR_BIN || path.join(ROOT, 'corescope-ingestor')); +const FIXTURE_DB = path.resolve(process.env.FIXTURE_DB || path.join(ROOT, 'test-fixtures', 'e2e-fixture.db')); +const PUBLIC_DIR = path.resolve(process.env.PUBLIC_DIR || path.join(ROOT, 'public')); +const TIMEOUT = 20000; +// The firmware-default "Public" channel key — must NOT collide with our +// generated key, since the ingestor/server auto-decrypts that one +// server-side (see cmd/ingestor/main.go), which would bypass the +// client-side decrypt path this test exercises. +const DEFAULT_PUBLIC_KEY_HEX = '8b3387e9c5cdea6ac9e5edbaa115cd72'; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' ✓ ' + name); } + catch (e) { failed++; console.error(' ✗ ' + name + '\n ' + ((e && e.stack) || e)); } +} +function ok(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +// ─── crypto: the exact client-side scheme from public/channel-decrypt.js ── +// (key is a raw 16-byte PSK, NOT derived from a channel name) +// channelHash(1B) = SHA-256(key)[0] +// ciphertext = AES-128-ECB(plaintext), block-by-block, no chaining +// plaintext = timestamp(4 LE) + flags(1) + "sender: message" + 0x00, +// zero-padded to a 16-byte boundary +// mac(2B) = HMAC-SHA256(key + 16 zero bytes, ciphertext)[0:2] + +function channelHashByteFor(keyBytes) { + return crypto.createHash('sha256').update(keyBytes).digest()[0]; +} + +function buildPlaintext(timestampSec, flags, senderMessage) { + const head = Buffer.alloc(5); + head.writeUInt32LE(timestampSec >>> 0, 0); + head.writeUInt8(flags & 0xff, 4); + const body = Buffer.concat([head, Buffer.from(senderMessage, 'utf8'), Buffer.from([0x00])]); + const padded = Buffer.alloc(Math.max(16, Math.ceil(body.length / 16) * 16), 0); + body.copy(padded); + return padded; +} + +function encryptECB(keyBytes, plaintext) { + const cipher = crypto.createCipheriv('aes-128-ecb', keyBytes, null); + cipher.setAutoPadding(false); + return Buffer.concat([cipher.update(plaintext), cipher.final()]); +} + +function computeMac(keyBytes, ciphertext) { + const secret = Buffer.concat([keyBytes, Buffer.alloc(16, 0)]); + return crypto.createHmac('sha256', secret).update(ciphertext).digest().slice(0, 2); +} + +function rfc3339(ms) { + return new Date(ms).toISOString().replace(/\.\d{3}Z$/, 'Z'); +} + +// MeshCore wire header byte (cmd/server/decoder.go decodeHeader): bits 0-1 +// route type, bits 2-5 payload type, bits 6-7 payload version. RouteDirect=2, +// PayloadGRP_TXT=5 — a real "direct, no path" GRP_TXT packet, so the +// server's startup content-hash pass (ComputeContentHash, cmd/server/ +// decoder.go) parses it exactly as it would a captured one instead of +// mis-parsing an arbitrary first byte and colliding with unrelated rows. +const ROUTE_DIRECT = 2; +const PAYLOAD_GRP_TXT = 5; +const PAYLOAD_VERSION = 1; +const HEADER_BYTE = (ROUTE_DIRECT & 0x03) | ((PAYLOAD_GRP_TXT & 0x0f) << 2) | ((PAYLOAD_VERSION & 0x03) << 6); +const PATH_BYTE = 0x00; // hashSize bits=0, hashCount=0 — direct, no recorded hops + +/** Build the realistic wire bytes for a GRP_TXT packet: header + path + payload. */ +function buildRawPacket(channelHashByte, macBytes, ciphertext) { + return Buffer.concat([Buffer.from([HEADER_BYTE, PATH_BYTE, channelHashByte]), macBytes, ciphertext]); +} + +/** + * JS port of cmd/server/decoder.go's ComputeContentHash, for a packet built + * by buildRawPacket() (route=Direct, so no transport-code offset and no path + * bytes to skip). Keeping this in lock-step with the real server logic means + * the "hash" column we store is already the canonical value, so the + * server's startup hash-migration pass (hash_migrate.go) has nothing to + * change — it reads newHash === tx.Hash and leaves our seeded rows alone. + */ +function computeContentHash16(rawPacket) { + const payloadType = (rawPacket[0] >> 2) & 0x0f; + const payload = rawPacket.slice(2); // skip header byte + path byte + const toHash = Buffer.concat([Buffer.from([payloadType]), payload]); + return crypto.createHash('sha256').update(toHash).digest('hex').slice(0, 16); +} + +/** + * Seed two real, client-decryptable GRP_TXT packets into `dbPath`, under a + * PSK key the server has never seen, before the (read-only) server starts. + * One packet is observed only by an SJC-region observer, the other only by + * an SFO-region observer — so a region-scoped /api/packets?region= + * fetch returns exactly one of them. + * + * Returns { keyHex, channelName, hash, sjc: {sender,text}, sfo: {sender,text} } + */ +function seedEncryptedChannel(dbPath) { + const db = new DatabaseSync(dbPath); + try { + // Avoid colliding with any GRP_TXT channelHash byte already present in + // the fixture (real captured traffic on other, unknown channels) — a + // collision wouldn't break correctness (the MAC check still gates + // rendering) but it would add noise to the candidate set. + const used = new Set(); + for (const row of db.prepare( + "SELECT decoded_json FROM transmissions WHERE payload_type = 5 AND decoded_json LIKE '%channelHash%'" + ).all()) { + try { + const dj = JSON.parse(row.decoded_json); + if (typeof dj.channelHash === 'number') used.add(dj.channelHash); + } catch (e) { /* ignore malformed rows */ } + } + + let keyBytes, keyHex, hashByte; + for (let i = 0; i < 1000; i++) { + keyBytes = crypto.randomBytes(16); + keyHex = keyBytes.toString('hex'); + if (keyHex === DEFAULT_PUBLIC_KEY_HEX) continue; + hashByte = channelHashByteFor(keyBytes); + if (!used.has(hashByte)) break; + } + ok(keyBytes && !used.has(hashByte), 'could not find a non-colliding PSK key after 1000 tries'); + + const hashHex = hashByte.toString(16).toUpperCase().padStart(2, '0'); + const channelName = 'psk:' + keyHex.substring(0, 8); // matches addUserChannel()'s naming + + function observerRowid(iata) { + const row = db.prepare( + 'SELECT rowid FROM observers WHERE UPPER(TRIM(iata)) = ? ORDER BY rowid LIMIT 1' + ).get(iata); + ok(row, 'fixture has no observer with iata=' + iata); + return row.rowid; + } + const sjcObserverRowid = observerRowid('SJC'); + const sfoObserverRowid = observerRowid('SFO'); + + // #1690: post-migration schemas add transmissions.last_seen (used by the + // server's memory-cap / hot-window load query, ORDER BY last_seen DESC). + // Set it explicitly so our freshly-seeded rows always sort as "freshest" + // rather than defaulting to 0 and risking eviction behind 500 real rows. + const hasLastSeen = db.prepare("PRAGMA table_info(transmissions)").all() + .some((c) => c.name === 'last_seen'); + + const insertTx = db.prepare( + `INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, payload_version, decoded_json, channel_hash${hasLastSeen ? ', last_seen' : ''}) + VALUES (?, ?, ?, ?, 5, 1, ?, ?${hasLastSeen ? ', ?' : ''})` + ); + const insertObs = db.prepare( + `INSERT INTO observations (transmission_id, observer_idx, direction, snr, rssi, score, path_json, timestamp) + VALUES (?, ?, NULL, ?, ?, NULL, '[]', ?)` + ); + + function seedOne(observerRowid, sender, text, ageSec, snr) { + const nowMs = Date.now(); + const tsSec = Math.floor((nowMs - ageSec * 1000) / 1000); + const plaintext = buildPlaintext(tsSec, 0, sender + ': ' + text); + const ciphertext = encryptECB(keyBytes, plaintext); + const mac = computeMac(keyBytes, ciphertext); + const macHex = mac.toString('hex'); + const encryptedDataHex = ciphertext.toString('hex'); + const decoded = JSON.stringify({ + type: 'GRP_TXT', + encryptedData: encryptedDataHex, + mac: macHex, + channelHash: hashByte, + channelHashHex: hashHex, + decryptionStatus: 'no_key', + }); + const rawPacket = buildRawPacket(hashByte, mac, ciphertext); + const rawHex = rawPacket.toString('hex'); + const txHash = computeContentHash16(rawPacket); + const args = [rawHex, txHash, rfc3339(tsSec * 1000), ROUTE_DIRECT, decoded, 'enc_' + hashHex]; + if (hasLastSeen) args.push(tsSec); + const info = insertTx.run(...args); + insertObs.run(info.lastInsertRowid, observerRowid, snr, snr - 90, tsSec); + return { sender, text, hash: txHash }; + } + + const sjc = seedOne(sjcObserverRowid, 'IssueBot', 'SJC-ALPHA-MSG-152', 120, 6.5); + const sfo = seedOne(sfoObserverRowid, 'IssueBot', 'SFO-BRAVO-MSG-152', 60, 5.0); + + return { keyHex, channelName, hash: 'user:' + channelName, sjc, sfo }; + } finally { + db.close(); + } +} + +// ─── process lifecycle (subset of test-channel-proposals-e2e.js's) ──────── + +function findFallbackChromium() { + const base = process.env.PLAYWRIGHT_BROWSERS_PATH; + if (!base || !fs.existsSync(base)) return undefined; + let dirs; + try { dirs = fs.readdirSync(base).filter((d) => d.startsWith('chromium-')); } catch (e) { return undefined; } + for (const d of dirs) { + const p = path.join(base, d, 'chrome-linux', 'chrome'); + if (fs.existsSync(p)) return p; + } + return undefined; +} + +function freePort() { + return new Promise((resolve, reject) => { + const s = net.createServer(); + s.listen(0, '127.0.0.1', () => { const p = s.address().port; s.close(() => resolve(p)); }); + s.on('error', reject); + }); +} + +const children = new Set(); +let startCount = 0; +function startProcess(label, bin, args, dir) { + const logFile = path.join(dir, label + '-' + (++startCount) + '.log'); + const out = fs.openSync(logFile, 'w'); + const proc = spawn(bin, args, { stdio: ['ignore', out, out] }); + fs.closeSync(out); + proc.label = label; + proc.logFile = logFile; + children.add(proc); + proc.once('exit', () => children.delete(proc)); + return proc; +} + +async function stopProcess(proc) { + if (!proc || proc.exitCode !== null) return; + const exited = new Promise((r) => proc.once('exit', r)); + proc.kill('SIGTERM'); + const t = setTimeout(() => proc.kill('SIGKILL'), 5000); + await exited; + clearTimeout(t); +} + +async function waitFor(what, fn, timeoutMs) { + const deadline = Date.now() + (timeoutMs || TIMEOUT); + let last; + while (Date.now() < deadline) { + try { const v = await fn(); if (v) return v; } catch (e) { last = e; } + await new Promise((r) => setTimeout(r, 200)); + } + throw new Error('timed out waiting for ' + what + (last ? ': ' + last.message : '')); +} + +function logHas(proc, re) { + try { return re.test(fs.readFileSync(proc.logFile, 'utf8')); } catch (e) { return false; } +} + +// ─── minimal MQTT 3.1.1 broker (lets the ingestor start — no packets are +// published; everything this test needs is seeded directly into the DB +// instead). Copied from test-channel-proposals-e2e.js. ─────────────────── +function startFakeBroker() { + return new Promise((resolve) => { + const sockets = new Set(); + const server = net.createServer((sock) => { + sockets.add(sock); + sock.on('close', () => sockets.delete(sock)); + sock.on('error', () => {}); + let buf = Buffer.alloc(0); + sock.on('data', (d) => { + buf = Buffer.concat([buf, d]); + for (;;) { + if (buf.length < 2) return; + let len = 0; + let mult = 1; + let i = 1; + let complete = false; + for (; i < buf.length && i <= 4; i++) { + len += (buf[i] & 0x7f) * mult; + mult *= 128; + if ((buf[i] & 0x80) === 0) { complete = true; i++; break; } + } + if (!complete || buf.length < i + len) return; + const type = buf[0] >> 4; + const body = buf.slice(i, i + len); + buf = buf.slice(i + len); + if (type === 1) sock.write(Buffer.from([0x20, 0x02, 0x00, 0x00])); // CONNACK accepted + else if (type === 8) { // SUBSCRIBE → SUBACK granting QoS 0 per topic + let n = 0; + for (let p = 2; p + 2 <= body.length;) { + p += 2 + body.readUInt16BE(p) + 1; + n++; + } + sock.write(Buffer.from([0x90, 2 + n, body[0], body[1]].concat(new Array(n).fill(0)))); + } else if (type === 12) sock.write(Buffer.from([0xd0, 0x00])); // PINGRESP + else if (type === 14) sock.end(); + } + }); + }); + server.listen(0, '127.0.0.1', () => resolve({ + port: server.address().port, + close: () => { sockets.forEach((s) => s.destroy()); server.close(); }, + })); + }); +} + +/** + * Run the real ingestor binary just long enough to apply its schema + * migrations to `dbPath` (the server refuses to start against an + * unmigrated DB), then stop it. No MQTT traffic is needed or sent — the + * fake broker only exists so the ingestor's startup sequence (which runs + * migrations before connecting) completes cleanly instead of timing out. + */ +async function migrateFixture(dbPath, dir) { + const broker = await startFakeBroker(); + const ingestorConfig = path.join(dir, 'ingestor.json'); + fs.writeFileSync(ingestorConfig, JSON.stringify({ + dbPath, + mqttSources: [{ name: 'e2e', broker: 'mqtt://127.0.0.1:' + broker.port, topics: ['meshcore/#'], connectTimeoutSec: 5 }], + })); + const ingestor = startProcess('ingestor', INGESTOR_BIN, ['-config', ingestorConfig], dir); + try { + await waitFor('ingestor migration + MQTT subscription', () => { + if (ingestor.exitCode !== null) throw new Error('ingestor exited: see ' + ingestor.logFile); + return logHas(ingestor, /MQTT \[e2e\] subscribed/); + }); + } finally { + await stopProcess(ingestor); + broker.close(); + } +} + +// ─── page helpers ────────────────────────────────────────────────────── + +// The region pills are a MULTI-select toggle (region-filter.js's +// toggleRegion): clicking a region that isn't selected ADDS it to the +// current set (or, when nothing is selected yet, becomes the sole +// selection — the "All regions" default is a null set, not an empty one). +// Starting from "All regions" (the page's default, untouched state) and +// clicking exactly one region pill is therefore a single, clean, +// uncontested region change — no intermediate combined-region state, and +// critically no intermediate /api/packets fetch that would populate +// channel-decrypt.js's (region-unaware) message cache with the wrong +// region's content before the real switch this test cares about happens. +async function selectOnlyRegion(page, code) { + const pill = page.locator('#chRegionFilter [data-region="' + code + '"]'); + await pill.waitFor({ state: 'attached', timeout: 8000 }); + const response = page.waitForResponse((r) => { + const u = decodeURIComponent(r.url()); + return u.indexOf('/api/channels?') !== -1 && u.indexOf('/messages') === -1 && u.indexOf('region=' + code) !== -1; + }, { timeout: 8000 }); + const click = pill.click({ timeout: 2000 }).catch(() => pill.evaluate((el) => el.click())); + await Promise.all([response, click]); +} + +function messagePaneState(page) { + return page.evaluate(() => { + const msgEl = document.getElementById('chMessages'); + const row = document.querySelector('#chList .ch-row.selected, #chList .ch-item.selected, #chList [aria-selected="true"]'); + return { + urlHash: location.hash, + loading: !!(msgEl && (msgEl.querySelector('.ch-loading') || /Decrypting/.test(msgEl.textContent))), + text: msgEl ? msgEl.textContent : '', + senders: msgEl ? Array.from(msgEl.querySelectorAll('.ch-msg-sender')).map((e) => e.textContent) : [], + bubbles: msgEl ? Array.from(msgEl.querySelectorAll('.ch-msg-bubble')).map((e) => e.textContent) : [], + rowSelectedHash: row ? row.getAttribute('data-hash') : null, + }; + }); +} + +// ─── test body ───────────────────────────────────────────────────────── + +async function runViewport(browser, vp, base, seed) { + const ctx = await browser.newContext({ viewport: { width: vp.width, height: vp.height } }); + const page = await ctx.newPage(); + page.setDefaultTimeout(10000); + const pageErrors = []; + page.on('pageerror', (e) => pageErrors.push(e.message)); + + await page.goto(base + '/#/channels', { waitUntil: 'domcontentloaded' }); + await page.evaluate(() => { try { localStorage.clear(); } catch (e) {} }); + await page.reload({ waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#chList .ch-item, #chList .ch-row', { timeout: 10000 }); + + // Region starts at its page default ("All regions", a null selection) — + // deliberately NOT pre-selecting SJC here. The first decrypt fetch below + // therefore runs unfiltered (sees both the SJC and SFO packets), and the + // single-click switch to SFO further down is a clean, uncontested region + // change (see selectOnlyRegion's doc comment) instead of needing a second + // click to undo a prior region pre-selection — which would itself issue + // an intermediate /api/packets fetch and pollute channel-decrypt.js's + // (region-unaware, keyed only by channel name) message cache with + // combined-region content before the real switch under test happens. + + // Delay only the FIRST /api/packets response by ~3s, so it's still + // in-flight when the region switch below fires. Later calls (the + // region-change handler's own re-fetch, once F1 is fixed) pass straight + // through so the test doesn't just become an exercise in waiting. + let packetsCalls = 0; + await page.route('**/api/packets**', async (route) => { + packetsCalls++; + if (packetsCalls === 1) { + await new Promise((r) => setTimeout(r, 3000)); + } + await route.continue(); + }); + + await step(vp.name + ': adding the seeded PSK opens its conversation and starts decrypting', async () => { + await page.click('#chAddChannelBtn'); + await page.waitForSelector('#chAddChannelModal:not(.hidden)'); + await page.fill('#chPskKey', seed.keyHex); + await page.fill('#chPskName', 'Issue152 Decrypt Race'); + await page.click('#chPskAddBtn'); + await page.waitForFunction((hash) => location.hash === '#/channels/' + encodeURIComponent(hash), + seed.hash, { timeout: 10000 }); + }); + + await step(vp.name + ': caught mid-decrypt — still showing "Decrypting messages..." when region changes', async () => { + const s = await messagePaneState(page); + ok(s.loading, 'expected the decrypt to still be in flight (pane: ' + JSON.stringify(s.text.slice(0, 80)) + ')'); + ok(packetsCalls === 1, 'expected exactly 1 /api/packets call so far, got ' + packetsCalls); + }); + + await step(vp.name + ': narrow to region SFO while the (unfiltered) decrypt is still in flight', async () => { + await selectOnlyRegion(page, 'SFO'); + }); + + await step(vp.name + ': the pane eventually settles (not stuck on "Decrypting...")', async () => { + await page.waitForFunction(() => { + const m = document.getElementById('chMessages'); + return m && !m.querySelector('.ch-loading') && !/Decrypting/.test(m.textContent); + }, null, { timeout: 15000 }); + }); + + await step(vp.name + ': the pane shows the NEW region (SFO) message, not the stale SJC one', async () => { + const s = await messagePaneState(page); + ok(s.text.indexOf(seed.sfo.text) !== -1, + 'expected the SFO message in the pane, got: ' + JSON.stringify(s.text.slice(0, 200))); + ok(s.text.indexOf(seed.sjc.text) === -1, + 'the stale SJC-region message must not be shown after switching to SFO, got: ' + JSON.stringify(s.text.slice(0, 200))); + }); + + await step(vp.name + ': the conversation stayed open on the same PSK channel (not closed/reset)', async () => { + const s = await messagePaneState(page); + ok(s.urlHash === '#/channels/' + encodeURIComponent(seed.hash), 'URL must stay on the PSK channel, got ' + s.urlHash); + ok(s.text.indexOf('Choose a channel') === -1, 'conversation must not have closed'); + ok(s.rowSelectedHash === seed.hash, 'the PSK row must still be selected, got ' + s.rowSelectedHash); + }); + + await step(vp.name + ': no uncaught page errors', async () => { + // "L is not defined" is Leaflet (public/index.html loads it from + // unpkg.com) failing to load because this test's sandbox has no + // outbound internet access to third-party CDNs — unrelated to the + // channels/decrypt code this test exercises, and already the case + // with no PSK/region interaction at all. A real environment (CI, + // staging) has network access and never sees it; test-channels- + // client-state-152-e2e.js (this test's sibling for the same issue) + // likewise only logs page errors for this page rather than failing on + // them, for the same reason. + const unexpected = pageErrors.filter((m) => !/^L is not defined$/.test(m)); + assert.deepStrictEqual(unexpected, [], 'unexpected page errors: ' + JSON.stringify(pageErrors)); + }); + + await page.evaluate(() => { try { localStorage.clear(); } catch (e) {} }); + await ctx.close(); +} + +async function main() { + for (const p of [SERVER_BIN, FIXTURE_DB]) { + if (!fs.existsSync(p)) { + console.error('test-channels-client-state-152-decrypt-e2e.js: FAIL — missing ' + p); + process.exit(1); + } + } + + let browser; + const launchArgs = { headless: true, args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'] }; + try { + browser = await chromium.launch(Object.assign({}, launchArgs, { executablePath: process.env.CHROMIUM_PATH || undefined })); + } catch (err) { + // Fallback: the resolved `playwright` package's expected browser revision + // may not match what's actually unpacked under PLAYWRIGHT_BROWSERS_PATH + // (e.g. a sibling process installed a newer playwright into this shared + // worktree's node_modules). Point at whatever chromium build IS present + // instead of giving up — an executablePath override works across minor + // version gaps for the plain navigation/fill/click/route APIs this test + // uses. + const fallback = !process.env.CHROMIUM_PATH && findFallbackChromium(); + if (fallback) { + try { + browser = await chromium.launch(Object.assign({}, launchArgs, { executablePath: fallback })); + console.log('test-channels-client-state-152-decrypt-e2e.js: using fallback Chromium at ' + fallback); + } catch (err2) { + err = err2; + } + } + if (!browser) { + if (process.env.CHROMIUM_REQUIRE === '1') { + console.error('test-channels-client-state-152-decrypt-e2e.js: FAIL — Chromium required but unavailable: ' + err.message); + process.exit(1); + } + console.log('test-channels-client-state-152-decrypt-e2e.js: SKIP (Chromium unavailable: ' + err.message.split('\n')[0] + ')'); + process.exit(0); + } + } + + if (!fs.existsSync(INGESTOR_BIN)) { + console.error('test-channels-client-state-152-decrypt-e2e.js: FAIL — missing ' + INGESTOR_BIN); + process.exit(1); + } + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'corescope-decrypt152-e2e-')); + const dbPath = path.join(dir, 'meshcore.db'); + const serverDir = path.join(dir, 'server'); + fs.mkdirSync(serverDir); + fs.copyFileSync(FIXTURE_DB, dbPath); + + console.log('running the ingestor once to migrate the fixture schema ...'); + await migrateFixture(dbPath, dir); + + console.log('seeding encrypted PSK packets into ' + dbPath + ' ...'); + const seed = seedEncryptedChannel(dbPath); + console.log(' PSK key: ' + seed.keyHex); + console.log(' channel hash: ' + seed.hash); + console.log(' SJC message: "' + seed.sjc.sender + ': ' + seed.sjc.text + '"'); + console.log(' SFO message: "' + seed.sfo.sender + ': ' + seed.sfo.text + '"'); + + const port = await freePort(); + const base = 'http://127.0.0.1:' + port; + + try { + const server = startProcess('server', SERVER_BIN, + ['-port', String(port), '-db', dbPath, '-public', PUBLIC_DIR, '-config-dir', serverDir], + dir); + await waitFor('server health', async () => { + if (server.exitCode !== null) throw new Error('server exited early: see ' + server.logFile); + const res = await fetch(base + '/api/healthz'); + return res.ok; + }); + await waitFor('server reports SJC and SFO regions', async () => { + const res = await fetch(base + '/api/config/regions'); + const regions = await res.json(); + return 'SJC' in regions && 'SFO' in regions; + }); + + console.log('\n=== #152 decrypt-race E2E against ' + base + ' ==='); + await runViewport(browser, { name: 'desktop', width: 1280, height: 800 }, base, seed); + await runViewport(browser, { name: 'mobile', width: 390, height: 844 }, base, seed); + } finally { + await browser.close().catch(() => {}); + for (const proc of Array.from(children)) await stopProcess(proc); + if (failed) console.error('logs kept in ' + dir); + else fs.rmSync(dir, { recursive: true, force: true }); + } + + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); + process.exit(failed > 0 ? 1 : 0); +} + +if (require.main === module) { + main().catch((e) => { + console.error('test-channels-client-state-152-decrypt-e2e.js: FATAL — ' + ((e && e.stack) || e)); + process.exit(1); + }); +} + +module.exports = { + seedEncryptedChannel, channelHashByteFor, buildPlaintext, encryptECB, computeMac, + buildRawPacket, computeContentHash16, +}; From 371d3f4969803ff3bacb88ff370cca47564f0102 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 10:27:20 +0000 Subject: [PATCH 08/23] =?UTF-8?q?fix(channels):=20N1=20=E2=80=94=20restart?= =?UTF-8?q?=20decryption=20when=20a=20remap=20happens=20mid-decrypt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A region switch, the show-encrypted toggle, or a shared-channel approval can all trigger loadChannels() -> reconcileSelectionAfterChannelRefresh(), which remaps an open user:* PSK conversation to a same-named row mergeUserChannels() just matched by name (#152 F3, round 2). If a decrypt for the old user:* hash was still in flight at that exact moment, the remap changes selectedHash out from under it: the in-flight fetch's own staleness check (isStaleMessageRequest) discards its result once it resolves, and nothing else restarts the fetch for the remapped hash — the pane is left stuck on "Decrypting messages…" forever. Track whether a decrypt is pending for the current selection (messageLoadPending, set/cleared around decryptAndRender()). When reconcile remaps a selection while that flag is set, it now also calls selectChannel(remapped.hash) to restart loading for the new hash. A quiet remap (no decrypt in flight) still leaves messages untouched, preserving the round-2 F3 invariant. Tests: a new unit test starts a real decrypt (in-flight /packets fetch) on a user:* hash, triggers a remap to a server-known row mid-flight, and asserts the remapped channel's own messages load (not stuck on "Decrypting…"). A second test guards the quiet-remap case. Confirmed red by reverting the reconcile change (pane stays stuck on "Decrypting…"). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- public/channels.js | 86 ++++++++++++++++++++----------- test-channels-client-state-152.js | 62 ++++++++++++++++++++++ 2 files changed, 118 insertions(+), 30 deletions(-) diff --git a/public/channels.js b/public/channels.js index 89dc2be8e..4bea7c6f0 100644 --- a/public/channels.js +++ b/public/channels.js @@ -236,6 +236,14 @@ let observerIataById = {}; let observerIataByName = {}; let messageRequestId = 0; + // N1 (#152 follow-up): true while a decryptAndRender() fetch for the + // current selection is in flight (set/cleared around that one call). + // Lets reconcileSelectionAfterChannelRefresh() know, when it remaps a + // user:* selection mid-decrypt, that it must restart loading for the + // remapped hash — otherwise the in-flight request's own staleness check + // discards its result once selectedHash changes, and nothing else ever + // re-fetches, leaving the pane stuck on "Decrypting messages…". + let messageLoadPending = false; var _nodeCacheTTL = 5 * 60 * 1000; // 5 minutes function getSelectedRegionsSnapshot() { @@ -317,9 +325,18 @@ var pskName = selectedHash.substring(5); var remapped = channels.find(function (ch) { return ch.userAdded === true && ch.name === pskName; }); if (remapped) { + // N1 (#152 follow-up): if a decrypt for the old hash was still in + // flight at the moment of this remap, it will discard its own + // result (isStaleMessageRequest() sees selectedHash change under + // it) and nothing else ever restarts the fetch — the pane would be + // stuck on "Decrypting messages…" forever. Restart loading for the + // remapped hash in that case. A quiet remap (no decrypt in flight) + // still leaves messages untouched, as before. + var hadPendingDecrypt = messageLoadPending; selectedHash = remapped.hash; history.replaceState(null, '', `#/channels/${encodeURIComponent(selectedHash)}`); renderChannelList(); + if (hadPendingDecrypt) selectChannel(selectedHash); return false; } } @@ -2417,39 +2434,48 @@ // Shared helper: fetch, decrypt, and render messages for a channel key (M5: cache-first) async function decryptAndRender(keyHex, channelHashByte, channelName) { msgEl.innerHTML = '
Decrypting messages…
'; - var result = await fetchAndDecryptChannel(keyHex, channelHashByte, channelName, { - onCacheHit: function (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(); - scrollToBottom(); + // N1 (#152 follow-up): mark a decrypt as pending for the current + // selection so reconcileSelectionAfterChannelRefresh() can tell, when + // it remaps a selection mid-decrypt, that it must restart loading for + // the remapped hash instead of leaving the pane stuck on this message. + messageLoadPending = true; + try { + var result = await fetchAndDecryptChannel(keyHex, channelHashByte, channelName, { + onCacheHit: function (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(); + scrollToBottom(); + } } + }); + if (isStaleMessageRequest(request)) return { stale: true }; + if (result.wrongKey) { + msgEl.innerHTML = '
Key does not match — no messages could be decrypted
'; + return { wrongKey: true, messageCount: 0 }; } - }); - if (isStaleMessageRequest(request)) return { stale: true }; - if (result.wrongKey) { - msgEl.innerHTML = '
Key does not match — no messages could be decrypted
'; - return { wrongKey: true, messageCount: 0 }; - } - if (result.error) { - msgEl.innerHTML = '
' + escapeHtml(result.error) + '
'; - return { error: result.error, messageCount: 0 }; - } - // #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 { - header.querySelector('.ch-header-text').textContent = `${name} — ${messages.length} messages (decrypted)`; - renderMessages(); - scrollToBottom(); + if (result.error) { + msgEl.innerHTML = '
' + escapeHtml(result.error) + '
'; + return { error: result.error, messageCount: 0 }; + } + // #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 { + header.querySelector('.ch-header-text').textContent = `${name} — ${messages.length} messages (decrypted)`; + renderMessages(); + scrollToBottom(); + } + return { messageCount: messages.length }; + } finally { + messageLoadPending = false; } - return { messageCount: messages.length }; } // Client-side decryption path (#725 M2) diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 987ac2fe9..82cbd5897 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -739,6 +739,68 @@ async function test(name, fn) { assert.strictEqual(row.messageCount, 1); }); + // ── N1 (#153 review round 3, P3): a remap that happens while a decrypt is + // still in flight must restart loading for the remapped channel, not + // leave the pane stuck on "Decrypting messages…". The in-flight request's + // own staleness check (isStaleMessageRequest — selectedHash changed under + // it) silently discards its result once reconcile remaps the selection, + // and nothing else used to restart the fetch. + await test('N1: a remap mid-decrypt restarts loading for the remapped channel', async () => { + const h = makeHarness(); + const KEY = 'a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1'; + const NAME = '#n1chat'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'N1 Chat'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + + const msg = encryptChannelMessage(KEY, 'Alice', 'remapped channel message'); + + const pending = deferred(); + h.respondPackets = () => pending.promise; + const select = h.w._channelsSelectChannelForTest(HASH); + await flush(300); + assert.ok(/Decrypting/.test(h.elements.chMessages.innerHTML), 'pane must show "Decrypting…" while the fetch is in flight'); + + // #n1chat becomes server-known (e.g. approved) in the next refresh — + // mergeUserChannels() name-matches it instead of keeping a user:* row, + // so reconcile remaps the open selection to it mid-decrypt. + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-remap', '2026-01-01T00:00:00Z', msg)] }); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel(NAME, { name: NAME })] }); + await h.w._channelsLoadChannelsForTest(true); + await flush(300); + + // The original (now-stale) fetch finally resolves too; it must not + // clobber the remapped channel's freshly-loaded view. + pending.resolve({ packets: [] }); + await select; + await flush(300); + + assert.strictEqual(h.state().selectedHash, NAME, 'selection must have remapped to the server-known hash'); + const msgEl = h.elements.chMessages; + assert.ok(!/Decrypting/.test(msgEl.innerHTML), 'pane must not be stuck on "Decrypting…" (got ' + msgEl.innerHTML.slice(0, 200) + ')'); + const texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('remapped channel message') !== -1, 'the remapped channel must actually load its messages (got ' + JSON.stringify(texts) + ')'); + }); + + await test('N1: a quiet remap (no decrypt in flight) still leaves messages untouched', async () => { + // Regression guard for the F3 (round 2) invariant: only restart loading + // when a decrypt was genuinely pending at remap time. + const h = makeHarness(); + const NAME = '#n1quiet'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, 'cc'.repeat(16), 'Quiet'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + h.setState({ selectedHash: HASH, messages: [{ sender: 'Alice', text: 'already loaded', timestamp: '2026-01-01T00:00:00Z', packetHash: 'm1' }] }); + + h.respondChannels = () => Promise.resolve({ channels: [serverChannel(NAME, { name: NAME })] }); + await h.w._channelsLoadChannelsForTest(true); + + assert.strictEqual(h.state().selectedHash, NAME, 'selection must remap'); + assert.strictEqual(h.state().messages.length, 1, 'messages must not be cleared by a quiet remap (got ' + JSON.stringify(h.state().messages) + ')'); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From c8d6f3c609a1be85c3e58814943a9608a5fdc22c Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 10:30:20 +0000 Subject: [PATCH 09/23] =?UTF-8?q?fix(channels):=20N2=20=E2=80=94=20region-?= =?UTF-8?q?scope=20the=20client-side=20decrypt=20cache?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fetchAndDecryptChannel()'s cache key was the channel name alone, with no region component, even though the fetched/decrypted message set depends entirely on which observers the current region filter includes. This let a cache entry primed under one region answer a fetch for a different region: - Zero candidates for the current region fell through to `return { messages: cachedMsgs, empty: true }`, showing a foreign region's messages instead of an empty list (e.g. All->OAK showed the SJC+SFO messages cached under "All"; SJC->MRY showed SJC's message). - The delta path's "0 new candidates since lastTs" short-circuit (`newCandidates.length === 0`) trusted the cache's count/lastTs across regions too: a different region with the same candidate count but an older timestamp returned the old region's cached message outright. - A stale (superseded) decrypt could still write its results into the cache after being superseded, since the write sites had no visibility into request staleness at all. Fixes: the cache key now includes the region param (sorted, so selection order doesn't fragment it); a zero-candidate result always renders as an empty list, never leftover cached content; every cache write goes through setCacheIfFresh(), which checks the caller-supplied isStale() (backed by isStaleMessageRequest()) first. Also capped the number of distinct cache entries (MAX_CACHE_KEYS, channel-decrypt.js) since region-scoped keys mean one channel can now occupy several, evicting the least-recently-written ones. Tests: three new unit tests cover cross-region isolation (All->OAK), the same-count delta-path pitfall (SJC->MRY), and that a superseded decrypt's evidence never reaches the cache. A new E2E step in test-channels-client-state-152-decrypt-e2e.js switches to a traffic-free region (OAK) after the existing decrypt has fully settled and asserts an empty pane, confirmed red against the pre-fix code in a real browser. Confirmed red for the unit tests too, each against its own mutant (dropping the region from the cache key; dropping the isStale guard in setCacheIfFresh). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- public/channel-decrypt.js | 10 ++ public/channels.js | 36 ++++-- test-channels-client-state-152-decrypt-e2e.js | 51 ++++++++ test-channels-client-state-152.js | 115 ++++++++++++++++++ 4 files changed, 204 insertions(+), 8 deletions(-) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index b74560622..447450682 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -383,6 +383,11 @@ window.ChannelDecrypt = (function () { // Cache with lastTimestamp and count (used by channels.js via getCache/setCache) var MAX_CACHED_MESSAGES = 1000; + // N2 (#152 follow-up): keys are now region-scoped ("|"), + // so one channel can occupy several entries. Cap the number of distinct + // entries so visiting many region combinations over time can't grow this + // localStorage blob unboundedly; evict the least-recently-written ones. + var MAX_CACHE_KEYS = 50; function setCache(key, messages, lastTimestamp, totalCount) { try { @@ -398,6 +403,11 @@ window.ChannelDecrypt = (function () { count: totalCount || toStore.length, ts: Date.now() }; + var keys = Object.keys(cache); + if (keys.length > MAX_CACHE_KEYS) { + keys.sort(function (a, b) { return (cache[a].ts || 0) - (cache[b].ts || 0); }); + for (var i = 0; i < keys.length - MAX_CACHE_KEYS; i++) delete cache[keys[i]]; + } localStorage.setItem(CACHE_KEY, JSON.stringify(cache)); } catch (e) { /* quota */ } } diff --git a/public/channels.js b/public/channels.js index 4bea7c6f0..720cc47c0 100644 --- a/public/channels.js +++ b/public/channels.js @@ -811,8 +811,23 @@ opts = opts || {}; var keyBytes = ChannelDecrypt.hexToBytes(keyHex); + // N2 (#152 follow-up): the cache must be region-scoped. A channel's + // decrypted message set depends on which observers the current region + // filter includes, so a cache entry primed under one region (e.g. "All" + // or SJC) must never answer a fetch for a different region (e.g. OAK or + // MRY) — region order doesn't matter, so sort for a stable key. + var rp = RegionFilter.getRegionParam(); + var regionKey = rp ? rp.split(',').filter(Boolean).sort().join(',') : ''; + var cacheKey = (channelName || String(channelHashByte)) + '|' + regionKey; + // A stale request (superseded by a newer one, or no longer the current + // selection/region by the time a write below would happen) must not + // persist its possibly-incomplete evidence into the cache. + function setCacheIfFresh(key, msgs, ts, count) { + if (opts.isStale && opts.isStale()) return; + ChannelDecrypt.setCache(key, msgs, ts, count); + } + // M5: Check cache first — serve cached messages immediately - var cacheKey = channelName || String(channelHashByte); var cached = ChannelDecrypt.getCache(cacheKey); var cachedMsgs = cached ? cached.messages : []; var lastTs = cached ? cached.lastTimestamp : ''; @@ -825,7 +840,6 @@ } // Fetch packets from API — get all payload_type=5 (GRP_TXT/CHAN) - var rp = RegionFilter.getRegionParam(); var qs = (rp ? '®ion=' + encodeURIComponent(rp) : ''); var data; try { @@ -872,7 +886,7 @@ // Nothing new to decrypt. Persist only when the API enriched the // evidence so legacy caches gain the badge on this render. if (reconciledCache.changed) { - ChannelDecrypt.setCache(cacheKey, cachedMsgs, lastTs, totalCandidates); + setCacheIfFresh(cacheKey, cachedMsgs, lastTs, totalCandidates); } return { messages: cachedMsgs, fromCache: true }; } @@ -881,7 +895,7 @@ var newDecrypted = await decryptCandidates(keyBytes, newCandidates); if (newDecrypted.wrongKey) { if (reconciledCache.changed) { - ChannelDecrypt.setCache(cacheKey, cachedMsgs, lastTs, totalCandidates); + setCacheIfFresh(cacheKey, cachedMsgs, lastTs, totalCandidates); } return { messages: cachedMsgs, wrongKey: true }; } @@ -889,12 +903,15 @@ // Merge: cached + new, deduplicate by packetHash, sort chronologically var merged = deduplicateAndMerge(cachedMsgs, newDecrypted.messages); var newLastTs = merged.length ? merged[merged.length - 1].timestamp : lastTs; - ChannelDecrypt.setCache(cacheKey, merged, newLastTs, totalCandidates); + setCacheIfFresh(cacheKey, merged, newLastTs, totalCandidates); return { messages: merged, deltaCount: newDecrypted.messages.length }; } if (candidates.length === 0) { - return { messages: cachedMsgs, empty: true }; + // N2 (#152 follow-up): zero candidates for the current (region-scoped) + // fetch must render as empty, never leftover content from a stale or + // foreign-region cache entry. + return { messages: [], empty: true }; } // Full decrypt @@ -913,7 +930,7 @@ // M5: Cache results var newLastTimestamp = decrypted.length ? decrypted[decrypted.length - 1].timestamp : ''; - ChannelDecrypt.setCache(cacheKey, decrypted, newLastTimestamp, totalCandidates); + setCacheIfFresh(cacheKey, decrypted, newLastTimestamp, totalCandidates); return { messages: decrypted }; } @@ -2452,7 +2469,10 @@ renderMessages(); scrollToBottom(); } - } + }, + // N2 (#152 follow-up): a superseded request must not persist its + // evidence into the (now region-scoped) decrypt cache. + isStale: function () { return isStaleMessageRequest(request); } }); if (isStaleMessageRequest(request)) return { stale: true }; if (result.wrongKey) { diff --git a/test-channels-client-state-152-decrypt-e2e.js b/test-channels-client-state-152-decrypt-e2e.js index b62953f76..b2a041e59 100644 --- a/test-channels-client-state-152-decrypt-e2e.js +++ b/test-channels-client-state-152-decrypt-e2e.js @@ -28,6 +28,12 @@ * showing just the SFO message (not stuck "Decrypting...", not the stale * SJC-region message) with the conversation still open on the same hash. * + * #153 review round 3 adds a further step after the decrypt has fully + * settled: switching to OAK (no observer saw either seeded packet) must + * show an empty pane, not a lingering SJC/SFO message from the decrypt + * cache (N2 — the cache was keyed by channel name alone, with no region + * component). + * * Runs at desktop (1280x800) and mobile (390x844) viewports. * * Seeding: no sqlite3 CLI is available in this environment, so this file @@ -410,6 +416,29 @@ async function selectOnlyRegion(page, code) { await Promise.all([response, click]); } +// Toggles an already-selected region pill back off (multi-select: clicking +// a selected pill removes it). Used to get back to a clean single-selection +// state before selectOnlyRegion's next clean click, instead of ending up +// with a combined "SFO,OAK" selection. +async function deselectRegion(page, code) { + const pill = page.locator('#chRegionFilter [data-region="' + code + '"]'); + await pill.waitFor({ state: 'attached', timeout: 8000 }); + // Deselecting the only selected region goes back to "All regions" — a + // null selection. loadChannels() then queries with no `region=` param, + // which the client's own TTL cache (CLIENT_TTL.channels) may already hold + // from the very first page load, so no new network request is guaranteed + // to fire here — wait for the pill UI to reflect the toggle instead. + await pill.click({ timeout: 2000 }).catch(() => pill.evaluate((el) => el.click())); + // Deselecting the one selected region goes back to "All regions", which + // re-renders EVERY pill (including the one just clicked) as checked — so + // "this pill is now unchecked" is never true here. Wait for the "__all__" + // pill to become the active one instead. + await page.waitForFunction(() => { + const el = document.querySelector('#chRegionFilter [data-region="__all__"]'); + return el && el.classList.contains('region-pill-active'); + }, null, { timeout: 8000 }); +} + function messagePaneState(page) { return page.evaluate(() => { const msgEl = document.getElementById('chMessages'); @@ -504,6 +533,28 @@ async function runViewport(browser, vp, base, seed) { ok(s.rowSelectedHash === seed.hash, 'the PSK row must still be selected, got ' + s.rowSelectedHash); }); + // N2 (#153 review round 3): the decrypt cache was keyed by channel name + // alone, so a region with zero traffic for this channel — switched to + // well AFTER the earlier decrypt has fully settled, no race involved — + // could still show a previous region's cached messages. OAK has no + // observer for either seeded packet. + await step(vp.name + ': switching to a region with no traffic for this channel shows an empty pane, not a stale one', async () => { + // Region pills are multi-select: deselect SFO first so the OAK click + // below is a clean single-region switch, not an accumulated "SFO,OAK". + await deselectRegion(page, 'SFO'); + await selectOnlyRegion(page, 'OAK'); + await page.waitForFunction(() => { + const m = document.getElementById('chMessages'); + return m && !m.querySelector('.ch-loading') && !/Decrypting/.test(m.textContent); + }, null, { timeout: 15000 }); + const s = await messagePaneState(page); + ok(s.text.indexOf(seed.sfo.text) === -1, + 'OAK (no traffic) must not show the SFO-region cached message, got: ' + JSON.stringify(s.text.slice(0, 200))); + ok(s.text.indexOf(seed.sjc.text) === -1, + 'OAK (no traffic) must not show the SJC-region cached message, got: ' + JSON.stringify(s.text.slice(0, 200))); + ok(s.urlHash === '#/channels/' + encodeURIComponent(seed.hash), 'URL must stay on the PSK channel, got ' + s.urlHash); + }); + await step(vp.name + ': no uncaught page errors', async () => { // "L is not defined" is Leaflet (public/index.html loads it from // unpkg.com) failing to load because this test's sandbox has no diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 82cbd5897..d40bf7f2e 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -801,6 +801,121 @@ async function test(name, fn) { assert.strictEqual(h.state().messages.length, 1, 'messages must not be cleared by a quiet remap (got ' + JSON.stringify(h.state().messages) + ')'); }); + // ── N2 (#153 review round 3, P3): the client-side decrypt cache must be + // region-scoped. The cache key was the channel name alone, so a cache + // entry primed under one region answered a fetch for a different region. + await test('N2: the decrypt cache is region-scoped — a different region never shows another region\'s cached messages', async () => { + const h = makeHarness(); + const KEY = 'a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2'; + const NAME = 'psk:n2test'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'N2 Team'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + + const sjcMsg = encryptChannelMessage(KEY, 'Alice', 'sjc message'); + const sfoMsg = encryptChannelMessage(KEY, 'Bob', 'sfo message'); + + // Region "All" (no filter): the decrypt fetch sees both packets. + h.respondPackets = () => Promise.resolve({ packets: [ + encryptedPacket('pkt-sjc', '2026-01-01T00:00:00Z', sjcMsg), + encryptedPacket('pkt-sfo', '2026-01-01T00:01:00Z', sfoMsg), + ] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + let texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('sjc message') !== -1 && texts.indexOf('sfo message') !== -1 && texts.length === 2, + 'All region must see both messages (got ' + JSON.stringify(texts) + ')'); + + // Switch to OAK: no observer there saw either packet (0 candidates). + h.regionParam = 'OAK'; + h.respondPackets = () => Promise.resolve({ packets: [] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + texts = h.state().messages.map((m) => m.text); + assert.strictEqual(texts.length, 0, 'OAK region (0 candidates) must show an empty list, not the All-region cache (got ' + JSON.stringify(texts) + ')'); + }); + + await test('N2: a same-candidate-count delta fetch in a different region does not show the old region\'s message', async () => { + const h = makeHarness(); + const KEY = 'b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2'; + const NAME = 'psk:n2delta'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'N2 Delta'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + + const sjcMsg = encryptChannelMessage(KEY, 'Alice', 'sjc-only message'); + const mryMsg = encryptChannelMessage(KEY, 'Carol', 'mry-only message'); + + h.regionParam = 'SJC'; + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-sjc', '2026-01-01T00:00:00Z', sjcMsg)] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + assert.ok(h.state().messages.some((m) => m.text === 'sjc-only message'), 'precondition: SJC message cached'); + + // MRY: exactly one candidate too (same count as the SJC cache), but an + // older timestamp — if the cache weren't region-scoped, the delta path + // would see "0 new candidates since lastTs" and return SJC's cached + // message instead of MRY's own. + h.regionParam = 'MRY'; + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-mry', '2025-01-01T00:00:00Z', mryMsg)] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + const texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('mry-only message') !== -1, 'MRY must show its own message (got ' + JSON.stringify(texts) + ')'); + assert.ok(texts.indexOf('sjc-only message') === -1, 'MRY must not show the SJC-region cached message (got ' + JSON.stringify(texts) + ')'); + }); + + await test('N2: a stale (superseded) decrypt does not write its evidence into the cache', async () => { + const h = makeHarness(); + const KEY = 'c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3c3'; + const NAME = 'psk:n2stale'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'N2 Stale'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + + const staleMsg = encryptChannelMessage(KEY, 'Alice', 'stale message'); + const realMsg = encryptChannelMessage(KEY, 'Bob', 'real message'); + + const pending = deferred(); + h.respondPackets = () => pending.promise; // first decrypt stays in flight + const select1 = h.w._channelsSelectChannelForTest(HASH); + await flush(300); + + // Supersede it before it resolves (e.g. a second region switch). Zero + // candidates — this leg never touches the cache either way, so it can't + // mask the thing under test. + h.respondPackets = () => Promise.resolve({ packets: [] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + + // The first (now-stale) fetch resolves with one real decryptable + // candidate. If the isStale guard were missing, this would write + // { messages: [stale message], count: 1, lastTimestamp: T } to the + // cache. + pending.resolve({ packets: [encryptedPacket('pkt-stale', '2020-01-01T00:00:00Z', staleMsg)] }); + await select1; + await flush(300); + + // A later select sees exactly ONE candidate too (same count a poisoned + // cache entry would hold) with the SAME timestamp as the stale one — + // not a different message rendered via onCacheHit, but specifically the + // delta path's own "0 candidates newer than lastTs" short-circuit + // (fetchAndDecryptChannel, the `newCandidates.length === 0` branch), + // which trusts the cache's count/lastTs and returns its cached messages + // outright. That only reaches the real candidate instead of the stale + // one if the cache was never poisoned in the first place (cachedCount + // stays 0, so this fetch takes the full-decrypt path instead of delta). + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-real', '2020-01-01T00:00:00Z', realMsg)] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + const texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('stale message') === -1, 'a stale decrypt must not have poisoned the cache (got ' + JSON.stringify(texts) + ')'); + assert.ok(texts.indexOf('real message') !== -1, 'the real candidate must be decrypted and shown (got ' + JSON.stringify(texts) + ')'); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From 69fd262efdad4ed3ca8546d4a40862d715e6f870 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 10:31:49 +0000 Subject: [PATCH 10/23] test(e2e): fix log-preservation and fast-exit nits in the decrypt-race E2E harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two nits in test-channels-client-state-152-decrypt-e2e.js's process lifecycle helpers: - A thrown error in main()'s try block (e.g. the server process never comes up) left `failed` at 0, since no step() had run yet to increment it. The `finally` block's `if (failed) ... else fs.rmSync(dir, ...)` then deleted the log directory anyway, and the FATAL message from the outer catch pointed at log files that no longer existed. Track a `fatal` flag alongside `failed` so logs are kept for either, then re-throw unchanged so the outer catch's behavior is otherwise identical. - waitFor() polled in fixed 200ms steps for up to the full timeout (20s) even when the callback's own check already knew the watched process had exited — the ingestor/server health checks threw on a dead process, but waitFor just caught that and kept retrying instead of stopping. waitFor now takes an optional `proc` and checks its exitCode itself before each poll, failing immediately instead of waiting out the timeout. No behavior change for the success path; these only affect error reporting and how fast a genuine startup failure is surfaced. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Vxw6Ez4BQJo7w99BQqxnji --- test-channels-client-state-152-decrypt-e2e.js | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/test-channels-client-state-152-decrypt-e2e.js b/test-channels-client-state-152-decrypt-e2e.js index b2a041e59..c60e3eb61 100644 --- a/test-channels-client-state-152-decrypt-e2e.js +++ b/test-channels-client-state-152-decrypt-e2e.js @@ -305,10 +305,16 @@ async function stopProcess(proc) { clearTimeout(t); } -async function waitFor(what, fn, timeoutMs) { +// Nit (#153 review round 3): when `proc` is given, stop polling the moment +// it exits instead of burning out the full timeout in 200ms retries against +// a process that is already dead. +async function waitFor(what, fn, timeoutMs, proc) { const deadline = Date.now() + (timeoutMs || TIMEOUT); let last; while (Date.now() < deadline) { + if (proc && proc.exitCode !== null) { + throw new Error(what + ': process "' + proc.label + '" exited early (code ' + proc.exitCode + '), see ' + proc.logFile); + } try { const v = await fn(); if (v) return v; } catch (e) { last = e; } await new Promise((r) => setTimeout(r, 200)); } @@ -384,9 +390,8 @@ async function migrateFixture(dbPath, dir) { const ingestor = startProcess('ingestor', INGESTOR_BIN, ['-config', ingestorConfig], dir); try { await waitFor('ingestor migration + MQTT subscription', () => { - if (ingestor.exitCode !== null) throw new Error('ingestor exited: see ' + ingestor.logFile); return logHas(ingestor, /MQTT \[e2e\] subscribed/); - }); + }, undefined, ingestor); } finally { await stopProcess(ingestor); broker.close(); @@ -636,30 +641,38 @@ async function main() { const port = await freePort(); const base = 'http://127.0.0.1:' + port; + // Nit (#153 review round 3): a thrown error here (e.g. the server never + // comes up) previously left `failed` at 0 — no step() had run yet — so the + // `finally` below deleted `dir` anyway, and the FATAL message from the + // outer catch then pointed at log files that no longer existed. Track a + // fatal separately so its logs are kept too, then re-throw unchanged. + let fatal = null; try { const server = startProcess('server', SERVER_BIN, ['-port', String(port), '-db', dbPath, '-public', PUBLIC_DIR, '-config-dir', serverDir], dir); await waitFor('server health', async () => { - if (server.exitCode !== null) throw new Error('server exited early: see ' + server.logFile); const res = await fetch(base + '/api/healthz'); return res.ok; - }); + }, undefined, server); await waitFor('server reports SJC and SFO regions', async () => { const res = await fetch(base + '/api/config/regions'); const regions = await res.json(); return 'SJC' in regions && 'SFO' in regions; - }); + }, undefined, server); console.log('\n=== #152 decrypt-race E2E against ' + base + ' ==='); await runViewport(browser, { name: 'desktop', width: 1280, height: 800 }, base, seed); await runViewport(browser, { name: 'mobile', width: 390, height: 844 }, base, seed); + } catch (e) { + fatal = e; } finally { await browser.close().catch(() => {}); for (const proc of Array.from(children)) await stopProcess(proc); - if (failed) console.error('logs kept in ' + dir); + if (failed || fatal) console.error('logs kept in ' + dir); else fs.rmSync(dir, { recursive: true, force: true }); } + if (fatal) throw fatal; console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); From 9cf1b792800aef187497fbc5465478e4c0814a30 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:02:53 +0000 Subject: [PATCH 11/23] =?UTF-8?q?fix(channels):=20R4-1=20=E2=80=94=20remov?= =?UTF-8?q?eKey=20clears=20every=20region-scoped=20decrypt=20cache=20entry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit N2 region-scoped the client-side decrypt cache ("|"), but removeKey() -> clearChannelCache() still only deleted the bare "" entry, so removing a PSK left its decrypted plaintext in localStorage under every region it had been viewed in. clearChannelCache(name) now deletes "" (the pre-N2 key) and every key starting with "|", and leaves channels whose name merely shares the prefix alone. The key format moves into ChannelDecrypt (channelCacheKey), so the writer in channels.js and the clearer can't drift apart again. Tests: two unit tests (raw blob with legacy/All/SJC/SFO,SJC entries plus a prefix-sharing channel; real decrypts in three regions removed through the channel-list handler) and a new decrypt-E2E step asserting that no localStorage value holds the channel's plaintext after Remove, at desktop and mobile. All red on 9706722d. Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- public/channel-decrypt.js | 22 +++++- public/channels.js | 6 +- test-channels-client-state-152-decrypt-e2e.js | 33 +++++++++ test-channels-client-state-152.js | 70 +++++++++++++++++++ 4 files changed, 126 insertions(+), 5 deletions(-) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index 447450682..14516c6c0 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -354,11 +354,28 @@ window.ChannelDecrypt = (function () { try { localStorage.setItem(LABELS_KEY, JSON.stringify(labels)); } catch (e) { /* quota */ } } - /** Remove cached messages for a specific channel (by name or hash). */ + // N2 (#152 follow-up): decrypted messages are cached per channel AND per + // region selection, as "|" ("|" for all + // regions). Region order doesn't change which observers are included, so + // it must not change the key either. + var CACHE_REGION_SEP = '|'; + + function channelCacheKey(channelName, regionParam) { + var regions = regionParam ? String(regionParam).split(',').filter(Boolean).sort().join(',') : ''; + return channelName + CACHE_REGION_SEP + regions; + } + + /** + * Remove every cached message set of a channel (by name or hash): each + * region-scoped "|" entry plus a pre-N2 "" one. + */ function clearChannelCache(channelKey) { try { var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); - delete cache[channelKey]; + var prefix = channelKey + CACHE_REGION_SEP; + Object.keys(cache).forEach(function (k) { + if (k === channelKey || k.indexOf(prefix) === 0) delete cache[k]; + }); localStorage.setItem(CACHE_KEY, JSON.stringify(cache)); } catch (e) { /* quota */ } } @@ -438,6 +455,7 @@ window.ChannelDecrypt = (function () { saveLabel: saveLabel, getLabel: getLabel, getLabels: getLabels, + channelCacheKey: channelCacheKey, clearChannelCache: clearChannelCache, cacheMessages: cacheMessages, getCachedMessages: getCachedMessages, diff --git a/public/channels.js b/public/channels.js index 720cc47c0..59209a667 100644 --- a/public/channels.js +++ b/public/channels.js @@ -815,10 +815,10 @@ // decrypted message set depends on which observers the current region // filter includes, so a cache entry primed under one region (e.g. "All" // or SJC) must never answer a fetch for a different region (e.g. OAK or - // MRY) — region order doesn't matter, so sort for a stable key. + // MRY). ChannelDecrypt owns the key format so removeKey() can clear + // every region of a channel. var rp = RegionFilter.getRegionParam(); - var regionKey = rp ? rp.split(',').filter(Boolean).sort().join(',') : ''; - var cacheKey = (channelName || String(channelHashByte)) + '|' + regionKey; + var cacheKey = ChannelDecrypt.channelCacheKey(channelName || String(channelHashByte), rp); // A stale request (superseded by a newer one, or no longer the current // selection/region by the time a write below would happen) must not // persist its possibly-incomplete evidence into the cache. diff --git a/test-channels-client-state-152-decrypt-e2e.js b/test-channels-client-state-152-decrypt-e2e.js index c60e3eb61..508703a39 100644 --- a/test-channels-client-state-152-decrypt-e2e.js +++ b/test-channels-client-state-152-decrypt-e2e.js @@ -560,6 +560,39 @@ async function runViewport(browser, vp, base, seed) { ok(s.urlHash === '#/channels/' + encodeURIComponent(seed.hash), 'URL must stay on the PSK channel, got ' + s.urlHash); }); + // R4-1 (#153 review round 4, P2): the decrypt cache is region-scoped + // ("|") since N2, but Remove still only cleared the bare + // "" entry, leaving the decrypted plaintext in localStorage. + await step(vp.name + ': Remove leaves no decrypted plaintext for the channel in localStorage', async () => { + const before = await page.evaluate(() => JSON.stringify(Object.assign({}, localStorage))); + ok(before.indexOf(seed.sfo.text) !== -1, + 'precondition: the SFO decrypt must have cached its plaintext before Remove (localStorage keys: ' + + JSON.stringify(await page.evaluate(() => Object.keys(localStorage))) + ')'); + const btn = page.locator('#chList [data-remove-channel="' + seed.hash + '"]'); + if (await btn.count()) { + page.once('dialog', (d) => d.accept()); + await btn.click({ timeout: 2000 }); + } else { + // The mobile channel list (.ch-row) has no Remove button yet — a known + // gap outside this PR's scope. Call the handler that button invokes + // after its confirm(), so mobile still covers what Remove clears. + ok(vp.width < 640, 'desktop must render a Remove button for the PSK row'); + await page.evaluate((hash) => window._channelsRemoveKeyHandlerForTest(hash), seed.hash); + } + await page.waitForFunction((hash) => !document.querySelector('#chList [data-hash="' + hash + '"]'), + seed.hash, { timeout: 8000 }); + const after = await page.evaluate(() => Object.assign({}, localStorage)); + for (const k of Object.keys(after)) { + for (const text of [seed.sjc.text, seed.sfo.text]) { + ok(String(after[k]).indexOf(text) === -1, + 'localStorage["' + k + '"] still holds plaintext "' + text + '" after Remove: ' + String(after[k]).slice(0, 200)); + } + } + const cacheKeys = Object.keys(JSON.parse(after.corescope_channel_cache || '{}')); + ok(!cacheKeys.some((k) => k === seed.channelName || k.indexOf(seed.channelName + '|') === 0), + 'no decrypt-cache entry for ' + seed.channelName + ' may survive Remove, got ' + JSON.stringify(cacheKeys)); + }); + await step(vp.name + ': no uncaught page errors', async () => { // "L is not defined" is Leaflet (public/index.html loads it from // unpkg.com) failing to load because this test's sandbox has no diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index d40bf7f2e..cdd89d787 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -260,6 +260,21 @@ function makeHarness(opts) { return h; } +// The decrypt cache exactly as channel-decrypt.js persisted it. +const DECRYPT_CACHE_KEY = 'corescope_channel_cache'; +function rawDecryptCache(h) { + return h.ctx.localStorage.getItem(DECRYPT_CACHE_KEY) || ''; +} +function decryptCacheKeys(h) { + return Object.keys(JSON.parse(rawDecryptCache(h) || '{}')); +} +function cacheEntry(text, ts) { + return { + messages: [{ sender: 'Alice', text, timestamp: ts || '2026-01-01T00:00:00Z', packetHash: 'p-' + text }], + lastTimestamp: ts || '2026-01-01T00:00:00Z', count: 1, ts: 1, + }; +} + function serverChannel(hash, extra) { return Object.assign({ hash, @@ -916,6 +931,61 @@ async function test(name, fn) { assert.ok(texts.indexOf('real message') !== -1, 'the real candidate must be decrypted and shown (got ' + JSON.stringify(texts) + ')'); }); + // ── R4-1 (#153 review round 4, P2): N2 made the decrypt cache keys + // region-scoped ("|"), but removeKey() still only + // deleted cache[""], so removing a key left the channel's + // decrypted plaintext in localStorage under every region it was viewed in. + await test('R4-1: removeKey drops every region-scoped cache entry and the pre-N2 one, keeping a prefix-sharing channel', async () => { + const h = makeHarness(); + const NAME = 'psk:r4rm'; + const OTHER = 'psk:r4rmx'; // "psk:r4rm" is a prefix of this name + h.storeKey(NAME, 'd4'.repeat(16), 'R4 Remove'); + h.storeKey(OTHER, 'e5'.repeat(16), 'R4 Other'); + const blob = {}; + blob[NAME] = cacheEntry('legacy secret'); + blob[NAME + '|'] = cacheEntry('all-regions secret'); + blob[NAME + '|SJC'] = cacheEntry('sjc secret'); + blob[NAME + '|SFO,SJC'] = cacheEntry('sfo-sjc secret'); + blob[OTHER + '|SJC'] = cacheEntry('other channel text'); + h.ctx.localStorage.setItem(DECRYPT_CACHE_KEY, JSON.stringify(blob)); + + h.removeKey(NAME); + + const keys = decryptCacheKeys(h); + assert.deepStrictEqual(keys.filter((k) => k === NAME || k.indexOf(NAME + '|') === 0), [], + 'no cache entry for the removed channel may survive (got ' + JSON.stringify(keys) + ')'); + assert.ok(!/secret/.test(rawDecryptCache(h)), 'no plaintext of the removed channel may stay in localStorage'); + assert.ok(keys.indexOf(OTHER + '|SJC') !== -1, 'a channel whose name merely shares the prefix must keep its cache (got ' + JSON.stringify(keys) + ')'); + assert.strictEqual(h.w.ChannelDecrypt.getCache(OTHER + '|SJC').messages[0].text, 'other channel text'); + }); + + await test('R4-1: removing a PSK through the channel list clears the plaintext cached by real decrypts in several regions', async () => { + const h = makeHarness(); + const KEY = 'f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6f6'; + const NAME = 'psk:r4ui'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'R4 UI'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + + const msg = encryptChannelMessage(KEY, 'Alice', 'plaintext-r4ui'); + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-r4ui', '2026-01-01T00:00:00Z', msg)] }); + for (const region of ['SJC', 'SFO,SJC', '']) { + h.regionParam = region; + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + } + assert.strictEqual(decryptCacheKeys(h).filter((k) => k.indexOf(NAME + '|') === 0).length, 3, + 'precondition: three region-scoped entries cached (got ' + JSON.stringify(decryptCacheKeys(h)) + ')'); + assert.ok(/plaintext-r4ui/.test(rawDecryptCache(h)), 'precondition: plaintext is cached'); + + h.w._channelsRemoveKeyHandlerForTest(HASH); + + assert.deepStrictEqual(decryptCacheKeys(h).filter((k) => k.indexOf(NAME) === 0), [], + 'Remove must clear every cached region of the channel (got ' + JSON.stringify(decryptCacheKeys(h)) + ')'); + assert.ok(!/plaintext-r4ui/.test(rawDecryptCache(h)), 'no decrypted plaintext may remain in localStorage after Remove'); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From 08702a10603aaecd78923db7ef58cfc0de94e8ac Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:05:54 +0000 Subject: [PATCH 12/23] =?UTF-8?q?fix(channels):=20R4-2=20=E2=80=94=20bound?= =?UTF-8?q?=20the=20decrypt=20cache=20in=20characters,=20not=20just=20entr?= =?UTF-8?q?ies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client-side decrypt cache was capped at 50 entries only. One entry can hold ~365 KiB (1000 messages) against a ~5.2M-character localStorage quota shared with the stored keys and labels, so a full cache made storeKey() and saveLabel() fail silently. - The whole cache blob is kept under 1.5M characters; the least recently used entries (last write, or last read this page load) go first. Read stamps are folded in on the next write, so a read never rewrites the blob. - A QuotaExceededError from the cache write evicts the oldest entry and retries; an entry that can never fit drops the blob instead. setItem() stores the whole new blob or keeps the old one, so it is never half-written. - Keys and labels go through setItemMakingRoom(): on a quota failure they evict decrypt cache until the write fits. - Pre-N2 entries (keyed by channel name alone, no "|") are dropped once, marked by corescope_channel_cache_v=2. Each entry is serialised once and joined, so the budget costs no extra stringify. Measured in Node with 12 channels x 1000 messages written in turn: setCache 23.0 -> 14.1 ms, getCache 13.0 -> 5.9 ms on average, since the blob no longer grows past the budget. Tests (all red on 9706722d): budget + eviction order, read keeps an entry alive, quota evict-and-retry with a never-fitting entry, storeKey/labels under a full quota, one-time legacy migration. Mutants killed: no eviction, reversed eviction, no quota retry, reads not counted, keys not making room, no migration, migration on every load. Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- public/channel-decrypt.js | 224 ++++++++++++++++++++++-------- test-channels-client-state-152.js | 106 ++++++++++++++ 2 files changed, 274 insertions(+), 56 deletions(-) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index 14516c6c0..864b867a4 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -295,7 +295,7 @@ window.ChannelDecrypt = (function () { function saveKey(channelName, keyHex, label) { var keys = getKeys(); keys[channelName] = keyHex; - try { localStorage.setItem(STORAGE_KEY, JSON.stringify(keys)); } catch (e) { /* quota */ } + setItemMakingRoom(STORAGE_KEY, JSON.stringify(keys)); _keyMapCache = null; // invalidate live-decrypt index if (typeof label === 'string' && label.trim()) { saveLabel(channelName, label.trim()); @@ -318,14 +318,14 @@ window.ChannelDecrypt = (function () { function removeKey(channelName) { var keys = getKeys(); delete keys[channelName]; - try { localStorage.setItem(STORAGE_KEY, JSON.stringify(keys)); } catch (e) { /* quota */ } + setItemMakingRoom(STORAGE_KEY, JSON.stringify(keys)); _keyMapCache = null; // invalidate live-decrypt index // Also clear cached messages and any label for this channel (#1020) clearChannelCache(channelName); var labels = getLabels(); if (labels[channelName]) { delete labels[channelName]; - try { localStorage.setItem(LABELS_KEY, JSON.stringify(labels)); } catch (e) { /* quota */ } + setItemMakingRoom(LABELS_KEY, JSON.stringify(labels)); } } @@ -351,7 +351,7 @@ window.ChannelDecrypt = (function () { } else { delete labels[channelName]; } - try { localStorage.setItem(LABELS_KEY, JSON.stringify(labels)); } catch (e) { /* quota */ } + setItemMakingRoom(LABELS_KEY, JSON.stringify(labels)); } // N2 (#152 follow-up): decrypted messages are cached per channel AND per @@ -365,75 +365,187 @@ window.ChannelDecrypt = (function () { return channelName + CACHE_REGION_SEP + regions; } + // ---- Message cache (localStorage) ---- + // + // One JSON blob under CACHE_KEY: { "|": entry }. It is + // only a cache, so it always gives way: it is kept under a fixed + // character budget, a quota failure evicts and retries, and keys and + // labels evict it before they would fail to save. + + // Cache with lastTimestamp and count (used by channels.js via getCache/setCache) + var MAX_CACHED_MESSAGES = 1000; + // N2 (#152 follow-up): keys are region-scoped, so one channel can occupy + // several entries. Cap the number of distinct entries so visiting many + // region combinations over time can't grow the blob unboundedly. + var MAX_CACHE_KEYS = 50; + // R4-2 (#153 review round 4): one entry can reach ~365 KiB (1000 + // messages) and localStorage holds ~5.2M characters per origin, shared + // with the keys and labels. Keep the whole blob under this many + // characters, least recently used entries going first. + var CACHE_BUDGET_CHARS = 1500000; + // Set once the pre-N2 entries (keyed by channel name alone) are dropped. + var CACHE_VERSION_KEY = 'corescope_channel_cache_v'; + var CACHE_VERSION = '2'; + + var _cacheMigrated = false; + var _cacheUseSeq = 0; + // Last-read stamps of this page load, folded into the entries' `at` on + // the next write so a read doesn't cost a rewrite of the whole blob. + var _cacheReadAt = {}; + + // Strictly increasing, so entries written or read in the same + // millisecond still have a defined least-recently-used order. + function nextCacheUse() { + _cacheUseSeq = Math.max(Date.now(), _cacheUseSeq + 1); + return _cacheUseSeq; + } + + function cacheLastUse(cache, key) { + var entry = cache[key] || {}; + return Math.max(entry.at || entry.ts || 0, _cacheReadAt[key] || 0); + } + + function readCacheBlob() { + try { + var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); + return (cache && typeof cache === 'object') ? cache : {}; + } catch (e) { return {}; } + } + + // Drop the pre-N2 entries once: they are keyed by channel name alone, so + // nothing reads them any more, and removeKey() of a later version would + // have no reason to look for them. + function ensureCacheMigrated() { + if (_cacheMigrated) return; + _cacheMigrated = true; + try { + if (localStorage.getItem(CACHE_VERSION_KEY) === CACHE_VERSION) return; + } catch (e) { return; } + var cache = readCacheBlob(); + var legacy = Object.keys(cache).filter(function (k) { return k.indexOf(CACHE_REGION_SEP) === -1; }); + if (legacy.length) { + legacy.forEach(function (k) { delete cache[k]; }); + writeCacheBlob(cache); + } + try { localStorage.setItem(CACHE_VERSION_KEY, CACHE_VERSION); } catch (e) { /* retried next load */ } + } + + /** + * Persist `cache`, evicting least recently used entries until it is within + * MAX_CACHE_KEYS and CACHE_BUDGET_CHARS and localStorage accepts it. Each + * entry is serialised once; setItem() either stores the whole new blob or + * leaves the old one, so the blob is never half-written. Returns whether + * the blob was written. + */ + function writeCacheBlob(cache) { + var parts = {}; + var size = 2; // "{}" + var keys = Object.keys(cache); + keys.forEach(function (k) { + if (cache[k] && _cacheReadAt[k]) cache[k].at = cacheLastUse(cache, k); + parts[k] = JSON.stringify(k) + ':' + JSON.stringify(cache[k]); + size += parts[k].length + 1; // + separating comma + }); + _cacheReadAt = {}; // folded into the entries above + keys.sort(function (a, b) { return cacheLastUse(cache, a) - cacheLastUse(cache, b); }); + while (keys.length > MAX_CACHE_KEYS || (keys.length && size - 1 > CACHE_BUDGET_CHARS)) { + size -= parts[keys.shift()].length + 1; + } + for (;;) { + try { + localStorage.setItem(CACHE_KEY, '{' + keys.map(function (k) { return parts[k]; }).join(',') + '}'); + return true; + } catch (e) { + // QuotaExceededError: localStorage is shared with the keys and + // labels, so give up room until the blob fits. + if (!keys.length) break; + keys.shift(); + } + } + try { localStorage.removeItem(CACHE_KEY); } catch (e) { /* nothing cached */ } + return false; + } + + // Drop the least recently used cache entry (or the empty blob itself). + // Returns false once there is no cache left to give up. + function evictOldestCacheEntry() { + var raw; + try { raw = localStorage.getItem(CACHE_KEY); } catch (e) { return false; } + if (raw === null) return false; + var cache = readCacheBlob(); + var keys = Object.keys(cache); + if (!keys.length) { + try { localStorage.removeItem(CACHE_KEY); } catch (e) { return false; } + return true; + } + keys.sort(function (a, b) { return cacheLastUse(cache, a) - cacheLastUse(cache, b); }); + delete cache[keys[0]]; + writeCacheBlob(cache); + return true; + } + + // setItem() for the keys and labels: when the quota is hit, evict decrypt + // cache until the write fits, so the cache can never cost the user a key. + function setItemMakingRoom(storageKey, value) { + for (;;) { + try { + localStorage.setItem(storageKey, value); + return true; + } catch (e) { + if (!evictOldestCacheEntry()) return false; + } + } + } + /** * Remove every cached message set of a channel (by name or hash): each * region-scoped "|" entry plus a pre-N2 "" one. */ function clearChannelCache(channelKey) { - try { - var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); - var prefix = channelKey + CACHE_REGION_SEP; - Object.keys(cache).forEach(function (k) { - if (k === channelKey || k.indexOf(prefix) === 0) delete cache[k]; - }); - localStorage.setItem(CACHE_KEY, JSON.stringify(cache)); - } catch (e) { /* quota */ } + ensureCacheMigrated(); + var cache = readCacheBlob(); + var prefix = channelKey + CACHE_REGION_SEP; + Object.keys(cache).forEach(function (k) { + if (k === channelKey || k.indexOf(prefix) === 0) delete cache[k]; + }); + writeCacheBlob(cache); } - // ---- Message cache (localStorage) ---- - function cacheMessages(channelHash, messages) { - try { - var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); - cache[channelHash] = { messages: messages, ts: Date.now() }; - localStorage.setItem(CACHE_KEY, JSON.stringify(cache)); - } catch (e) { /* quota */ } + ensureCacheMigrated(); + var cache = readCacheBlob(); + cache[channelHash] = { messages: messages, ts: Date.now(), at: nextCacheUse() }; + writeCacheBlob(cache); } function getCachedMessages(channelHash) { - try { - var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); - var entry = cache[channelHash]; - return entry ? entry.messages : null; - } catch (e) { return null; } + var entry = getCache(channelHash); + return entry ? entry.messages : null; } - // Cache with lastTimestamp and count (used by channels.js via getCache/setCache) - var MAX_CACHED_MESSAGES = 1000; - // N2 (#152 follow-up): keys are now region-scoped ("|"), - // so one channel can occupy several entries. Cap the number of distinct - // entries so visiting many region combinations over time can't grow this - // localStorage blob unboundedly; evict the least-recently-written ones. - var MAX_CACHE_KEYS = 50; - function setCache(key, messages, lastTimestamp, totalCount) { - try { - // Enforce cache size limit: only keep most recent MAX_CACHED_MESSAGES - var toStore = messages; - if (messages.length > MAX_CACHED_MESSAGES) { - toStore = messages.slice(messages.length - MAX_CACHED_MESSAGES); - } - var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); - cache[key] = { - messages: toStore, - lastTimestamp: lastTimestamp, - count: totalCount || toStore.length, - ts: Date.now() - }; - var keys = Object.keys(cache); - if (keys.length > MAX_CACHE_KEYS) { - keys.sort(function (a, b) { return (cache[a].ts || 0) - (cache[b].ts || 0); }); - for (var i = 0; i < keys.length - MAX_CACHE_KEYS; i++) delete cache[keys[i]]; - } - localStorage.setItem(CACHE_KEY, JSON.stringify(cache)); - } catch (e) { /* quota */ } + ensureCacheMigrated(); + // Enforce cache size limit: only keep most recent MAX_CACHED_MESSAGES + var toStore = messages; + if (messages.length > MAX_CACHED_MESSAGES) { + toStore = messages.slice(messages.length - MAX_CACHED_MESSAGES); + } + var cache = readCacheBlob(); + cache[key] = { + messages: toStore, + lastTimestamp: lastTimestamp, + count: totalCount || toStore.length, + ts: Date.now(), + at: nextCacheUse() + }; + writeCacheBlob(cache); } function getCache(key) { - try { - var cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); - return cache[key] || null; - } catch (e) { return null; } + ensureCacheMigrated(); + var entry = readCacheBlob()[key] || null; + if (entry) _cacheReadAt[key] = nextCacheUse(); + return entry; } return { diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index cdd89d787..c0692f294 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -275,6 +275,45 @@ function cacheEntry(text, ts) { }; } +// channel-decrypt.js alone, over a localStorage that enforces a quota the +// way browsers do: a setItem() that would push the total stored characters +// past `quota` throws QuotaExceededError and leaves the old value in place. +function makeDecryptSandbox(opts) { + opts = opts || {}; + const sb = { storage: Object.assign({}, opts.storage || {}), quota: opts.quota || Infinity }; + sb.used = (exceptKey) => Object.keys(sb.storage).reduce((n, k) => n + (k === exceptKey ? 0 : k.length + sb.storage[k].length), 0); + const ctx = { + localStorage: { + getItem: (k) => Object.prototype.hasOwnProperty.call(sb.storage, k) ? sb.storage[k] : null, + setItem: (k, v) => { + v = String(v); + if (sb.used(k) + k.length + v.length > sb.quota) { + const e = new Error('The quota has been exceeded.'); + e.name = 'QuotaExceededError'; + throw e; + } + sb.storage[k] = v; + }, + removeItem: (k) => { delete sb.storage[k]; }, + }, + console, Date, JSON, Math, String, Number, Object, Array, RegExp, Error, Promise, parseInt, + crypto: webcrypto, TextEncoder, TextDecoder, Uint8Array, + }; + ctx.window = ctx; + ctx.self = ctx; + vm.createContext(ctx); + vm.runInContext(fs.readFileSync(path.join(__dirname, 'public/channel-decrypt.js'), 'utf8'), ctx, { filename: 'public/channel-decrypt.js' }); + sb.CD = ctx.ChannelDecrypt; + sb.raw = () => sb.storage[DECRYPT_CACHE_KEY] || ''; + sb.keys = () => Object.keys(JSON.parse(sb.raw() || '{}')); + return sb; +} +// One cached message whose text is `chars` long, so a cache entry +// serialises to roughly that many characters. +function bigMessages(chars, tag) { + return [{ sender: 'S', text: tag + ':' + 'x'.repeat(chars), timestamp: '2026-01-01T00:00:00Z', packetHash: 'h-' + tag }]; +} + function serverChannel(hash, extra) { return Object.assign({ hash, @@ -986,6 +1025,73 @@ async function test(name, fn) { assert.ok(!/plaintext-r4ui/.test(rawDecryptCache(h)), 'no decrypted plaintext may remain in localStorage after Remove'); }); + // ── R4-2 (#153 review round 4, P3): the decrypt cache was only capped by + // entry count. One entry can be ~365 KiB (1000 messages) against a + // ~5.2M-character localStorage quota, so a full cache made storeKey() and + // the labels fail silently. It is now kept under a fixed character budget, + // evicting the least recently used entries. + await test('R4-2: the decrypt cache stays under its 1.5M-character budget, evicting least recently written entries first', async () => { + const sb = makeDecryptSandbox(); + for (let i = 1; i <= 5; i++) sb.CD.setCache('budget|' + i, bigMessages(400000, 'b' + i), 'ts', 1); + assert.ok(sb.raw().length <= 1500000, 'cache blob must stay within 1.5M characters (got ' + sb.raw().length + ')'); + assert.deepStrictEqual(sb.keys().sort(), ['budget|3', 'budget|4', 'budget|5'], + 'the oldest entries must be evicted first, the newest kept (got ' + JSON.stringify(sb.keys()) + ')'); + assert.ok(/b5:/.test(sb.CD.getCache('budget|5').messages[0].text), 'the entry just written must be readable'); + }); + + await test('R4-2: a recently read entry outlives an older unread one', async () => { + const sb = makeDecryptSandbox(); + for (let i = 1; i <= 3; i++) sb.CD.setCache('lru|' + i, bigMessages(400000, 'l' + i), 'ts', 1); + assert.ok(sb.CD.getCache('lru|1'), 'precondition: lru|1 cached'); + sb.CD.setCache('lru|4', bigMessages(400000, 'l4'), 'ts', 1); + assert.deepStrictEqual(sb.keys().sort(), ['lru|1', 'lru|3', 'lru|4'], + 'lru|2 (oldest use) must go, not the just-read lru|1 (got ' + JSON.stringify(sb.keys()) + ')'); + }); + + await test('R4-2: a QuotaExceededError evicts and retries, and an entry that can never fit leaves a valid blob', async () => { + const sb = makeDecryptSandbox({ quota: 1000000 }); + for (let i = 1; i <= 3; i++) sb.CD.setCache('quota|' + i, bigMessages(300000, 'q' + i), 'ts', 1); + assert.deepStrictEqual(sb.keys().sort(), ['quota|1', 'quota|2', 'quota|3'], 'precondition: 900K fits the quota'); + sb.CD.setCache('quota|4', bigMessages(300000, 'q4'), 'ts', 1); + assert.deepStrictEqual(sb.keys().sort(), ['quota|2', 'quota|3', 'quota|4'], + 'a quota failure must evict the oldest entry and retry (got ' + JSON.stringify(sb.keys()) + ')'); + + sb.CD.setCache('quota|huge', bigMessages(1100000, 'qh'), 'ts', 1); + const raw = sb.storage[DECRYPT_CACHE_KEY]; + if (raw !== undefined) assert.doesNotThrow(() => JSON.parse(raw), 'the cache blob must never be left half-written'); + assert.strictEqual(sb.CD.getCache('quota|huge'), null, 'an entry larger than the quota is not cached'); + }); + + await test('R4-2: storeKey and labels still save when the decrypt cache has filled the quota', async () => { + const sb = makeDecryptSandbox(); + for (let i = 1; i <= 3; i++) sb.CD.setCache('full|' + i, bigMessages(300000, 'f' + i), 'ts', 1); + sb.quota = sb.used() + 10; // localStorage is now full + const KEY = '0123456789abcdef0123456789abcdef'; + sb.CD.storeKey('psk:quotakey', KEY, 'Quota Label'); + assert.strictEqual(sb.CD.getKeys()['psk:quotakey'], KEY, 'the key must be saved, evicting decrypt cache to make room'); + assert.strictEqual(sb.CD.getLabel('psk:quotakey'), 'Quota Label', 'the label must be saved too'); + sb.CD.saveLabel('psk:quotakey', 'Renamed ' + 'L'.repeat(200)); + assert.strictEqual(sb.CD.getLabel('psk:quotakey'), 'Renamed ' + 'L'.repeat(200), 'a later label change must also make room'); + assert.ok(sb.keys().length < 3, 'room must come from the decrypt cache (got ' + JSON.stringify(sb.keys()) + ')'); + }); + + await test('R4-2: pre-N2 cache entries (no "|" region part) are dropped once, on first use after upgrade', async () => { + const legacy = {}; + legacy['#oldchan'] = cacheEntry('pre-N2 plaintext'); + legacy['psk:legacy'] = cacheEntry('pre-N2 psk plaintext'); + legacy['psk:legacy|SJC'] = cacheEntry('region-scoped text'); + const sb = makeDecryptSandbox({ storage: { [DECRYPT_CACHE_KEY]: JSON.stringify(legacy) } }); + assert.strictEqual(sb.CD.getCache('psk:legacy|SJC').messages[0].text, 'region-scoped text', 'region-scoped entries survive'); + assert.deepStrictEqual(sb.keys(), ['psk:legacy|SJC'], 'pre-N2 entries must be dropped (got ' + JSON.stringify(sb.keys()) + ')'); + assert.ok(!/pre-N2/.test(sb.raw()), 'no pre-N2 plaintext may remain'); + + // Only once: an entry written after the migration (the old public + // setCache/cacheMessages API still accepts any key) survives a reload. + sb.CD.setCache('written-later', [{ text: 'kept' }], 'ts', 1); + const reloaded = makeDecryptSandbox({ storage: sb.storage }); + assert.ok(reloaded.CD.getCache('written-later'), 'the migration must not run again on the next page load'); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From 8134d170965ed30df86cba5b6d1d57a922013cfe Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:08:03 +0000 Subject: [PATCH 13/23] =?UTF-8?q?fix(channels):=20R4-3=20=E2=80=94=20make?= =?UTF-8?q?=20the=20N1=20pending-load=20flag=20per=20request?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit N1's "a decrypt is pending" flag was one shared boolean, set only inside decryptAndRender(). That left three races: - S1: decrypt A, a region switch starts decrypt B, A finishes and clears the flag while B still runs; a remap then saw no pending load and left the pane on "Decrypting messages…". - S2: a remap while selectChannel() still awaited computeChannelHash() happened before the flag was set at all. - S3: a superseded selectChannel() resuming after computeChannelHash() still painted "Decrypting messages…" over the newer request's view (e.g. a REST refresh after a region switch revealed a same-named server channel). selectChannel() now sets messageLoadPending to its own request id before its first await and only the owning request clears it (the body moves into loadSelectedChannel(request, ...)). reconcileSelectionAfterChannel- Refresh() restarts loading when the flag equals the current messageRequestId, and decryptAndRender() returns early for a stale request instead of writing "Decrypting messages…". Tests S1, S2 and S3 are red on 9706722d. Mutants killed: a global flag cleared by any request (S1), the flag set after the first await (S2), no stale check before "Decrypting…" (S3), no restart on remap (N1, S1, S2). Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- public/channels.js | 120 ++++++++++++++++-------------- test-channels-client-state-152.js | 109 +++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 56 deletions(-) diff --git a/public/channels.js b/public/channels.js index 59209a667..42d123e85 100644 --- a/public/channels.js +++ b/public/channels.js @@ -236,14 +236,15 @@ let observerIataById = {}; let observerIataByName = {}; let messageRequestId = 0; - // N1 (#152 follow-up): true while a decryptAndRender() fetch for the - // current selection is in flight (set/cleared around that one call). - // Lets reconcileSelectionAfterChannelRefresh() know, when it remaps a - // user:* selection mid-decrypt, that it must restart loading for the - // remapped hash — otherwise the in-flight request's own staleness check - // discards its result once selectedHash changes, and nothing else ever - // re-fetches, leaving the pane stuck on "Decrypting messages…". - let messageLoadPending = false; + // N1 (#152 follow-up): the id of the selectChannel() request whose load is + // still running (0 when none). Lets reconcileSelectionAfterChannelRefresh() + // know, when it remaps a user:* selection mid-load, that it must restart + // loading for the remapped hash — otherwise the in-flight request's own + // staleness check discards its result once selectedHash changes, and + // nothing else ever re-fetches, leaving the pane stuck on "Decrypting + // messages…". R4-3: an id rather than a boolean, so a superseded request + // finishing can't clear the flag of the request that replaced it. + let messageLoadPending = 0; var _nodeCacheTTL = 5 * 60 * 1000; // 5 minutes function getSelectedRegionsSnapshot() { @@ -332,11 +333,11 @@ // stuck on "Decrypting messages…" forever. Restart loading for the // remapped hash in that case. A quiet remap (no decrypt in flight) // still leaves messages untouched, as before. - var hadPendingDecrypt = messageLoadPending; + var hadPendingLoad = messageLoadPending !== 0 && messageLoadPending === messageRequestId; selectedHash = remapped.hash; history.replaceState(null, '', `#/channels/${encodeURIComponent(selectedHash)}`); renderChannelList(); - if (hadPendingDecrypt) selectChannel(selectedHash); + if (hadPendingLoad) selectChannel(selectedHash); return false; } } @@ -2423,8 +2424,21 @@ } async function selectChannel(hash, decryptOpts) { - const rp = RegionFilter.getRegionParam() || ''; - const request = beginMessageRequest(hash, rp); + const request = beginMessageRequest(hash, RegionFilter.getRegionParam() || ''); + // R4-3: mark this request's load as pending before its first await + // (computeChannelHash), so a remap at any point of it restarts loading. + // Only the request that owns the flag may clear it. + messageLoadPending = request.id; + try { + return await loadSelectedChannel(request, decryptOpts); + } finally { + if (messageLoadPending === request.id) messageLoadPending = 0; + } + } + + async function loadSelectedChannel(request, decryptOpts) { + const hash = request.hash; + const rp = request.regionParam; // #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 @@ -2450,52 +2464,46 @@ // Shared helper: fetch, decrypt, and render messages for a channel key (M5: cache-first) async function decryptAndRender(keyHex, channelHashByte, channelName) { + // R4-3: callers reach this after awaiting computeChannelHash; a + // superseded request must not paint over the current one's view. + if (isStaleMessageRequest(request)) return { stale: true }; msgEl.innerHTML = '
Decrypting messages…
'; - // N1 (#152 follow-up): mark a decrypt as pending for the current - // selection so reconcileSelectionAfterChannelRefresh() can tell, when - // it remaps a selection mid-decrypt, that it must restart loading for - // the remapped hash instead of leaving the pane stuck on this message. - messageLoadPending = true; - try { - var result = await fetchAndDecryptChannel(keyHex, channelHashByte, channelName, { - onCacheHit: function (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(); - scrollToBottom(); - } - }, - // N2 (#152 follow-up): a superseded request must not persist its - // evidence into the (now region-scoped) decrypt cache. - isStale: function () { return isStaleMessageRequest(request); } - }); - if (isStaleMessageRequest(request)) return { stale: true }; - if (result.wrongKey) { - msgEl.innerHTML = '
Key does not match — no messages could be decrypted
'; - return { wrongKey: true, messageCount: 0 }; - } - if (result.error) { - msgEl.innerHTML = '
' + escapeHtml(result.error) + '
'; - return { error: result.error, messageCount: 0 }; - } - // #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 { - header.querySelector('.ch-header-text').textContent = `${name} — ${messages.length} messages (decrypted)`; - renderMessages(); - scrollToBottom(); - } - return { messageCount: messages.length }; - } finally { - messageLoadPending = false; + var result = await fetchAndDecryptChannel(keyHex, channelHashByte, channelName, { + onCacheHit: function (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(); + scrollToBottom(); + } + }, + // N2 (#152 follow-up): a superseded request must not persist its + // evidence into the (now region-scoped) decrypt cache. + isStale: function () { return isStaleMessageRequest(request); } + }); + if (isStaleMessageRequest(request)) return { stale: true }; + if (result.wrongKey) { + msgEl.innerHTML = '
Key does not match — no messages could be decrypted
'; + return { wrongKey: true, messageCount: 0 }; + } + if (result.error) { + msgEl.innerHTML = '
' + escapeHtml(result.error) + '
'; + return { error: result.error, messageCount: 0 }; + } + // #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 { + header.querySelector('.ch-header-text').textContent = `${name} — ${messages.length} messages (decrypted)`; + renderMessages(); + scrollToBottom(); } + return { messageCount: messages.length }; } // Client-side decryption path (#725 M2) diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index c0692f294..ac125adbf 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -1092,6 +1092,115 @@ async function test(name, fn) { assert.ok(reloaded.CD.getCache('written-later'), 'the migration must not run again on the next page load'); }); + // ── R4-3 (#153 review round 4, P3): the N1 "a load is pending" flag was + // one shared boolean, set only inside decryptAndRender(). A superseded + // decrypt cleared it in its finally while a newer one was still running + // (S1), and a remap during selectChannel()'s first await (S2) saw no + // flag at all. A superseded selectChannel() also still painted + // "Decrypting messages…" over a newer request's view (S3). + await test('R4-3 S1: decrypt A, region switch starts decrypt B, A ends, then a remap — the pane must not hang', async () => { + const h = makeHarness(); + const KEY = 'a7a7a7a7a7a7a7a7a7a7a7a7a7a7a7a7'; + const NAME = '#r4s1'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'R4 S1'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + const msg = encryptChannelMessage(KEY, 'Alice', 's1 remapped message'); + + const decryptA = deferred(); + h.respondPackets = () => decryptA.promise; + const selectA = h.w._channelsSelectChannelForTest(HASH); + await flush(300); + + const decryptB = deferred(); + h.respondPackets = () => decryptB.promise; + h.regionParam = 'SJC'; + h.regionChange(); // re-runs selectChannel for the encrypted selection (F1) + await flush(300); + assert.strictEqual(h.packetsRequests.length, 2, 'precondition: decrypt B is in flight'); + + decryptA.resolve({ packets: [] }); // A is superseded and finishes first + await selectA; + await flush(300); + + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-s1', '2026-01-01T00:00:00Z', msg)] }); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel(NAME, { name: NAME })] }); + await h.w._channelsLoadChannelsForTest(true); + await flush(300); + decryptB.resolve({ packets: [] }); + await flush(300); + + assert.strictEqual(h.state().selectedHash, NAME, 'selection must have remapped'); + assert.ok(!/Decrypting/.test(h.elements.chMessages.innerHTML), 'pane must not hang on "Decrypting…" (got ' + h.elements.chMessages.innerHTML.slice(0, 120) + ')'); + const texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('s1 remapped message') !== -1, 'the remapped channel must load (got ' + JSON.stringify(texts) + ')'); + }); + + await test('R4-3 S2: a remap before computeChannelHash resolves restarts loading', async () => { + const h = makeHarness(); + const KEY = 'b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8'; + const NAME = '#r4s2'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'R4 S2'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + const msg = encryptChannelMessage(KEY, 'Bob', 's2 remapped message'); + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-s2', '2026-01-01T00:00:00Z', msg)] }); + + const hashReady = deferred(); + h.w.ChannelDecrypt.computeChannelHash = () => hashReady.promise; + const select = h.w._channelsSelectChannelForTest(HASH); + await flush(50); + assert.strictEqual(h.packetsRequests.length, 0, 'precondition: still waiting for computeChannelHash'); + + h.respondChannels = () => Promise.resolve({ channels: [serverChannel(NAME, { name: NAME })] }); + await h.w._channelsLoadChannelsForTest(true); + hashReady.resolve(msg.channelHash); + await select; + await flush(300); + + assert.strictEqual(h.state().selectedHash, NAME, 'selection must have remapped'); + assert.ok(!/Decrypting/.test(h.elements.chMessages.innerHTML), 'pane must not hang on "Decrypting…" (got ' + h.elements.chMessages.innerHTML.slice(0, 120) + ')'); + const texts = h.state().messages.map((m) => m.text); + assert.ok(texts.indexOf('s2 remapped message') !== -1, 'the remapped channel must load (got ' + JSON.stringify(texts) + ')'); + }); + + await test('R4-3 S3: a region switch that reveals a same-named channel mid-decrypt is not overwritten by a late "Decrypting…"', async () => { + const h = makeHarness(); + const KEY = 'c9c9c9c9c9c9c9c9c9c9c9c9c9c9c9c9'; + const NAME = '#r4s3'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'R4 S3'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + const realApi = h.ctx.api; + h.ctx.api = (p, o) => (p.indexOf('/channels/' + encodeURIComponent(NAME) + '/messages') === 0 + ? Promise.resolve({ messages: [{ id: 1, sender: 'Srv', text: 's3 server message', timestamp: '2026-01-01T00:00:00Z', packetHash: 'srv1' }] }) + : realApi(p, o)); + + const hashReady = deferred(); + h.w.ChannelDecrypt.computeChannelHash = () => hashReady.promise; + const select = h.w._channelsSelectChannelForTest(HASH); + await flush(50); + + // SJC lists #r4s3 as a server channel: reconcile remaps the selection, + // and the region handler refreshes it over REST. + h.regionParam = 'SJC'; + h.respondChannels = () => Promise.resolve({ channels: [serverChannel(NAME, { name: NAME })] }); + h.regionChange(); + await flush(300); + assert.strictEqual(h.state().selectedHash, NAME, 'precondition: selection remapped'); + assert.ok(h.state().messages.some((m) => m.text === 's3 server message'), 'precondition: REST refresh rendered the server channel'); + + hashReady.resolve(encryptChannelMessage(KEY, 'x', 'y').channelHash); + await select; + await flush(300); + assert.ok(!/Decrypting/.test(h.elements.chMessages.innerHTML), + 'a superseded selectChannel must not paint "Decrypting…" over the current view (got ' + h.elements.chMessages.innerHTML.slice(0, 120) + ')'); + assert.ok(h.state().messages.some((m) => m.text === 's3 server message'), 'the current view must keep its messages'); + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From 08e88b160a63e699ea5d0bed84cadbd3715748ec Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:10:09 +0000 Subject: [PATCH 14/23] test(channels): wait for Web Crypto in the R4-3 S1 test instead of fixed flushes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S1's "decrypt B is in flight" precondition counted /packets fetches after a fixed flush(300). Those fetches only start once computeChannelHash()'s Web Crypto digest resolves, which runs off the event loop and can outlast the flushes on a busy machine — seen failing once in about eight runs while mutation-testing R4-4. A settle(cond) helper now flushes until the condition holds or ~2s of real time pass, and S1 uses it for its in-flight preconditions and for the remapped load. Production code is unchanged. Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- test-channels-client-state-152.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index ac125adbf..8d56ce9ba 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -87,6 +87,15 @@ async function flush(n) { for (let i = 0; i < (n || 20); i++) await new Promise((r) => setImmediate(r)); } +// Flushes until cond() holds or ~2s of real time pass. For steps that wait +// on Web Crypto (computeChannelHash), whose digest runs off the event loop +// and can outlast a fixed number of flushes on a busy machine. +async function settle(cond, ms) { + const deadline = RealDate.now() + (ms || 2000); + while (!cond() && RealDate.now() < deadline) await new Promise((r) => setTimeout(r, 5)); + return cond(); +} + function makeHarness(opts) { opts = opts || {}; const storage = {}; @@ -1111,14 +1120,13 @@ async function test(name, fn) { const decryptA = deferred(); h.respondPackets = () => decryptA.promise; const selectA = h.w._channelsSelectChannelForTest(HASH); - await flush(300); + assert.ok(await settle(() => h.packetsRequests.length === 1), 'precondition: decrypt A is in flight'); const decryptB = deferred(); h.respondPackets = () => decryptB.promise; h.regionParam = 'SJC'; h.regionChange(); // re-runs selectChannel for the encrypted selection (F1) - await flush(300); - assert.strictEqual(h.packetsRequests.length, 2, 'precondition: decrypt B is in flight'); + assert.ok(await settle(() => h.packetsRequests.length === 2), 'precondition: decrypt B is in flight (got ' + h.packetsRequests.length + ' fetches)'); decryptA.resolve({ packets: [] }); // A is superseded and finishes first await selectA; @@ -1129,6 +1137,7 @@ async function test(name, fn) { await h.w._channelsLoadChannelsForTest(true); await flush(300); decryptB.resolve({ packets: [] }); + await settle(() => h.state().messages.length > 0); await flush(300); assert.strictEqual(h.state().selectedHash, NAME, 'selection must have remapped'); From d8c967598041a05d7fc685414f5912475624580a Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:10:28 +0000 Subject: [PATCH 15/23] =?UTF-8?q?fix(channels):=20R4-4=20=E2=80=94=20zero?= =?UTF-8?q?=20candidates=20drop=20the=20region's=20decrypt=20cache=20entry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the current region's decrypt fetch finds zero candidates, N2 renders an empty pane but left that region's cache entry in place, so the next visit flashed the outdated history (onCacheHit) before its own fetch answered. fetchAndDecryptChannel() now deletes the "|" entry in that case, unless the request is stale by then (a newer request owns the cache). ChannelDecrypt gains deleteCache(key) for removing one entry. Test (red on 9706722d): a stale zero-candidate answer leaves the entry, a current one drops it, and the next visit shows nothing outdated while its fetch is in flight. Mutants killed: zero candidates falling back to the cache, no delete, deleting even when stale. Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- public/channel-decrypt.js | 10 ++++++ public/channels.js | 5 ++- test-channels-client-state-152.js | 51 +++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index 864b867a4..072ce4f07 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -511,6 +511,15 @@ window.ChannelDecrypt = (function () { writeCacheBlob(cache); } + /** Remove one cache entry, e.g. one region of a channel. */ + function deleteCache(key) { + ensureCacheMigrated(); + var cache = readCacheBlob(); + if (!Object.prototype.hasOwnProperty.call(cache, key)) return; + delete cache[key]; + writeCacheBlob(cache); + } + function cacheMessages(channelHash, messages) { ensureCacheMigrated(); var cache = readCacheBlob(); @@ -573,6 +582,7 @@ window.ChannelDecrypt = (function () { getCachedMessages: getCachedMessages, setCache: setCache, getCache: getCache, + deleteCache: deleteCache, buildKeyMap: buildKeyMap, tryDecryptLive: tryDecryptLive }; diff --git a/public/channels.js b/public/channels.js index 42d123e85..afb14ea50 100644 --- a/public/channels.js +++ b/public/channels.js @@ -911,7 +911,10 @@ if (candidates.length === 0) { // N2 (#152 follow-up): zero candidates for the current (region-scoped) // fetch must render as empty, never leftover content from a stale or - // foreign-region cache entry. + // foreign-region cache entry. R4-4: drop this region's entry too, so + // the next visit doesn't flash the outdated history before its fetch + // answers — unless a newer request owns the cache by now. + if (cached && !(opts.isStale && opts.isStale())) ChannelDecrypt.deleteCache(cacheKey); return { messages: [], empty: true }; } diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 8d56ce9ba..48603b428 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -1210,6 +1210,57 @@ async function test(name, fn) { assert.ok(h.state().messages.some((m) => m.text === 's3 server message'), 'the current view must keep its messages'); }); + // ── R4-4 (#153 review round 4, P3): when the current region's fetch finds + // zero candidates, N2 rendered an empty pane but left that region's cache + // entry in place, so the next visit flashed the outdated history (via + // onCacheHit) before its own fetch answered. + await test('R4-4: zero candidates in the same region drop that region\'s cache entry (a stale request leaves it)', async () => { + const h = makeHarness(); + const KEY = 'dadadadadadadadadadadadadadadada'; + const NAME = 'psk:r4zero'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'R4 Zero'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + h.regionParam = 'SJC'; + const msg = encryptChannelMessage(KEY, 'Alice', 'aged-out message'); + + const withMessage = () => Promise.resolve({ packets: [encryptedPacket('pkt-zero', '2026-01-01T00:00:00Z', msg)] }); + h.respondPackets = withMessage; + await h.w._channelsSelectChannelForTest(HASH); + assert.ok(h.w.ChannelDecrypt.getCache(NAME + '|SJC'), 'precondition: SJC entry cached'); + + // A superseded request answering "zero candidates" must not drop the + // entry the current request owns. + const staleFetch = deferred(); + h.respondPackets = () => staleFetch.promise; + const staleSelect = h.w._channelsSelectChannelForTest(HASH); + assert.ok(await settle(() => h.packetsRequests.length === 2), 'precondition: the soon-stale fetch is in flight'); + h.respondPackets = withMessage; + await h.w._channelsSelectChannelForTest(HASH); + staleFetch.resolve({ packets: [] }); + await staleSelect; + await flush(300); + assert.ok(h.w.ChannelDecrypt.getCache(NAME + '|SJC'), 'a stale zero-candidate response must leave the cache alone'); + + // The current request finds nothing any more (the packets aged out). + h.respondPackets = () => Promise.resolve({ packets: [] }); + await h.w._channelsSelectChannelForTest(HASH); + await flush(300); + assert.strictEqual(h.state().messages.length, 0, 'zero candidates render an empty pane (got ' + JSON.stringify(h.state().messages.map((m) => m.text)) + ')'); + assert.strictEqual(h.w.ChannelDecrypt.getCache(NAME + '|SJC'), null, 'the region\'s cache entry must be dropped'); + + // Next visit: nothing outdated may flash while the fetch is in flight. + const nextFetch = deferred(); + h.respondPackets = () => nextFetch.promise; + const nextSelect = h.w._channelsSelectChannelForTest(HASH); + await flush(300); + assert.ok(!/aged-out message/.test(h.elements.chMessages.innerHTML) && h.state().messages.length === 0, + 'the next visit must not flash the outdated history (got ' + JSON.stringify(h.state().messages.map((m) => m.text)) + ')'); + nextFetch.resolve({ packets: [] }); + await nextSelect; + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From fb7000e6153a2d5b362c25b458027bb62f868bb3 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:11:50 +0000 Subject: [PATCH 16/23] =?UTF-8?q?test(channels):=20R4-5=20=E2=80=94=20sign?= =?UTF-8?q?al-aware=20E2E=20process=20helpers,=20pin=20the=20sorted=20regi?= =?UTF-8?q?on=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The decrypt E2E's waitFor() and stopProcess() only checked exitCode. A child killed by a signal (SIGKILL, SIGSEGV) has exitCode null and signalCode set, so waitFor() polled out its whole timeout and stopProcess() waited forever for an 'exit' that had already fired. Both now use hasExited() (exitCode or signalCode), and the early-exit error names the signal. - The E2E file requires node:sqlite and playwright where they are used, and exports its process helpers, so the unit suite can test them. - Unit tests: waitFor() rejects at once for SIGKILL and SIGSEGV, and stopProcess() returns at once for a signal-killed child (both red on 9706722d); SJC,SFO and SFO,SJC share one decrypt cache entry (pins existing behaviour — green on 9706722d, red on the unsorted mutant). Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- test-channels-client-state-152-decrypt-e2e.js | 23 +++++-- test-channels-client-state-152.js | 65 +++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/test-channels-client-state-152-decrypt-e2e.js b/test-channels-client-state-152-decrypt-e2e.js index 508703a39..6dd49113c 100644 --- a/test-channels-client-state-152-decrypt-e2e.js +++ b/test-channels-client-state-152-decrypt-e2e.js @@ -69,8 +69,8 @@ const fs = require('fs'); const net = require('net'); const os = require('os'); const path = require('path'); -const { DatabaseSync } = require('node:sqlite'); -const { chromium } = require('playwright'); +// node:sqlite and playwright are required where they're used, so the unit +// suite can load this file's process helpers without either. const ROOT = __dirname; const SERVER_BIN = path.resolve(process.env.CORESCOPE_SERVER_BIN || path.join(ROOT, 'corescope-server')); @@ -170,6 +170,7 @@ function computeContentHash16(rawPacket) { * Returns { keyHex, channelName, hash, sjc: {sender,text}, sfo: {sender,text} } */ function seedEncryptedChannel(dbPath) { + const { DatabaseSync } = require('node:sqlite'); const db = new DatabaseSync(dbPath); try { // Avoid colliding with any GRP_TXT channelHash byte already present in @@ -296,8 +297,18 @@ function startProcess(label, bin, args, dir) { return proc; } +// A child killed by a signal (SIGKILL, SIGSEGV, ...) exits with exitCode +// null and signalCode set, so both must be checked. +function hasExited(proc) { + return proc.exitCode !== null || proc.signalCode !== null; +} + +function exitDescription(proc) { + return proc.signalCode !== null ? 'signal ' + proc.signalCode : 'code ' + proc.exitCode; +} + async function stopProcess(proc) { - if (!proc || proc.exitCode !== null) return; + if (!proc || hasExited(proc)) return; const exited = new Promise((r) => proc.once('exit', r)); proc.kill('SIGTERM'); const t = setTimeout(() => proc.kill('SIGKILL'), 5000); @@ -312,8 +323,8 @@ async function waitFor(what, fn, timeoutMs, proc) { const deadline = Date.now() + (timeoutMs || TIMEOUT); let last; while (Date.now() < deadline) { - if (proc && proc.exitCode !== null) { - throw new Error(what + ': process "' + proc.label + '" exited early (code ' + proc.exitCode + '), see ' + proc.logFile); + if (proc && hasExited(proc)) { + throw new Error(what + ': process "' + proc.label + '" exited early (' + exitDescription(proc) + '), see ' + proc.logFile); } try { const v = await fn(); if (v) return v; } catch (e) { last = e; } await new Promise((r) => setTimeout(r, 200)); @@ -619,6 +630,7 @@ async function main() { } } + const { chromium } = require('playwright'); let browser; const launchArgs = { headless: true, args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'] }; try { @@ -721,4 +733,5 @@ if (require.main === module) { module.exports = { seedEncryptedChannel, channelHashByteFor, buildPlaintext, encryptECB, computeMac, buildRawPacket, computeContentHash16, + startProcess, stopProcess, waitFor, }; diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 48603b428..9bee304ce 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -1261,6 +1261,71 @@ async function test(name, fn) { await nextSelect; }); + // ── R4-5 (#153 review round 4, nits). + await test('R4-5: region order does not change the decrypt cache key (SJC,SFO and SFO,SJC share one entry)', async () => { + const h = makeHarness(); + const KEY = 'ebebebebebebebebebebebebebebebeb'; + const NAME = 'psk:r4sort'; + const HASH = 'user:' + NAME; + h.storeKey(NAME, KEY, 'R4 Sort'); + h.respondChannels = () => Promise.resolve({ channels: [] }); + await h.init(); + const msg = encryptChannelMessage(KEY, 'Alice', 'sorted message'); + h.respondPackets = () => Promise.resolve({ packets: [encryptedPacket('pkt-sort', '2026-01-01T00:00:00Z', msg)] }); + for (const region of ['SJC,SFO', 'SFO,SJC']) { + h.regionParam = region; + await h.w._channelsSelectChannelForTest(HASH); + } + assert.deepStrictEqual(decryptCacheKeys(h).filter((k) => k.indexOf(NAME + '|') === 0), [NAME + '|SFO,SJC'], + 'both orders must map to the one sorted key (got ' + JSON.stringify(decryptCacheKeys(h)) + ')'); + }); + + // The decrypt E2E's process helpers: a child killed by a signal has + // exitCode null and signalCode set, so checking exitCode alone missed a + // SIGKILL/SIGSEGV — waitFor() polled out its whole timeout and + // stopProcess() waited forever for an 'exit' that had already fired. + const e2eHarness = require('./test-channels-client-state-152-decrypt-e2e.js'); + const os = require('os'); + function spawnSleeper(dir) { + return e2eHarness.startProcess('sleeper', process.execPath, ['-e', 'setTimeout(() => {}, 60000)'], dir); + } + function within(ms, p, what) { + let t; + return Promise.race([p, new Promise((_, rej) => { t = setTimeout(() => rej(new Error(what + ' did not settle within ' + ms + 'ms')), ms); })]) + .finally(() => clearTimeout(t)); + } + async function killedBySignal(dir, sig) { + const proc = spawnSleeper(dir); + const exited = new Promise((r) => proc.once('exit', r)); + proc.kill(sig); + await exited; + assert.strictEqual(proc.exitCode, null, 'precondition: a signal-killed child has no exit code'); + assert.strictEqual(proc.signalCode, sig, 'precondition: signalCode is set'); + return proc; + } + + await test('R4-5: the decrypt E2E\'s waitFor() stops at once when its process dies from SIGKILL or SIGSEGV', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'r4-waitfor-')); + try { + for (const sig of ['SIGKILL', 'SIGSEGV']) { + const proc = await killedBySignal(dir, sig); + const started = RealDate.now(); + await assert.rejects(within(3000, e2eHarness.waitFor('never', () => false, 10000, proc), 'waitFor'), + (e) => /exited early/.test(e.message) && e.message.indexOf(sig) !== -1, + 'waitFor must reject naming ' + sig + ' instead of polling out its timeout'); + assert.ok(RealDate.now() - started < 1000, 'waitFor must notice the dead process at once'); + } + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); + + await test('R4-5: the decrypt E2E\'s stopProcess() returns at once for a process a signal already killed', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'r4-stop-')); + try { + const proc = await killedBySignal(dir, 'SIGKILL'); + await within(3000, e2eHarness.stopProcess(proc), 'stopProcess'); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); + console.log('\n=== Results: ' + passed + ' passed, ' + failed + ' failed ==='); process.exit(failed > 0 ? 1 : 0); })().catch((e) => { console.error('FATAL:', e); process.exit(1); }); From dde4fc9893a73b02590f138f2df940ae98d3f618 Mon Sep 17 00:00:00 2001 From: dborup Date: Thu, 1 Oct 2026 12:13:00 +0000 Subject: [PATCH 17/23] test(channels): give the observed-path-hash-size ChannelDecrypt stub channelCacheKey R4-1 moved the decrypt cache key format into ChannelDecrypt (channelCacheKey), which fetchAndDecryptChannel() now calls. The two delta-cache tests in test-channels-observed-path-hash-size.js replace ChannelDecrypt with a partial stub, so they failed with "ChannelDecrypt.channelCacheKey is not a function" (11/11 -> 9/11). The stub now provides it; the tests are unchanged otherwise and pass 11/11. Relates to #152 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SfbYnGhDMMurKHbweEqTbr --- test-channels-observed-path-hash-size.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test-channels-observed-path-hash-size.js b/test-channels-observed-path-hash-size.js index 7d759ec71..96ffe2d56 100644 --- a/test-channels-observed-path-hash-size.js +++ b/test-channels-observed-path-hash-size.js @@ -201,6 +201,7 @@ test('delta cache absorbs later evidence for the same packet without re-decrypti ctx.ChannelDecrypt = { hexToBytes: () => new Uint8Array(16), + channelCacheKey: (name, regionParam) => name + '|' + (regionParam || ''), getCache: () => ({ messages: [legacyMessage], lastTimestamp: timestamp, count: 1 }), setCache: (key, messages, lastTimestamp, count) => { stored = { key, messages: plain(messages), lastTimestamp, count }; @@ -239,6 +240,7 @@ test('delta cache unions richer evidence without replacing cached plaintext', as ctx.ChannelDecrypt = { hexToBytes: () => new Uint8Array(16), + channelCacheKey: (name, regionParam) => name + '|' + (regionParam || ''), getCache: () => ({ messages: [cachedMessage], lastTimestamp: timestamp, count: 1 }), setCache: (key, messages, lastTimestamp, count) => { stored = { key, messages: plain(messages), lastTimestamp, count }; From e635b28a437342c142256ef5c847be432efd48cf Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 2 Oct 2026 13:40:14 +0200 Subject: [PATCH 18/23] fix(channels): superseded selectChannel must not paint over the newer view (#163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selectChannel() walks the stored keys of an encrypted channel, awaiting computeChannelHash once per key, then paints the #781 lock text. It never checked isStaleMessageRequest(), so a channel or region change during the loop let the lock text land on the newer request's pane. The deep-link lookup of '#'-named channels had the same gap: when its /channels request failed, the catch fell through to "Loading messages…" regardless of who owned the pane. Check isStaleMessageRequest() after each await in the key loop (which also stops hashing keys for a dead request) and before the fall-through write. The /channels lookup branch and decryptAndRender() already checked. Tests: four new cases in test-channels-client-state-152.js, all red on dde4fc98 and on master 727efca0. --- public/channels.js | 5 ++ test-channels-client-state-152.js | 114 ++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/public/channels.js b/public/channels.js index afb14ea50..2252b2257 100644 --- a/public/channels.js +++ b/public/channels.js @@ -2533,6 +2533,8 @@ var kh = storedKeys[kn]; var kb = ChannelDecrypt.hexToBytes(kh); var hb = await ChannelDecrypt.computeChannelHash(kb); + // #163: a newer request owns the pane; stop hashing keys for it. + if (isStaleMessageRequest(request)) return; if (String(hb) === String(hash) || String(ch.hash) === String(hb)) { await decryptAndRender(kh, hb, kn); return; @@ -2577,6 +2579,9 @@ } } + // #163: the encrypted-ness lookup above falls through here when it + // fails, which can be after a newer request took over the pane. + if (isStaleMessageRequest(request)) return; msgEl.innerHTML = '
Loading messages…
'; try { diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 9bee304ce..0b70007ff 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -1210,6 +1210,120 @@ async function test(name, fn) { assert.ok(h.state().messages.some((m) => m.text === 's3 server message'), 'the current view must keep its messages'); }); + // ── #163 item 1: selectChannel() walks every stored key for an encrypted + // channel, awaiting computeChannelHash once per key, and then paints the + // #781 "no decryption key" lock text. Nothing checked whether the request + // was still current, so the lock text landed on top of a newer request's + // view — the same late-write pattern R4-3 S3 fixed for "Decrypting…". + const LOCK_TEXT = /no decryption key is configured/; + const msgBelongsTo = (h, text) => h.state().messages.some((m) => m.text === text); + + async function encryptedChannelHarness(storedKeys) { + const h = makeHarness(); + for (let i = 0; i < storedKeys; i++) h.storeKey('psk:i163k' + i, (i + 1).toString(16).repeat(32), 'K' + i); + h.respondChannels = () => Promise.resolve({ channels: [serverChannel('public')] }); + await h.init(); + h.setState({ + channels: [ + Object.assign({}, h.row('public')), + serverChannel('42', { encrypted: true, name: 'Encrypted 42' }), + ], + }); + const realApi = h.ctx.api; + h.ctx.api = (p, o) => (p.indexOf('/channels/public/messages') === 0 + ? Promise.resolve({ messages: [{ id: 1, sender: 'Srv', text: 'i163 public message', timestamp: '2026-01-01T00:00:00Z', packetHash: 'srv-public' }] }) + : realApi(p, o)); + // computeChannelHash is slow: each stored key's call stays pending + // until the test releases it, and none of them matches channel 42. + h.hashCalls = []; + h.w.ChannelDecrypt.computeChannelHash = () => { const d = deferred(); h.hashCalls.push(d); return d.promise; }; + return h; + } + + // Releases A's computeChannelHash calls as they appear (none matches), + // until A's selectChannel() has returned. + async function releaseKeyLoop(h, selection) { + let done = false; + selection.then(() => { done = true; }, () => { done = true; }); + let released = 0; + for (let i = 0; i < 200 && !done; i++) { + while (released < h.hashCalls.length) h.hashCalls[released++].resolve(250); + await flush(5); + } + assert.ok(done, 'the superseded selectChannel() must return'); + await flush(100); + } + + await test('#163 item 1: the lock text of a superseded encrypted selection does not overwrite the newer channel\'s view', async () => { + const h = await encryptedChannelHarness(3); + const selectA = h.w._channelsSelectChannelForTest('42'); + assert.ok(await settle(() => h.hashCalls.length === 1), 'precondition: A is awaiting its first computeChannelHash'); + + await h.w._channelsSelectChannelForTest('public'); + await flush(50); + assert.strictEqual(h.state().selectedHash, 'public', 'precondition: B is selected'); + assert.ok(msgBelongsTo(h, 'i163 public message'), 'precondition: B\'s messages are rendered'); + assert.ok(!LOCK_TEXT.test(h.elements.chMessages.innerHTML), 'precondition: no lock text yet'); + + await releaseKeyLoop(h, selectA); + + assert.ok(!LOCK_TEXT.test(h.elements.chMessages.innerHTML), + 'the lock text of the superseded request must not be painted over B (got ' + h.elements.chMessages.innerHTML.slice(0, 160) + ')'); + assert.strictEqual(h.state().selectedHash, 'public', 'B must stay selected'); + assert.ok(msgBelongsTo(h, 'i163 public message'), 'B\'s messages must be intact'); + }); + + await test('#163 item 1: a superseded encrypted selection stops walking the stored keys', async () => { + const h = await encryptedChannelHarness(4); + // Only the first call is slow; any further call is counted and answered. + let calls = 0; + const first = deferred(); + h.w.ChannelDecrypt.computeChannelHash = () => (++calls === 1 ? first.promise : Promise.resolve(250)); + const selectA = h.w._channelsSelectChannelForTest('42'); + assert.ok(await settle(() => calls === 1), 'precondition: A awaits the first key'); + await h.w._channelsSelectChannelForTest('public'); + first.resolve(250); + await selectA; + await flush(100); + assert.strictEqual(calls, 1, 'A must not hash the remaining keys once it is superseded (got ' + calls + ' calls)'); + }); + + await test('#163 item 1: a region change while the key loop runs keeps the lock text off the pane', async () => { + const h = await encryptedChannelHarness(2); + const selectA = h.w._channelsSelectChannelForTest('42'); + assert.ok(await settle(() => h.hashCalls.length === 1), 'precondition: A awaits the first key'); + // The region changes under A, which makes A stale; the pane now shows + // something else. + h.regionParam = 'SJC'; + h.elements.chMessages.innerHTML = '
region view
'; + await releaseKeyLoop(h, selectA); + assert.ok(/i163-region-view/.test(h.elements.chMessages.innerHTML), + 'a request superseded by a region change must not touch the pane (got ' + h.elements.chMessages.innerHTML.slice(0, 160) + ')'); + }); + + await test('#163 item 1: a failed encrypted-ness lookup of a superseded deep link does not paint "Loading messages…" over the newer view', async () => { + const h = await encryptedChannelHarness(0); + // A deep link to a '#'-named channel that is not in the loaded list + // starts a /channels?includeEncrypted lookup; it fails after the user + // has moved on to another channel. + let failLookup; + const lookup = new Promise((_resolve, reject) => { failLookup = reject; }); + h.respondChannels = () => lookup; + const selectA = h.w._channelsSelectChannelForTest('#i163deep'); + assert.ok(await settle(() => h.channelRequests.some((p) => p.indexOf('includeEncrypted=true') !== -1)), 'precondition: the lookup is in flight'); + + await h.w._channelsSelectChannelForTest('public'); + await flush(50); + assert.ok(msgBelongsTo(h, 'i163 public message'), 'precondition: B\'s messages are rendered'); + + h.elements.chMessages.innerHTML = '
B view
'; + failLookup(new Error('lookup failed')); + await selectA; + await flush(100); + assert.ok(/i163-b-view/.test(h.elements.chMessages.innerHTML), + 'a superseded request must not paint over B after its lookup fails (got ' + h.elements.chMessages.innerHTML.slice(0, 160) + ')'); + }); + // ── R4-4 (#153 review round 4, P3): when the current region's fetch finds // zero candidates, N2 rendered an empty pane but left that region's cache // entry in place, so the next visit flashed the outdated history (via From 29d47198092c98fd153acd27011c85522d5d9a93 Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 2 Oct 2026 13:41:20 +0200 Subject: [PATCH 19/23] fix(channels): make decrypt-cache keys unambiguous for names containing '|' (#163) Cache keys are "|" and clearChannelCache() removed every key starting with "|", so a channel "#a" also cleared "#a|b"'s entries, and their keys could be confused. The channel part now has '%' and '|' percent-encoded (names without either keep their plain key), so the first '|' of a key is always the separator and the encoding is injective. clearChannelCache() matches that exact prefix and no longer special-cases a bare-name key. Migration: CACHE_VERSION is 3. A blob written before it (pre-#153 bare-name keys or #153's unescaped keys, which can't be told apart for a name with '|') is dropped whole on first use. It is only a cache, and no old plaintext remains. Tests: new #163 item 2 cases (clear "#a" keeps "#a|b"; removeKey; the encoding is injective; both old formats are gone, with and without a version marker), all red on dde4fc98. Three existing tests change with the behaviour: the R4-2 pre-N2 migration test (a region-scoped entry of the old format no longer survives), R4-1 (the legacy bare-name entry is the migration's job now) and two cases in test-channel-decrypt-m345.js that cached under a bare name instead of channelCacheKey(). --- public/channel-decrypt.js | 41 ++++++++------ test-channel-decrypt-m345.js | 14 ++--- test-channels-client-state-152.js | 90 ++++++++++++++++++++++++------- 3 files changed, 105 insertions(+), 40 deletions(-) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index 072ce4f07..ab88bb39d 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -358,11 +358,20 @@ window.ChannelDecrypt = (function () { // region selection, as "|" ("|" for all // regions). Region order doesn't change which observers are included, so // it must not change the key either. + // + // #163: a channel name may contain '|', which would make the key (and + // clearChannelCache()'s prefix match) ambiguous. The channel part therefore + // has '%' and '|' percent-encoded, so the first '|' of a key is always the + // separator. Names without either character keep their plain key. var CACHE_REGION_SEP = '|'; + function encodeChannelPart(channelName) { + return String(channelName).replace(/[%|]/g, encodeURIComponent); + } + function channelCacheKey(channelName, regionParam) { var regions = regionParam ? String(regionParam).split(',').filter(Boolean).sort().join(',') : ''; - return channelName + CACHE_REGION_SEP + regions; + return encodeChannelPart(channelName) + CACHE_REGION_SEP + regions; } // ---- Message cache (localStorage) ---- @@ -383,9 +392,10 @@ window.ChannelDecrypt = (function () { // with the keys and labels. Keep the whole blob under this many // characters, least recently used entries going first. var CACHE_BUDGET_CHARS = 1500000; - // Set once the pre-N2 entries (keyed by channel name alone) are dropped. + // Set once the cache blob of an earlier key format is dropped. 2: the + // unescaped "|" keys of #153; 3 (#163): escaped keys. var CACHE_VERSION_KEY = 'corescope_channel_cache_v'; - var CACHE_VERSION = '2'; + var CACHE_VERSION = '3'; var _cacheMigrated = false; var _cacheUseSeq = 0; @@ -412,21 +422,19 @@ window.ChannelDecrypt = (function () { } catch (e) { return {}; } } - // Drop the pre-N2 entries once: they are keyed by channel name alone, so - // nothing reads them any more, and removeKey() of a later version would - // have no reason to look for them. + // Drop the whole cache blob once when its key format is older than + // CACHE_VERSION. Pre-#153 entries are keyed by the bare channel name and + // #153's by the unescaped "|"; for a name containing '|' + // the two can't be told apart from the current keys, and nothing reads + // them any more. It is only a cache, so dropping all of it is simpler than + // rewriting it, and leaves no old plaintext behind. function ensureCacheMigrated() { if (_cacheMigrated) return; _cacheMigrated = true; try { if (localStorage.getItem(CACHE_VERSION_KEY) === CACHE_VERSION) return; - } catch (e) { return; } - var cache = readCacheBlob(); - var legacy = Object.keys(cache).filter(function (k) { return k.indexOf(CACHE_REGION_SEP) === -1; }); - if (legacy.length) { - legacy.forEach(function (k) { delete cache[k]; }); - writeCacheBlob(cache); - } + localStorage.removeItem(CACHE_KEY); + } catch (e) { _cacheMigrated = false; return; /* storage unavailable: retried on next use */ } try { localStorage.setItem(CACHE_VERSION_KEY, CACHE_VERSION); } catch (e) { /* retried next load */ } } @@ -499,14 +507,15 @@ window.ChannelDecrypt = (function () { /** * Remove every cached message set of a channel (by name or hash): each - * region-scoped "|" entry plus a pre-N2 "" one. + * region-scoped "|" entry, and nothing of any other + * channel (see encodeChannelPart()). */ function clearChannelCache(channelKey) { ensureCacheMigrated(); var cache = readCacheBlob(); - var prefix = channelKey + CACHE_REGION_SEP; + var prefix = encodeChannelPart(channelKey) + CACHE_REGION_SEP; Object.keys(cache).forEach(function (k) { - if (k === channelKey || k.indexOf(prefix) === 0) delete cache[k]; + if (k.indexOf(prefix) === 0) delete cache[k]; }); writeCacheBlob(cache); } diff --git a/test-channel-decrypt-m345.js b/test-channel-decrypt-m345.js index 0dccbbdf8..87dd1b184 100644 --- a/test-channel-decrypt-m345.js +++ b/test-channel-decrypt-m345.js @@ -103,19 +103,19 @@ async function runTests() { // Store a key and some cached messages CD.storeKey('#TestChannel', 'deadbeefdeadbeefdeadbeefdeadbeef'); - CD.setCache('#TestChannel', [{ sender: 'A', text: 'hello', timestamp: '2026-01-01T00:00:00Z', packetHash: 'h1' }], '2026-01-01T00:00:00Z', 1); + CD.setCache(CD.channelCacheKey('#TestChannel', ''), [{ sender: 'A', text: 'hello', timestamp: '2026-01-01T00:00:00Z', packetHash: 'h1' }], '2026-01-01T00:00:00Z', 1); // Verify they exist var storedKeys = CD.getStoredKeys(); assert(storedKeys['#TestChannel'] === 'deadbeefdeadbeefdeadbeefdeadbeef', 'Key exists before removal'); - var cachedBefore = CD.getCache('#TestChannel'); + var cachedBefore = CD.getCache(CD.channelCacheKey('#TestChannel', '')); assert(cachedBefore && cachedBefore.messages.length === 1, 'Cache exists before removal'); // Remove the key (also clears cache) CD.removeKey('#TestChannel'); var storedAfter = CD.getStoredKeys(); assert(!storedAfter['#TestChannel'], 'Key cleared after removal'); - var cachedAfter = CD.getCache('#TestChannel'); + var cachedAfter = CD.getCache(CD.channelCacheKey('#TestChannel', '')); assert(!cachedAfter, 'Cache cleared after removal'); console.log('\n=== M5: Cache operations ==='); @@ -144,10 +144,12 @@ async function runTests() { assert(deltaCache.count === 1, 'Delta cache count correct'); // Test: clearChannelCache - CD.setCache('clearthis', [{ sender: 'X', text: 'y' }], 'ts', 1); - assert(CD.getCache('clearthis') !== null, 'Cache exists before clear'); + // The key is the real one (#153/#163): "|". + var clearKey = CD.channelCacheKey('clearthis', ''); + CD.setCache(clearKey, [{ sender: 'X', text: 'y' }], 'ts', 1); + assert(CD.getCache(clearKey) !== null, 'Cache exists before clear'); CD.clearChannelCache('clearthis'); - assert(CD.getCache('clearthis') === null, 'Cache cleared by clearChannelCache'); + assert(CD.getCache(clearKey) === null, 'Cache cleared by clearChannelCache'); console.log('\n=== Results ==='); console.log('Passed: ' + passed + ', Failed: ' + failed); diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 0b70007ff..aa0312697 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -983,14 +983,16 @@ async function test(name, fn) { // region-scoped ("|"), but removeKey() still only // deleted cache[""], so removing a key left the channel's // decrypted plaintext in localStorage under every region it was viewed in. - await test('R4-1: removeKey drops every region-scoped cache entry and the pre-N2 one, keeping a prefix-sharing channel', async () => { + await test('R4-1: removeKey drops every region-scoped cache entry, keeping a prefix-sharing channel', async () => { const h = makeHarness(); const NAME = 'psk:r4rm'; const OTHER = 'psk:r4rmx'; // "psk:r4rm" is a prefix of this name h.storeKey(NAME, 'd4'.repeat(16), 'R4 Remove'); h.storeKey(OTHER, 'e5'.repeat(16), 'R4 Other'); + // Entries of the current key format; the older formats are dropped by + // the version migration (see the #163 item 2 tests), not by removeKey. + h.ctx.localStorage.setItem('corescope_channel_cache_v', '3'); const blob = {}; - blob[NAME] = cacheEntry('legacy secret'); blob[NAME + '|'] = cacheEntry('all-regions secret'); blob[NAME + '|SJC'] = cacheEntry('sjc secret'); blob[NAME + '|SFO,SJC'] = cacheEntry('sfo-sjc secret'); @@ -1000,7 +1002,7 @@ async function test(name, fn) { h.removeKey(NAME); const keys = decryptCacheKeys(h); - assert.deepStrictEqual(keys.filter((k) => k === NAME || k.indexOf(NAME + '|') === 0), [], + assert.deepStrictEqual(keys.filter((k) => k.indexOf(NAME + '|') === 0), [], 'no cache entry for the removed channel may survive (got ' + JSON.stringify(keys) + ')'); assert.ok(!/secret/.test(rawDecryptCache(h)), 'no plaintext of the removed channel may stay in localStorage'); assert.ok(keys.indexOf(OTHER + '|SJC') !== -1, 'a channel whose name merely shares the prefix must keep its cache (got ' + JSON.stringify(keys) + ')'); @@ -1084,23 +1086,75 @@ async function test(name, fn) { assert.ok(sb.keys().length < 3, 'room must come from the decrypt cache (got ' + JSON.stringify(sb.keys()) + ')'); }); - await test('R4-2: pre-N2 cache entries (no "|" region part) are dropped once, on first use after upgrade', async () => { - const legacy = {}; - legacy['#oldchan'] = cacheEntry('pre-N2 plaintext'); - legacy['psk:legacy'] = cacheEntry('pre-N2 psk plaintext'); - legacy['psk:legacy|SJC'] = cacheEntry('region-scoped text'); - const sb = makeDecryptSandbox({ storage: { [DECRYPT_CACHE_KEY]: JSON.stringify(legacy) } }); - assert.strictEqual(sb.CD.getCache('psk:legacy|SJC').messages[0].text, 'region-scoped text', 'region-scoped entries survive'); - assert.deepStrictEqual(sb.keys(), ['psk:legacy|SJC'], 'pre-N2 entries must be dropped (got ' + JSON.stringify(sb.keys()) + ')'); - assert.ok(!/pre-N2/.test(sb.raw()), 'no pre-N2 plaintext may remain'); - - // Only once: an entry written after the migration (the old public - // setCache/cacheMessages API still accepts any key) survives a reload. - sb.CD.setCache('written-later', [{ text: 'kept' }], 'ts', 1); - const reloaded = makeDecryptSandbox({ storage: sb.storage }); - assert.ok(reloaded.CD.getCache('written-later'), 'the migration must not run again on the next page load'); + // #163 item 2: cache keys are "|", but a channel name may + // contain '|' itself, so clearing "#a" also cleared "#a|b" and the two + // channels' entries could be confused. The channel part now has '|' (and + // '%', which escapes it) percent-encoded, so the first '|' of a key is + // always the separator. + await test('#163 item 2: removing channel "#a" leaves the cache of channel "#a|b" intact', async () => { + const sb = makeDecryptSandbox(); + const CD = sb.CD; + const keysOf = { a: [CD.channelCacheKey('#a', ''), CD.channelCacheKey('#a', 'SJC')], ab: [CD.channelCacheKey('#a|b', ''), CD.channelCacheKey('#a|b', 'SJC')] }; + keysOf.a.concat(keysOf.ab).forEach((k) => CD.setCache(k, [{ text: 'plain ' + k }], 'ts', 1)); + assert.strictEqual(new Set(keysOf.a.concat(keysOf.ab)).size, 4, 'every (channel, region) pair needs its own key'); + + CD.clearChannelCache('#a'); + keysOf.a.forEach((k) => assert.strictEqual(CD.getCache(k), null, '#a\'s entry ' + k + ' must be cleared')); + keysOf.ab.forEach((k) => assert.ok(CD.getCache(k), '#a|b\'s entry ' + k + ' must survive (left: ' + JSON.stringify(sb.keys()) + ')')); + + CD.clearChannelCache('#a|b'); + assert.deepStrictEqual(sb.keys(), [], 'clearing #a|b removes its own entries (left: ' + JSON.stringify(sb.keys()) + ')'); + }); + + await test('#163 item 2: removeKey("#a") clears only that channel\'s decrypt cache', async () => { + const sb = makeDecryptSandbox(); + const CD = sb.CD; + CD.storeKey('#a', '0123456789abcdef0123456789abcdef', 'A'); + CD.storeKey('#a|b', 'fedcba9876543210fedcba9876543210', 'AB'); + CD.setCache(CD.channelCacheKey('#a', 'SJC'), [{ text: 'a' }], 'ts', 1); + CD.setCache(CD.channelCacheKey('#a|b', ''), [{ text: 'ab' }], 'ts', 1); + CD.removeKey('#a'); + assert.strictEqual(CD.getCache(CD.channelCacheKey('#a', 'SJC')), null, '#a\'s cache goes with its key'); + assert.ok(CD.getCache(CD.channelCacheKey('#a|b', '')), '#a|b\'s cache must stay'); + assert.ok(CD.getKeys()['#a|b'], '#a|b\'s key must stay'); }); + await test('#163 item 2: the escaping is injective — "a|b" and "a%7Cb" are different channels', async () => { + const { CD } = makeDecryptSandbox(); + const names = ['a', 'a|b', 'a%7Cb', 'a%', 'a%25', 'a|', '|', '%7C', 'a|b|c']; + const keys = names.map((n) => CD.channelCacheKey(n, '')); + assert.strictEqual(new Set(keys).size, names.length, 'distinct channels must have distinct keys (got ' + JSON.stringify(keys) + ')'); + keys.forEach((k, i) => assert.strictEqual(k.indexOf('|'), k.length - 1, 'the only "|" of ' + JSON.stringify(names[i]) + '\'s key is the separator (got ' + k + ')')); + assert.strictEqual(CD.channelCacheKey('#a', 'SFO,SJC'), CD.channelCacheKey('#a', 'SJC,SFO'), 'region order still does not matter'); + assert.strictEqual(CD.channelCacheKey('psk:r4sort', 'SJC'), 'psk:r4sort|SJC', 'a name without "|" or "%" keeps its key'); + }); + + // Every cache blob written before the key format settled is dropped once: + // pre-#153 entries are keyed by the bare channel name, #153's by the + // unescaped "|", and the two can't be told apart for a name + // containing '|'. It is only a cache, and none of them may keep plaintext. + for (const marker of [undefined, '2']) { + await test('#163 item 2: cache entries of the pre-#153 and #153 key formats are dropped once (version marker ' + marker + ')', async () => { + const old = {}; + old['#oldchan'] = cacheEntry('PLAIN pre-153 bare name'); + old['#a|b'] = cacheEntry('PLAIN pre-153 name with pipe'); + old['psk:legacy|SJC'] = cacheEntry('PLAIN 153 region key'); + old['#a|b|'] = cacheEntry('PLAIN 153 pipe name'); + old['#a|b|SJC'] = cacheEntry('PLAIN 153 pipe name region'); + const storage = { [DECRYPT_CACHE_KEY]: JSON.stringify(old) }; + if (marker !== undefined) storage.corescope_channel_cache_v = marker; + const sb = makeDecryptSandbox({ storage }); + assert.strictEqual(sb.CD.getCache('psk:legacy|SJC'), null, 'an old-format entry must not be served'); + assert.deepStrictEqual(sb.keys(), [], 'old-format entries must be dropped (got ' + JSON.stringify(sb.keys()) + ')'); + assert.ok(!/PLAIN/.test(sb.raw()), 'no old plaintext may remain'); + + // Only once: an entry written after the migration survives a reload. + sb.CD.setCache(sb.CD.channelCacheKey('written-later', ''), [{ text: 'kept' }], 'ts', 1); + const reloaded = makeDecryptSandbox({ storage: sb.storage }); + assert.ok(reloaded.CD.getCache(sb.CD.channelCacheKey('written-later', '')), 'the migration must not run again on the next page load'); + }); + } + // ── R4-3 (#153 review round 4, P3): the N1 "a load is pending" flag was // one shared boolean, set only inside decryptAndRender(). A superseded // decrypt cleared it in its finally while a newer one was still running From 728fa982593616bfbb84618eb6d52a4a25744763 Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 2 Oct 2026 13:41:34 +0200 Subject: [PATCH 20/23] chore: ignore the corescope-ingestor build output (#163) Every local build of the ingestor binary into the repo root left an untracked file next to the already-ignored corescope-server. Proof: after (cd cmd/ingestor && go build -o ../../corescope-ingestor .) git status shows nothing for the binary. There is no go.mod at the root, so the issue's go build -o corescope-ingestor ./cmd/ingestor does not run from the root; this is the equivalent that writes the same file. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f50e06424..bbe6df765 100644 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,7 @@ cmd/ingestor/ingestor.exe # CI trigger !test-fixtures/e2e-fixture.db corescope-server +corescope-ingestor cmd/server/server # Local-only planning and design files docs/superpowers/ From 9fdfb857e845e8c171af8ce0fcac209b22dbd1a4 Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 2 Oct 2026 13:41:54 +0200 Subject: [PATCH 21/23] refactor(channels): remove unused cacheMessages/getCachedMessages (#163) Both wrote and read bare "" keys, outside the "|" format channelCacheKey() produces, and no production code called them. A future caller goes through channelCacheKey() + setCache()/getCache(). Tests: the decrypt E2E's roundtrip step now exercises setCache()/getCache() under channelCacheKey() (same roundtrip coverage, plus a check that nothing lands under the bare name). A unit test asserts the old exports are gone (red on dde4fc98). --- public/channel-decrypt.js | 14 -------------- test-channel-decrypt-e2e.js | 12 ++++++++---- test-channels-client-state-152.js | 12 ++++++++++++ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index ab88bb39d..d1b152b11 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -529,18 +529,6 @@ window.ChannelDecrypt = (function () { writeCacheBlob(cache); } - function cacheMessages(channelHash, messages) { - ensureCacheMigrated(); - var cache = readCacheBlob(); - cache[channelHash] = { messages: messages, ts: Date.now(), at: nextCacheUse() }; - writeCacheBlob(cache); - } - - function getCachedMessages(channelHash) { - var entry = getCache(channelHash); - return entry ? entry.messages : null; - } - function setCache(key, messages, lastTimestamp, totalCount) { ensureCacheMigrated(); // Enforce cache size limit: only keep most recent MAX_CACHED_MESSAGES @@ -587,8 +575,6 @@ window.ChannelDecrypt = (function () { getLabels: getLabels, channelCacheKey: channelCacheKey, clearChannelCache: clearChannelCache, - cacheMessages: cacheMessages, - getCachedMessages: getCachedMessages, setCache: setCache, getCache: getCache, deleteCache: deleteCache, diff --git a/test-channel-decrypt-e2e.js b/test-channel-decrypt-e2e.js index f14c6eaf6..2f37172c1 100644 --- a/test-channel-decrypt-e2e.js +++ b/test-channel-decrypt-e2e.js @@ -194,12 +194,16 @@ function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } assert(result.count === 1500, 'count: ' + result.count); }); - await step('cacheMessages / getCachedMessages roundtrip', async () => { + await step('setCache / getCache roundtrip under channelCacheKey()', async () => { const out = await page.evaluate(() => { - window.ChannelDecrypt.cacheMessages('hash42', [{ a: 1 }, { a: 2 }]); - return window.ChannelDecrypt.getCachedMessages('hash42'); + const key = window.ChannelDecrypt.channelCacheKey('hash42', 'SJC'); + window.ChannelDecrypt.setCache(key, [{ a: 1 }, { a: 2 }], 'ts', 2); + const entry = window.ChannelDecrypt.getCache(key); + return { key: key, messages: entry && entry.messages, bareName: window.ChannelDecrypt.getCache('hash42') }; }); - assert(Array.isArray(out) && out.length === 2, 'cache roundtrip failed: ' + JSON.stringify(out)); + assert(out.key === 'hash42|SJC', 'channelCacheKey: ' + out.key); + assert(Array.isArray(out.messages) && out.messages.length === 2, 'cache roundtrip failed: ' + JSON.stringify(out)); + assert(out.bareName === null, 'nothing is cached under the bare channel name: ' + JSON.stringify(out.bareName)); }); await step('buildKeyMap indexes stored keys by computed hash byte', async () => { diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index aa0312697..5645e389c 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -1129,6 +1129,18 @@ async function test(name, fn) { assert.strictEqual(CD.channelCacheKey('psk:r4sort', 'SJC'), 'psk:r4sort|SJC', 'a name without "|" or "%" keeps its key'); }); + // #163 item 4: cacheMessages()/getCachedMessages() wrote and read bare + // "" keys, outside the "|" format. Nothing + // called them; any caller has to go through channelCacheKey(). + await test('#163 item 4: the unused bare-name cacheMessages/getCachedMessages API is gone', async () => { + const { CD } = makeDecryptSandbox(); + assert.strictEqual(CD.cacheMessages, undefined, 'cacheMessages must not be exported'); + assert.strictEqual(CD.getCachedMessages, undefined, 'getCachedMessages must not be exported'); + assert.strictEqual(typeof CD.channelCacheKey, 'function', 'channelCacheKey stays the way to build a key'); + assert.strictEqual(typeof CD.setCache, 'function'); + assert.strictEqual(typeof CD.getCache, 'function'); + }); + // Every cache blob written before the key format settled is dropped once: // pre-#153 entries are keyed by the bare channel name, #153's by the // unescaped "|", and the two can't be told apart for a name From 40bd9d3e14972b13ac76dd64b4ab680cd8cbd6bd Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 2 Oct 2026 13:42:16 +0200 Subject: [PATCH 22/23] fix(channels): migrate the decrypt cache when ChannelDecrypt loads (#163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureCacheMigrated() ran on the first cache read or write, so old-format decrypted plaintext stayed in localStorage until the channels page used the cache — even for a channel whose key was removed before the upgrade. Call it once when the module initialises. The calls in the cache functions stay as the fallback for a localStorage that isn't available yet; a failed getItem/removeItem no longer marks the migration as done, so that fallback can still run. Tests: loading the module alone (no cache call) drops old-format entries (version marker absent and "2"), red on dde4fc98; a current-format cache survives the load; the lazy fallback runs when localStorage appears after load. --- public/channel-decrypt.js | 5 ++++ test-channels-client-state-152.js | 38 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/public/channel-decrypt.js b/public/channel-decrypt.js index d1b152b11..e6e9891ac 100644 --- a/public/channel-decrypt.js +++ b/public/channel-decrypt.js @@ -554,6 +554,11 @@ window.ChannelDecrypt = (function () { return entry; } + // #163: drop old-format cache entries (and their plaintext) when the module + // loads, not on the first cache use. The calls in the cache functions stay + // as the fallback for a localStorage that wasn't available yet. + ensureCacheMigrated(); + return { deriveKey: deriveKey, decrypt: decrypt, diff --git a/test-channels-client-state-152.js b/test-channels-client-state-152.js index 5645e389c..fcd21ea31 100644 --- a/test-channels-client-state-152.js +++ b/test-channels-client-state-152.js @@ -311,6 +311,10 @@ function makeDecryptSandbox(opts) { ctx.window = ctx; ctx.self = ctx; vm.createContext(ctx); + // lateStorage: the module loads before localStorage exists. + const storageApi = ctx.localStorage; + if (opts.lateStorage) delete ctx.localStorage; + sb.attachStorage = () => { ctx.localStorage = storageApi; }; vm.runInContext(fs.readFileSync(path.join(__dirname, 'public/channel-decrypt.js'), 'utf8'), ctx, { filename: 'public/channel-decrypt.js' }); sb.CD = ctx.ChannelDecrypt; sb.raw = () => sb.storage[DECRYPT_CACHE_KEY] || ''; @@ -1167,6 +1171,40 @@ async function test(name, fn) { }); } + // #163 item 5: the cleanup ran lazily, on the first cache read or write, + // so old-format plaintext stayed in localStorage until the channels page + // happened to use the cache. It now runs when ChannelDecrypt initialises. + const OLD_BLOB = () => JSON.stringify({ + '#oldchan': cacheEntry('PLAIN pre-153'), + 'psk:legacy|SJC': cacheEntry('PLAIN 153'), + }); + + for (const marker of [undefined, '2']) { + await test('#163 item 5: loading the module drops old-format cache entries without any cache use (version marker ' + marker + ')', async () => { + const storage = { [DECRYPT_CACHE_KEY]: OLD_BLOB() }; + if (marker !== undefined) storage.corescope_channel_cache_v = marker; + const sb = makeDecryptSandbox({ storage }); + // No getCache()/setCache()/clearChannelCache() call: only the load. + assert.ok(!/PLAIN/.test(sb.raw()), 'old plaintext must be gone right after load (got ' + sb.raw().slice(0, 100) + ')'); + assert.strictEqual(sb.storage.corescope_channel_cache_v, '3', 'the version marker must be set'); + }); + } + + await test('#163 item 5: a current-format cache survives the load', async () => { + const key = makeDecryptSandbox().CD.channelCacheKey('psk:keep', 'SJC'); + const sb = makeDecryptSandbox({ storage: { corescope_channel_cache_v: '3', [DECRYPT_CACHE_KEY]: JSON.stringify({ [key]: cacheEntry('KEEP') }) } }); + assert.deepStrictEqual(sb.keys(), [key], 'a version-3 cache must be left alone by the load'); + assert.strictEqual(sb.CD.getCache(key).messages[0].text, 'KEEP'); + }); + + await test('#163 item 5: the lazy call stays as a fallback when localStorage was not available at load', async () => { + const sb = makeDecryptSandbox({ lateStorage: true, storage: { [DECRYPT_CACHE_KEY]: OLD_BLOB() } }); + assert.ok(/PLAIN/.test(sb.raw()), 'precondition: nothing could be cleaned at load'); + sb.attachStorage(); + assert.strictEqual(sb.CD.getCache('psk:legacy|SJC'), null, 'the first cache use must not serve an old-format entry'); + assert.ok(!/PLAIN/.test(sb.raw()), 'the first cache use must drop the old plaintext (got ' + sb.raw().slice(0, 100) + ')'); + }); + // ── R4-3 (#153 review round 4, P3): the N1 "a load is pending" flag was // one shared boolean, set only inside decryptAndRender(). A superseded // decrypt cleared it in its finally while a newer one was still running From 403075e80495fce5b1080b630a9b96e19bcdfa49 Mon Sep 17 00:00:00 2001 From: dborup Date: Fri, 2 Oct 2026 15:06:19 +0200 Subject: [PATCH 23/23] test(e2e): wait for the loaded channel list in the #1468 tests Both #1468 tests snapshotted window._channelsGetStateForTest() as soon as the WS test hook existed. channels.js sets that hook synchronously in init(), before loadChannels() has an /api/channels answer, so `before` could read 0 channels. When the response then landed between the two evaluate() calls, `after` read the full list and the orphan test failed with before=0, after=5 although the orphan message was dropped (CI run 37002943213). With a delay on /api/channels the same race fails on #153's head too, and it also hits the control test: a WS-born channel pushed while the request is in flight is not in the snapshot, and mergeClientChannelState() only enriches rows the snapshot has. waitForChannelListSettled() waits until #chList has rendered the list (no .ch-loading), the count is > 0 and unchanged for 300 ms; both tests take their `before` snapshot only after that. Test-only change. Co-Authored-By: Claude Opus 5.5 --- test-e2e-playwright.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test-e2e-playwright.js b/test-e2e-playwright.js index c41685224..74182fce1 100644 --- a/test-e2e-playwright.js +++ b/test-e2e-playwright.js @@ -45,6 +45,28 @@ async function gotoPackets(page) { await page.waitForSelector('table tbody tr[data-hash]', { timeout: 15000 }); } +// Waits until the channels page has rendered its /api/channels list and the +// channel count has stopped changing. channels.js sets its WS test hooks +// synchronously in init(), before loadChannels() resolves, so a test that +// snapshots the list as soon as the hook exists can see 0 channels and then +// the full list one evaluate later, whatever its WS message did. +async function waitForChannelListSettled(page, { timeout = 10000, stableMs = 300 } = {}) { + const deadline = Date.now() + timeout; + let last = -1; + let since = Date.now(); + for (;;) { + const count = await page.evaluate(() => { + const list = document.getElementById('chList'); + if (!list || list.querySelector('.ch-loading') || typeof window._channelsGetStateForTest !== 'function') return -1; + return window._channelsGetStateForTest().channels.length; + }); + if (count !== last) { last = count; since = Date.now(); } + if (count > 0 && Date.now() - since >= stableMs) return count; + if (Date.now() > deadline) throw new Error(`channel list did not settle within ${timeout}ms (last count ${count})`); + await page.waitForTimeout(50); + } +} + async function run() { console.log('Launching Chromium...'); const browser = await chromium.launch({ @@ -2296,6 +2318,9 @@ async function run() { await page.goto(`${BASE}/#/channels`, { waitUntil: 'domcontentloaded' }); // Wait for the channels init() to mount and expose the test hook. await page.waitForFunction(() => typeof window._channelsProcessWSBatchForTest === 'function', { timeout: 10000 }); + // The hook exists before /api/channels has answered; snapshot only the + // loaded list, or the count can jump from 0 between before and after. + await waitForChannelListSettled(page); // Snapshot starting state so we can compare deltas. const before = await page.evaluate(() => { @@ -2336,6 +2361,9 @@ async function run() { await page.setViewportSize({ width: 1280, height: 720 }); await page.goto(`${BASE}/#/channels`, { waitUntil: 'domcontentloaded' }); await page.waitForFunction(() => typeof window._channelsProcessWSBatchForTest === 'function', { timeout: 10000 }); + // Same as above: route the message into the loaded list, not one that + // loadChannels() replaces a moment later. + await waitForChannelListSettled(page); const sentinel = '__test_chan_1468_' + Date.now(); const before = await page.evaluate((name) => {