From 59fd2437757688e97224dbc935e6854988726419 Mon Sep 17 00:00:00 2001 From: dborup Date: Tue, 6 Oct 2026 11:37:31 +0200 Subject: [PATCH 1/2] test(packets,channels,server): cover the #282 follow-ups to #268/#260/#212 Tests first, red on master where there is a behaviour/offset change to pin: - #282 (1) test-issue-282-pktesc-listener.js: drives packets.js init/destroy over several #/packets visits and counts document keydown listeners. Red on master (listeners stack; destroy() leaves one behind), green once _pktEsc is a stable module-level reference removed in destroy(). - #282 (3) test-issue-259-nodes-esc-listener.js: adds the missing _nodesPanelEsc "only act while a detail panel is open" guard case (kills mutant ND). - #282 (6) cmd/server/channel_transport_hash_size_282_test.go: sends TRANSPORT frames through GetChannelMessages so the substr(t.raw_hex, 1, 12) slice is exercised to byte 5; shortening it to 11 turns this red while the FLOOD test stays green. - #282 (7) test-packets.js: buildFieldTable must read the Path Length width from the byte it prints (one offset source), not a second header-derived offset. - #282 (8) test-channels-observed-path-hash-size.js: drops the badge-only cases for the removed renderObservedPathHashBadge export. - #282 (2,4,5) test-issue-282-comment-guards.js: source-text guards for the comment/text corrections, red against the pre-#282 wording. - test-all.sh: register the two new node suites. Co-Authored-By: Claude Opus 4.8 --- .../channel_transport_hash_size_282_test.go | 91 ++++++ test-all.sh | 2 + test-channels-observed-path-hash-size.js | 23 +- test-issue-259-nodes-esc-listener.js | 24 ++ test-issue-282-comment-guards.js | 60 ++++ test-issue-282-pktesc-listener.js | 271 ++++++++++++++++++ test-packets.js | 24 ++ 7 files changed, 477 insertions(+), 18 deletions(-) create mode 100644 cmd/server/channel_transport_hash_size_282_test.go create mode 100644 test-issue-282-comment-guards.js create mode 100644 test-issue-282-pktesc-listener.js diff --git a/cmd/server/channel_transport_hash_size_282_test.go b/cmd/server/channel_transport_hash_size_282_test.go new file mode 100644 index 000000000..325b23a9e --- /dev/null +++ b/cmd/server/channel_transport_hash_size_282_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "fmt" + "testing" + "time" +) + +// #282 (6): GetChannelMessages selects `substr(t.raw_hex, 1, 12)` -- the first +// six bytes -- and feeds it to packetpath.SenderHashSize(). Six bytes is exactly +// the minimum a TRANSPORT route needs: the header byte, four transport-code +// bytes (next/last hop), and then the path-length byte at offset 5. The existing +// sender-hash-size test only sends FLOOD frames, whose path-length byte is at +// offset 1, so it never exercised the transport layout and nothing proved the +// 12-hex-char slice reaches byte 5. Shortening the slice to `substr(…, 1, 11)` +// truncates that byte and this test goes red, while the FLOOD test stays green. +func TestChannelMessagesSenderPathHashSize_TransportRoute_282(t *testing.T) { + for _, makeDB := range []struct { + name string + open func(*testing.T) *DB + }{ + {"v3", func(t *testing.T) *DB { return setupTestDB(t) }}, + {"v2", setupTestDBV2}, + } { + t.Run(makeDB.name, func(t *testing.T) { + db := makeDB.open(t) + defer db.Close() + if _, err := db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('obs-tr', 'Observer', 'AAR')`); err != nil { + t.Fatal(err) + } + // header byte 0x14 = route 0 (TRANSPORT_FLOOD), path-type 5 (hops); + // 0x17 = route 3 (TRANSPORT_DIRECT). Four transport-code bytes follow + // (AABBCCDD), then the path-length byte at offset 5 (hex chars 11-12), + // which only survives because the slice keeps all 12 characters. + for i, tc := range []struct { + raw string + routeType int + want int + }{ + {"14AABBCCDD40", 0, 2}, // transport flood, width bits 1 -> 2 bytes + {"14AABBCCDD80", 0, 3}, // transport flood, width bits 2 -> 3 bytes + {"17AABBCCDD00", 3, 0}, // transport direct, 0x00 marker -> no width + } { + ts := time.Now().UTC().Add(time.Duration(i-5) * time.Minute) + res, err := db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES (?, ?, ?, ?, 5, '{"type":"CHAN","channel":"#transport-size","text":"Alice: message"}', '#transport-size')`, + tc.raw, fmt.Sprintf("transport-size-%d", i), ts.Format(time.RFC3339), tc.routeType) + if err != nil { + t.Fatal(err) + } + id, _ := res.LastInsertId() + if makeDB.name == "v3" { + _, err = db.conn.Exec(`INSERT INTO observations (transmission_id, observer_idx, path_json, timestamp) VALUES (?, 1, '[]', ?)`, id, ts.Unix()) + } else { + _, err = db.conn.Exec(`INSERT INTO observations (transmission_id, observer_id, observer_name, path_json, timestamp) VALUES (?, 'obs-tr', 'Observer', '[]', ?)`, id, ts.Unix()) + } + if err != nil { + t.Fatal(err) + } + } + check := func(source string, messages []map[string]interface{}) { + t.Helper() + if len(messages) != 3 { + t.Fatalf("%s: got %d messages, want 3", source, len(messages)) + } + for _, message := range messages { + hash, _ := message["packetHash"].(string) + var n int + if _, err := fmt.Sscanf(hash, "transport-size-%d", &n); err != nil { + t.Fatalf("%s: unexpected hash %q", source, hash) + } + want := []int{2, 3, 0}[n] + if got := message["senderPathHashSize"]; got != want { + t.Errorf("%s %s: senderPathHashSize=%v, want %d (the path-length byte at offset 5 must survive the raw_hex slice)", source, hash, got, want) + } + } + } + messages, _, err := db.GetChannelMessages("#transport-size", 100, 0) + if err != nil { + t.Fatal(err) + } + check("db", messages) + store := NewPacketStore(db, nil) + if err := store.Load(); err != nil { + t.Fatal(err) + } + messages, _ = store.GetChannelMessages("#transport-size", 100, 0) + check("store", messages) + }) + } +} diff --git a/test-all.sh b/test-all.sh index 6f57b31bd..a180874b7 100755 --- a/test-all.sh +++ b/test-all.sh @@ -61,6 +61,8 @@ run test-node-adverts.js run test-issue-254-affinity-debug-toggle.js run test-issue-258-column-widths.js run test-issue-259-nodes-esc-listener.js +run test-issue-282-pktesc-listener.js +run test-issue-282-comment-guards.js run test-perf-go-runtime.js run test-channel-psk-ux.js run test-channel-sidebar-layout.js diff --git a/test-channels-observed-path-hash-size.js b/test-channels-observed-path-hash-size.js index e5c550e8e..5440da316 100644 --- a/test-channels-observed-path-hash-size.js +++ b/test-channels-observed-path-hash-size.js @@ -120,13 +120,12 @@ vm.runInContext(channelsSource, ctx); const normalize = ctx.window._channelsNormalizeObservedPathHashSizesForTest; const union = ctx.window._channelsUnionObservedPathHashSizesForTest; -const badge = ctx.window._channelsRenderObservedPathHashBadgeForTest; const senderBadge = ctx.window._channelsRenderSenderPathHashBadgeForTest; const merge = ctx.window._channelsMergeWsAppendedIntoRestForTest; const dedup = ctx.window._channelsDeduplicateAndMergeForTest; const fetchAndDecrypt = ctx.window._channelsFetchAndDecryptChannelForTest; -for (const [name, fn] of Object.entries({ normalize, union, badge, senderBadge, merge, dedup, fetchAndDecrypt })) { +for (const [name, fn] of Object.entries({ normalize, union, senderBadge, merge, dedup, fetchAndDecrypt })) { if (typeof fn !== 'function') { console.error('FATAL: missing channels.js test export: ' + name); process.exit(2); @@ -171,22 +170,10 @@ test('unions evidence without mutating either input', () => { assert.deepStrictEqual(b.observed_path_hash_sizes, [2, 1]); }); -test('single evidence badge has exact label and conservative tooltip', () => { - const html = badge({ observedPathHashSizes: [2] }); - assert.match(html, />Observed path hash: 2-byte { - const html = badge({ observed_path_hash_sizes: [3, 1, 2] }); - assert.match(html, />Mixed path hashes: 1\/2\/3-byte { - assert.strictEqual(badge({}), ''); - assert.strictEqual(badge({ observedPathHashSizes: [''] }), ''); -}); +// #282 (8): renderObservedPathHashBadge() (the "Observed path hash" badge) was +// removed from channels.js -- no shipped code rendered it, only this test did. +// Its evidence normalization (including the injection-shaped rejection its badge +// test used to assert) is still covered by the normalize/union cases above. test('sender badge uses only a valid encoded width', () => { assert.match(senderBadge({ senderPathHashSize: 2 }), />Sent with: 2-byte 'one panel close, got ' + JSON.stringify(s.replaceStateWrites)); }); +// #282 (3): _nodesPanelEsc's "only act while a detail panel is open" guard +// (`if (!panel || panel.classList.contains('empty')) return;`) had no test, so +// mutant ND (dropping the guard) survived. With the panel left .empty -- the +// list's cold state -- Escape must do nothing: no closeDetailView(), no +// history.replaceState, no re-render. Without the guard Escape would write +// '#/nodes' and re-render even though no panel is open. +test('Escape does nothing while the list view\'s detail panel is still empty (guard, kills ND)', async () => { + const s = loadNodes(); + s.page.init(s.app, null); + await settle(); + assert.deepStrictEqual(s.errors, [], 'the list view rendered without errors'); + // The panel is seeded .empty (the real markup's panel-right empty) and never + // opened here, so the guard must short-circuit. + assert.strictEqual(s.panel.classList.contains('empty'), true, 'panel starts empty'); + s.replaceStateWrites.length = 0; + s.hashWrites.length = 0; + s.pressEscape(); + assert.deepStrictEqual(s.replaceStateWrites, [], + 'Escape must not close/navigate while the panel is empty: ' + JSON.stringify(s.replaceStateWrites)); + assert.deepStrictEqual(s.hashWrites, [], + 'Escape must not write the hash while the panel is empty: ' + JSON.stringify(s.hashWrites)); + assert.strictEqual(s.panel.classList.contains('empty'), true, 'panel stays empty'); +}); + test('destroy() removes the list view\'s Escape listener', async () => { const s = loadNodes(); s.page.init(s.app, null); diff --git a/test-issue-282-comment-guards.js b/test-issue-282-comment-guards.js new file mode 100644 index 000000000..76e1663ae --- /dev/null +++ b/test-issue-282-comment-guards.js @@ -0,0 +1,60 @@ +/** + * #282 comment/text fixes (points 2, 4, 5). These are documentation corrections + * with no runtime surface, so the regression guard is on the source text itself: + * each assertion is red against the pre-#282 wording and green once corrected, + * and re-introducing the old wording (the mutant) turns it red again. The + * behaviours the comments describe are exercised elsewhere: + * - the #866 deep-link form (point 2) by test-issue-147-packets-url-*; + * - the Time-handle resize (point 4) by test-issue-258-column-widths-e2e.js; + * - a sparse first body creating one MutationObserver (point 5) by + * test-issue-258-column-widths.js ("empty"/"near-empty first body" cases). + * + * Usage: node test-issue-282-comment-guards.js + */ +'use strict'; +const fs = require('fs'); +const assert = require('assert'); + +let passed = 0, failed = 0; +function test(name, fn) { + try { fn(); passed++; console.log(' ✅ ' + name); } + catch (e) { failed++; console.log(' ❌ ' + name + ': ' + e.message); } +} + +const read = (f) => fs.readFileSync(f, 'utf8'); + +console.log('\n=== #282 comment/text fixes ==='); + +// --- Point 2: the #866 deep-link form in packets.js ------------------------ +test('point 2: packets.js names the #866 deep link as #/packets/?obs=', () => { + const src = read('public/packets.js'); + assert(!src.includes('#/packets//'), + 'the stale #/packets// form is still in packets.js'); + assert(src.includes('#/packets/?obs='), + 'the corrected #/packets/?obs= form is missing from packets.js'); +}); + +// --- Point 4: the resize-handle header in the #258 column-widths E2E -------- +test('point 4: the #258 E2E header describes dragging the Time handle right, not the Path handle left', () => { + const src = read('test-issue-258-column-widths-e2e.js'); + const header = src.slice(0, src.indexOf('*/')); + assert(!/Path handle is dragged left/.test(header), + 'the wrong "Path handle is dragged left" description is still in the header'); + assert(/Time handle is dragged right/.test(header), + 'the header should describe dragging the Time handle right (what the test does)'); +}); + +// --- Point 5: the "never create an observer" claim in the #258 unit test ---- +test('point 5: the #258 column-widths header does not claim analytics tables never create an observer', () => { + const src = read('test-issue-258-column-widths.js'); + const header = src.slice(0, src.indexOf('*/')); + // The correction makes the trigger the usable-row count of the first render, + // and spells out that analytics tables are not exempt. + assert(/analytics included/.test(header), + 'the header should state the row-count rule applies to any table, analytics included'); + assert(/does create the one-shot observer/.test(header), + 'the header should state that a sparse first render does create the observer'); +}); + +console.log(`\n${passed} passed, ${failed} failed`); +process.exit(failed === 0 ? 0 : 1); diff --git a/test-issue-282-pktesc-listener.js b/test-issue-282-pktesc-listener.js new file mode 100644 index 000000000..fbca0e90d --- /dev/null +++ b/test-issue-282-pktesc-listener.js @@ -0,0 +1,271 @@ +/** + * #282 (1): the packets page's Escape-to-close-the-detail-panel listener must + * not leak. + * + * public/packets.js registered a fresh `pktEsc` closure on `document` inside + * renderLeft(), and renderLeft() runs on every visit to #/packets (and on every + * filter/region change within a visit). The closure was never removed, so each + * entry stacked one more live `keydown` listener on `document` -- the same leak + * class as nodes.js' #259 nodesEsc/nodesPanelEsc. destroy() never took it off. + * + * The fix makes `_pktEsc` a stable module-level reference: a repeat + * addEventListener of the same function is a DOM no-op (so at most one listener + * survives), and destroy() removes it. + * + * This drives the real packets.js in a vm sandbox whose document + * addEventListener/removeEventListener record the live listeners and + * deduplicate on (type, handler) exactly as the DOM does, then runs the + * router's init/destroy cycle for several #/packets visits and asserts: + * - a single visit leaves exactly one document keydown listener; + * - entering and leaving #/packets several times does not stack listeners; + * - a second render inside the same visit does not add a second listener; + * - destroy() leaves no document keydown listener behind; + * - the listener still works on a page opened after a destroy. + * + * init() renders the list from the asynchronous loadPackets(); the sandbox's + * api() resolves immediately, so draining the microtask queue gets renderLeft() + * -- where the listener is registered -- to run before the assertions. + * + * Usage: node test-issue-282-pktesc-listener.js + */ +'use strict'; +const vm = require('vm'); +const fs = require('fs'); +const assert = require('assert'); + +let passed = 0, failed = 0; +const queue = []; +function test(name, fn) { queue.push([name, fn]); } +async function runQueue() { + for (const [name, fn] of queue) { + try { await fn(); passed++; console.log(' ✅ ' + name); } + catch (e) { failed++; console.log(' ❌ ' + name + ': ' + e.message); } + } +} + +// packets.js' list view renders from the async loadPackets(); the sandbox's +// api() resolves immediately, so draining the microtask queue is enough to get +// renderLeft() to run. Nothing here depends on timers (the sandbox's are +// no-ops, so nothing fires behind the assertions). +async function settle() { + for (let i = 0; i < 80; i++) await Promise.resolve(); +} + +const noop = () => {}; + +// A sandbox that records every document-level listener and the panel closes. +function loadPackets() { + const docListeners = []; // { type, fn } + const errors = []; + let regionChange = null; // the list view's RegionFilter.onChange handler + const pages = {}; + const els = new Map(); + function makeEl(id) { + const classes = new Set(); + const el = { + id, tagName: 'DIV', innerHTML: '', textContent: '', value: '', checked: false, + style: {}, dataset: {}, isConnected: true, children: [], rows: [], + classList: { + add: (c) => classes.add(c), remove: (c) => classes.delete(c), + toggle: (c, on) => (on === undefined ? (classes.has(c) ? classes.delete(c) : classes.add(c)) : (on ? classes.add(c) : classes.delete(c))), + contains: (c) => classes.has(c), + }, + addEventListener: noop, removeEventListener: noop, + querySelector: () => null, querySelectorAll: () => [], + appendChild: (c) => { el.children.push(c); return c; }, + insertBefore: (c) => { el.children.push(c); return c; }, + removeChild: noop, remove: noop, replaceChildren: noop, + setAttribute: (k, v) => { el.dataset[k] = v; }, getAttribute: () => null, + removeAttribute: noop, hasAttribute: () => false, + closest: () => null, focus: noop, blur: noop, scrollIntoView: noop, + getBoundingClientRect: () => ({ width: 0, height: 0, top: 0, left: 0, right: 0, bottom: 0 }), + offsetWidth: 0, offsetHeight: 0, scrollWidth: 0, clientWidth: 0, + }; + return el; + } + function el(id) { + if (!els.has(id)) els.set(id, makeEl(id)); + return els.get(id); + } + const location = { + _hash: '#/packets', + get hash() { return this._hash; }, + set hash(v) { this._hash = v; }, + search: '', pathname: '/', href: 'http://localhost/', origin: 'http://localhost', + }; + const stubComponent = { + init: noop, register: noop, setSelected: noop, offChange: noop, + getRegionParam: () => '', getAreaParam: () => '', getParam: () => '', + getSelected: () => [], get: () => null, + onChange: (fn) => { regionChange = fn; return fn; }, + unhidden: (_t, fn) => { if (typeof fn === 'function') fn(); }, + }; + const ctx = { + window: { + addEventListener: noop, removeEventListener: noop, dispatchEvent: noop, + innerWidth: 1200, innerHeight: 900, location, + // Component singletons packets.js guards on; keep them present but inert. + HopDisplay: { renderHop: () => '', renderPathSymbolsLegend: () => '', _showFromBtn: noop }, + // PacketFilter/FilterUX/ChannelColorPicker/URLState/PacketPathMap stay + // undefined on purpose -- each call site is guarded, so the render path + // that registers _pktEsc runs without them. + }, + document: { + readyState: 'complete', + body: { classList: { add: noop, remove: noop, toggle: noop, contains: () => false }, appendChild: noop }, + createElement: () => makeEl('created'), + head: { appendChild: noop }, + getElementById: (id) => el(id), + // Deduplicated on (type, fn) exactly as the DOM does, so a stable handler + // reference registered twice counts once -- this measures real listener + // count, not addEventListener calls. + addEventListener: (type, fn) => { + if (docListeners.some((l) => l.type === type && l.fn === fn)) return; + docListeners.push({ type, fn }); + }, + removeEventListener: (type, fn) => { + const i = docListeners.findIndex((l) => l.type === type && l.fn === fn); + if (i >= 0) docListeners.splice(i, 1); + }, + querySelector: () => null, + querySelectorAll: () => [], + }, + console: { log: noop, warn: noop, debug: noop, info: noop, + error: (...a) => { errors.push(a.map(String).join(' ')); } }, + Date, Math, Array, Object, String, Number, JSON, RegExp, Error, TypeError, RangeError, + Infinity, NaN, Map, Set, Promise, URLSearchParams, parseInt, parseFloat, isNaN, isFinite, + encodeURIComponent, decodeURIComponent, + setTimeout: noop, clearTimeout: noop, setInterval: noop, clearInterval: noop, + requestAnimationFrame: noop, cancelAnimationFrame: noop, + fetch: () => Promise.resolve({ ok: true, json: () => Promise.resolve({}) }), + performance: { now: () => Date.now() }, + localStorage: { getItem: () => null, setItem: noop, removeItem: noop }, + location, + history: { replaceState: noop, pushState: noop }, + CustomEvent: class CustomEvent {}, + // api() resolves immediately with empty-but-shaped data so loadObservers() + // and loadPackets() complete and renderLeft() runs. + api: () => Promise.resolve({ packets: [], observations: [], messages: [], observers: [], total: 0 }), + invalidateApiCache: noop, + onWS: noop, offWS: noop, debouncedOnWS: () => null, debounce: (fn) => fn, + registerPage: (name, page) => { pages[name] = page; }, + RegionFilter: stubComponent, + AreaFilter: stubComponent, + TableSort: { init: () => ({ destroy: noop }) }, + TableResponsive: { register: noop, unhidden: (_t, fn) => { if (typeof fn === 'function') fn(); } }, + makeColumnsResizable: noop, + CLIENT_TTL: {}, + }; + ctx.window.document = ctx.document; + vm.createContext(ctx); + // Load the real dependency surface packets.js needs, then packets.js itself. + for (const f of ['public/payload-labels.js', 'public/roles.js', 'public/app.js', 'public/packet-helpers.js']) { + vm.runInContext(fs.readFileSync(f, 'utf8'), ctx, { filename: f }); + for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k]; + } + // Keep the heavy layout/sort helpers inert even though app.js defined real + // ones: the listener under test is registered before them, and they want a + // live table DOM this sandbox does not build. + ctx.makeColumnsResizable = noop; + // app.js defines the real registerPage() (into its own lexical `pages`), which + // the dep load above installed over the stub. Restore a capturing registerPage + // so packets.js' registerPage('packets', …) lands in this sandbox's `pages`. + ctx.registerPage = (name, page) => { pages[name] = page; }; + vm.runInContext(fs.readFileSync('public/packets.js', 'utf8'), ctx, { filename: 'public/packets.js' }); + + return { + page: pages.packets, + app: el('app'), + el, + keydownListeners: () => docListeners.filter((l) => l.type === 'keydown'), + errors, + regionChange: () => regionChange && regionChange(), + pressEscape: () => { + for (const l of docListeners.filter((x) => x.type === 'keydown').slice()) { + l.fn({ key: 'Escape', preventDefault: noop, stopPropagation: noop }); + } + }, + }; +} + +console.log('\n=== #282 packets page: one Escape listener, not one per visit ==='); + +test('packets.js registers a page with init and destroy', () => { + const s = loadPackets(); + assert(s.page, 'registerPage("packets", …) ran'); + assert.strictEqual(typeof s.page.init, 'function', 'init'); + assert.strictEqual(typeof s.page.destroy, 'function', 'destroy'); +}); + +test('a single #/packets visit leaves exactly one document keydown listener', async () => { + const s = loadPackets(); + s.page.init(s.app, null); + await settle(); + assert.deepStrictEqual(s.errors, [], 'the list view rendered without errors'); + assert.strictEqual(s.keydownListeners().length, 1, + 'one Escape listener, got ' + s.keydownListeners().length); +}); + +test('entering and leaving #/packets several times does not stack listeners', async () => { + const s = loadPackets(); + for (let i = 0; i < 4; i++) { + s.page.destroy(); // the router destroys the current page first + s.page.init(s.app, null); + await settle(); + } + assert.deepStrictEqual(s.errors, [], 'every render ran without errors'); + const n = s.keydownListeners().length; + assert.strictEqual(n, 1, 'at most one listener after four visits, got ' + n); +}); + +test('a second render inside the same visit does not add a second listener', async () => { + const s = loadPackets(); + s.page.init(s.app, null); + await settle(); + s.regionChange(); // a region change re-runs loadPackets() -> renderLeft() + await settle(); + assert.deepStrictEqual(s.errors, [], 'both renders ran without errors'); + const n = s.keydownListeners().length; + assert.strictEqual(n, 1, 'still one listener after two renders, got ' + n); +}); + +test('destroy() removes the Escape listener', async () => { + const s = loadPackets(); + s.page.init(s.app, null); + await settle(); + s.page.destroy(); + assert.strictEqual(s.keydownListeners().length, 0, + 'got ' + s.keydownListeners().length); +}); + +test('the Escape listener still works on a page opened after a destroy', async () => { + const s = loadPackets(); + s.page.init(s.app, null); + await settle(); + s.page.destroy(); + s.page.init(s.app, null); + await settle(); + assert.strictEqual(s.keydownListeners().length, 1, + 'one listener on the reopened page, got ' + s.keydownListeners().length); +}); + +test('the one listener is the functioning Escape handler: it closes an open panel', async () => { + const s = loadPackets(); + s.page.init(s.app, null); + await settle(); + // closeDetailPanel() marks #pktRight .empty and resets its body. Simulate an + // open detail panel, then fire Escape through the registered listener. + const panel = s.el('pktRight'); + panel.classList.remove('empty'); + panel.innerHTML = '
a selected packet
'; + s.pressEscape(); + assert.strictEqual(panel.classList.contains('empty'), true, + 'Escape should close (mark empty) the open detail panel'); + assert.ok(/Select a packet to view details/.test(panel.innerHTML), + 'Escape should reset the panel body, got: ' + panel.innerHTML); +}); + +runQueue().then(() => { + console.log(`\n${passed} passed, ${failed} failed`); + process.exit(failed === 0 ? 0 : 1); +}); diff --git a/test-packets.js b/test-packets.js index 6d21d33b0..495b95920 100644 --- a/test-packets.js +++ b/test-packets.js @@ -1073,6 +1073,30 @@ console.log('\n=== packets.js: buildFieldTable ==='); assert(result.includes('field-table')); assert(result.includes('0B') || result.includes('0 bytes') || result.includes('??')); }); + + // #282 (7): the Path Length row must read ONE offset source. The byte it + // prints comes from `off` (derived from pkt.route_type); the width label used + // to come from senderPathHashSize(buf), which independently re-derives the + // offset from the raw_hex header byte. A transport route whose stored + // route_type disagrees with its on-wire header route bits splits the two: they + // point at different bytes. The fix reads the width from the byte it prints, + // so the value and its hash_size label always describe the same byte 5. + test('buildFieldTable reads the transport path-length width from the byte it prints (#282 one offset source)', () => { + // route_type 0 = TRANSPORT_FLOOD → path length at byte 5. Header byte 0x15: + // route bits 1 (FLOOD → byte-1 offset), path-type 5 (hops). Byte 1 = 0x40 + // (width bits 1 → 2 bytes), byte 5 = 0x80 (width bits 2 → 3 bytes). The row + // prints byte 5 (0x80), so its label must say hash_size=3 — not 2, which is + // byte 1's width reached via the second (header-derived) offset. + const pkt = { raw_hex: '1540aabbcc80', route_type: 0, payload_type: 5 }; + const result = api.buildFieldTable(pkt, {}, [], []); + const m = /Path Length<\/td>([^<]*)<\/td>([^<]*)<\/td>/.exec(result); + assert(m, 'has a Path Length row, got: ' + result); + assert.strictEqual(m[1], '0x80', 'prints the byte at the route_type offset (byte 5), got: ' + m[1]); + assert(/hash_size=3 bytes?\b/.test(m[2]), + 'width must be read from the printed byte (0x80 → 3 bytes), got: ' + m[2]); + assert(!/hash_size=2\b/.test(m[2]), + 'must not read byte 1 (0x40 → 2 bytes) via a second offset source, got: ' + m[2]); + }); } console.log('\n=== packets.js: _getRowCount ==='); From 275296a150b4c5bb1dcc4f8a27d84cacc101cccc Mon Sep 17 00:00:00 2001 From: dborup Date: Tue, 6 Oct 2026 11:37:44 +0200 Subject: [PATCH 2/2] fix(packets): one pktEsc listener per page and #268/#260/#212 comment/text fixes - #282 (1) public/packets.js: _pktEsc is now a stable module-level handler, registered once in renderLeft() (a repeat add is a DOM no-op) and removed in destroy(), so #/packets no longer stacks a document keydown listener per visit. Same fix shape as nodes.js' #259 nodesPanelEsc. - #282 (7) public/packets.js: the Path Length row in the hex breakdown derives the encoded hash size from the same path-length byte it prints (offset from pkt.route_type) instead of re-deriving the offset inside senderPathHashSize() from the raw_hex header byte. One offset source; for a well-formed frame the result is unchanged, including transport-route frames. - #282 (2) public/packets.js: the #866 deep-link comment now reads #/packets/?obs=, the actual form. - #282 (8) public/channels.js: remove renderObservedPathHashBadge and OBSERVED_PATH_HASH_TOOLTIP -- the only reader was the test export; no shipped code rendered the "Observed path hash" badge. - #282 (4) test-issue-258-column-widths-e2e.js: header now describes dragging the Time handle right (what the test does), not the Path handle left. - #282 (5) test-issue-258-column-widths.js: correct the claim that analytics tables never create an observer -- any table whose first render has < 5 usable rows creates the one-shot MutationObserver. Co-Authored-By: Claude Opus 4.8 --- public/channels.js | 17 ++++------ public/packets.js | 50 +++++++++++++++++++++-------- test-issue-258-column-widths-e2e.js | 4 +-- test-issue-258-column-widths.js | 9 ++++-- 4 files changed, 51 insertions(+), 29 deletions(-) diff --git a/public/channels.js b/public/channels.js index 11668e9c6..660dee042 100644 --- a/public/channels.js +++ b/public/channels.js @@ -11,8 +11,6 @@ let messages = []; let wsHandler = null; - var OBSERVED_PATH_HASH_TOOLTIP = 'Path hash size observed in one or more relayed wire paths for this message. Direct zero-hop copies do not provide hash-size evidence. This does not prove the sender’s permanent configuration.'; - // Normalize the two API spellings at the browser boundary. Only the three // MeshCore path-hash widths are evidence; direct/zero-hop and malformed // values intentionally collapse to an empty set (no badge). @@ -57,14 +55,12 @@ return copy; } - function renderObservedPathHashBadge(message) { - var sizes = normalizeObservedPathHashSizes(message); - if (!sizes.length) return ''; - var label = sizes.length === 1 - ? 'Observed path hash: ' + sizes[0] + '-byte' - : 'Mixed path hashes: ' + sizes.join('/') + '-byte'; - return '' + escapeHtml(label) + ''; - } + // #282 (8): renderObservedPathHashBadge() + OBSERVED_PATH_HASH_TOOLTIP used to + // live here, but the only remaining reader was the test export -- no shipped + // code rendered the "Observed path hash" badge (renderSenderPathHashBadge is + // the one wired into renderMessages). Removed with its test so a dead helper + // cannot drift. normalizeObservedPathHashSizes() stays: the union/merge path + // and the sender badge still use it. // The header records the sender's choice even before a flood has relayed. // Observation-path evidence is retained internally, but is not the label. @@ -2852,7 +2848,6 @@ window._channelsMergeClientChannelStateForTest = mergeClientChannelState; window._channelsNormalizeObservedPathHashSizesForTest = normalizeObservedPathHashSizes; window._channelsUnionObservedPathHashSizesForTest = unionObservedPathHashSizes; - window._channelsRenderObservedPathHashBadgeForTest = renderObservedPathHashBadge; window._channelsRenderSenderPathHashBadgeForTest = renderSenderPathHashBadge; window._channelsDeduplicateAndMergeForTest = deduplicateAndMerge; window._channelsLoadChannelsForTest = loadChannels; diff --git a/public/packets.js b/public/packets.js index 44a289be0..ed073ec3e 100644 --- a/public/packets.js +++ b/public/packets.js @@ -1182,6 +1182,18 @@ let directObsId = null; + // #282 (1): the packets list view's Escape-to-close-the-detail-panel handler. + // It used to be a fresh `pktEsc` closure registered inside renderLeft(), the + // same leak class as nodes.js' #259 nodesPanelEsc: renderLeft() runs on every + // visit to #/packets, so each one stacked another live `keydown` listener on + // document that destroy() never took off. Module level means a repeat add is + // a DOM no-op (at most one listener) and destroy() can remove it. + function _pktEsc(e) { + if (e.key === 'Escape') { + closeDetailPanel(); + } + } + function removeAllByopOverlays() { document.querySelectorAll('.byop-overlay').forEach(function (el) { el.remove(); }); } @@ -1525,6 +1537,9 @@ if (_docActionHandler) { document.removeEventListener('click', _docActionHandler); _docActionHandler = null; } if (_docMenuCloseHandler) { document.removeEventListener('click', _docMenuCloseHandler); _docMenuCloseHandler = null; } if (_docColMenuCloseHandler) { document.removeEventListener('click', _docColMenuCloseHandler); _docColMenuCloseHandler = null; } + // #282 (1): drop the module-level Escape handler so a destroyed packets page + // leaves no live document keydown listener (and never stacks across visits). + document.removeEventListener('keydown', _pktEsc); removeAllByopOverlays(); packets = []; hashIndex = new Map(); selectedId = null; @@ -2377,12 +2392,10 @@ pktBody.addEventListener('keydown', handler); } - // Escape to close packet detail panel - document.addEventListener('keydown', function pktEsc(e) { - if (e.key === 'Escape') { - closeDetailPanel(); - } - }); + // Escape to close packet detail panel. #282 (1): one listener per page, not + // one per renderLeft() -- _pktEsc is a stable module-level reference, so a + // repeat add is a DOM no-op, and destroy() takes it off again. + document.addEventListener('keydown', _pktEsc); renderTableRows(); makeColumnsResizable('#pktTable', 'meshcore-pkt-col-widths'); @@ -2435,7 +2448,7 @@ // children come back on desktop and a phone rotation does not throw the // state away; it is only the rendered slice that leaves them out. This also // covers the first render at a narrow width, e.g. the #866 deep link - // #/packets//, which expands the hash before any render. + // #/packets/?obs=, which expands the hash before any render. function groupIsExpandedInView(hash) { return expandedHashes.has(hash) && !groupRowSelectsOnActivate(); } @@ -3908,15 +3921,24 @@ const pathLenOffset = off; const pathByte0 = parseInt(buf.slice(off * 2, off * 2 + 2), 16); const hashCountVal = isNaN(pathByte0) ? '?' : (pathByte0 & 0x3F); - const encodedHashSize = senderPathHashSize(buf); - // senderPathHashSize collapses every "no width here" case to null. This is - // the byte breakdown, so say WHICH one it is instead of dropping the bits - // on the floor: path bytes that are not hops at all (TRACE carries SNR), - // a width field of 0b11 (there is no 4-byte width -- the backend evidence - // model only knows 1/2/3, see observed_path_hash_sizes.go), or - // sendZeroHop's 0x00 direct marker, which encodes no width by design. const headerByte = parseInt(buf.slice(0, 2), 16); const pathBytesAreHops = !isNaN(headerByte) && ((headerByte >> 2) & 0x0F) !== 9; + // #282 (7): derive the encoded hash size from the SAME path-length byte this + // row shows (pathByte0 at offset `off`, taken from pkt.route_type) rather + // than calling senderPathHashSize(buf), which independently re-derives the + // offset from the raw_hex header byte. For a well-formed frame the two + // offsets agree, but one source means the printed byte and its hash_size + // label can never describe different bytes -- including a transport route + // (path length at byte 5) whose stored route_type and on-wire header route + // bits might disagree. The width semantics are senderPathHashSize's own: + // null for a non-hop path (TRACE carries SNR), for a 0b11 width field (there + // is no 4-byte width -- the backend evidence model only knows 1/2/3, see + // observed_path_hash_sizes.go), and for sendZeroHop's 0x00 direct marker. + let encodedHashSize = null; + if (pathBytesAreHops && !isNaN(pathByte0) && (pathByte0 >> 6) !== 3 && + !(pathByte0 === 0 && (pkt.route_type === 2 || pkt.route_type === 3))) { + encodedHashSize = (pathByte0 >> 6) + 1; + } let pathDescription; if (encodedHashSize != null) { pathDescription = `hash_size=${encodedHashSize} byte${encodedHashSize !== 1 ? 's' : ''}, hash_count=${hashCountVal}`; diff --git a/test-issue-258-column-widths-e2e.js b/test-issue-258-column-widths-e2e.js index 1a34ef741..e505adefa 100644 --- a/test-issue-258-column-widths-e2e.js +++ b/test-issue-258-column-widths-e2e.js @@ -22,8 +22,8 @@ * pinned long name ("KN6PLV-BrkOxfLA-Yebes") shows at least half of it; * - later window changes keep the widths (one re-measure, no per-render work). * Also: saved widths (meshcore-pkt-col-widths) are applied unchanged, and a - * resize handle still resizes and saves (Details is the last column, so the - * Path handle is dragged left, which gives the room to the columns after it). + * resize handle still resizes and saves: the Time handle is dragged right, + * which widens Time and takes the room from the columns after it. * * Usage: BASE_URL=http://localhost:13581 node test-issue-258-column-widths-e2e.js * SCREENSHOT_DIR= also saves screenshots. diff --git a/test-issue-258-column-widths.js b/test-issue-258-column-widths.js index df0f75758..eed1935d3 100644 --- a/test-issue-258-column-widths.js +++ b/test-issue-258-column-widths.js @@ -8,8 +8,13 @@ * 2. Widths are not locked from an (almost) empty body. With fewer than * COL_MEASURE_MIN_ROWS usable rows the table gets provisional widths and the * columns are measured again ONCE, when enough rows arrive -- unless the user - * has saved widths by then. A table measured from a full body gets no - * observer at all, so there is no per-render work. + * has saved widths by then. What governs the one-shot MutationObserver is the + * usable-row count of the FIRST render, not which page the table belongs to: + * a table whose first body already has >= COL_MEASURE_MIN_ROWS usable rows + * gets no observer (no per-render work), but any table -- analytics included + * -- whose first render has fewer than that (e.g. an analytics tab with < 5 + * data rows) does create the one-shot observer. (#282 (5): an earlier claim + * that analytics tables "never create an observer" was wrong on that count.) * 3. Saved widths (localStorage) are applied as before, without measuring. * * Runs the real app.js in a vm sandbox against a minimal fake table DOM. A