fix(packets): one pktEsc listener per page and #268/#260/#212 follow-ups (#282)

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)
+		})
+	}
+}

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 '<span class="ch-path-hash-badge" title="' + escapeHtml(OBSERVED_PATH_HASH_TOOLTIP) + '">' + escapeHtml(label) + '</span>';
-  }
+  // #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;

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/<hash>/<obs>, which expands the hash before any render.
+  // #/packets/<hash>?obs=<id>, 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}`;

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

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</);
-  assert.match(html, /title="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\."/);
-});
-
-test('mixed evidence badge is sorted and compact', () => {
-  const html = badge({ observed_path_hash_sizes: [3, 1, 2] });
-  assert.match(html, />Mixed path hashes: 1\/2\/3-byte</);
-  assert.ok(!html.includes('undefined'));
-});
-
-test('unknown evidence renders no badge and cannot inject markup', () => {
-  assert.strictEqual(badge({}), '');
-  assert.strictEqual(badge({ observedPathHashSizes: ['<img src=x onerror=alert(1)>'] }), '');
-});
+// #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</);

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=<dir> also saves screenshots.

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

test-issue-259-nodes-esc-listener.js

@@ -290,6 +290,30 @@ test('one Escape closes the list view\'s detail panel exactly once', async () =>
     '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);

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/<hash>?obs=<id>', () => {
+  const src = read('public/packets.js');
+  assert(!src.includes('#/packets/<hash>/<obs>'),
+    'the stale #/packets/<hash>/<obs> form is still in packets.js');
+  assert(src.includes('#/packets/<hash>?obs=<id>'),
+    'the corrected #/packets/<hash>?obs=<id> 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);