fix(live): region filter wipes feed — parse {observers:[...]} response (#1136) (#1140)

## Summary Fixes #1136. The live page region filter wiped all packets, polylines, and feed entries the moment any region was selected. Root cause: `public/live.js` parsed `/api/observers` as a top-level array, but the endpoint returns `{observers:[...], server_time:"..."}` — so `observerIataMap` stayed empty and `packetMatchesRegion` rejected every packet. This was a regression introduced in #1080 (live region filter) after the typed-struct refactor wrapped the observer list in `ObserverListResponse` (cmd/server/types.go). ## Fix - Extracted the parse into `buildObserverIataMap(data)` — a pure helper that accepts both the real `{observers:[...]}` shape and a bare array (defensive). Skips observers with no IATA so the result is a direct lookup map. - `initLiveRegionFilter` now uses the helper, so the map is populated on first paint. - Exposed `_liveBuildObserverIataMap` and `_liveGetObserverIataMap` on `window` for tests (read-only — no behavior change). Backend untouched — the API shape is correct. ## Tests (red → green) **Red commit** (`test(live): failing tests for #1136 region filter wipes feed`): - `test-issue-1136-observer-iata-map.js` — failed at "helper must be exposed" assertion (parser was inlined, not extracted). - `test-issue-1136-live-region-e2e.js` — Playwright. Loads `/#/live`, queries `/api/observers` to discover an SJC observer, asserts the live module's `observerIataMap` is populated, selects SJC via `RegionFilter.setSelected`, pushes a fixture packet through `_liveBufferPacket`, and asserts a `.live-feed-item[data-hash=...]` renders. Failed at both the "map populated" and "feed renders" assertions — exactly the user-reported symptom. - Both wired into `.github/workflows/deploy.yml` (unit step + Playwright step). **Green commit** (`fix(live): parse {observers:[...]} ...`): all five unit assertions + all five E2E assertions pass. Existing `test-live-region-filter.js` from #1080 still passes (no behavior change to `packetMatchesRegion`). ## Verification (local) ``` node test-issue-1136-observer-iata-map.js # 5/5 pass node test-live-region-filter.js # 9/9 pass (regression guard) BASE_URL=http://localhost:13581 \ CHROMIUM_PATH=/usr/bin/chromium \ node test-issue-1136-live-region-e2e.js # 5/5 pass against fixture DB ``` ## Scope - One frontend file changed (`public/live.js`). - Two new tests + 2 lines of CI wiring. - No backend changes. - No refactor of unrelated `live.js` code. - Out of scope: #1108 (the related "hide nodes not seen by region" feature request) is intentionally not addressed here. Fixes #1136 --------- Co-authored-by: corescope-bot <bot@corescope.local>
2026-05-11 03:07:01 +00:00 · 2026-05-06 21:24:32 -07:00
parent cd238d366f
commit eddca7acde
4 changed files with 300 additions and 10 deletions
@@ -92,6 +92,7 @@ jobs:
          node test-packet-filter-time.js
          node test-channel-decrypt-insecure-context.js
          node test-live-region-filter.js
+          node test-issue-1136-observer-iata-map.js
          node test-channel-qr.js
          node test-channel-qr-wiring.js
          node test-channel-modal-ux.js
@@ -232,6 +233,7 @@ jobs:
          BASE_URL=http://localhost:13581 node test-issue-1122-packets-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1128-packets-layout-e2e.js 2>&1 | tee -a e2e-output.txt
          BASE_URL=http://localhost:13581 node test-issue-1128-multi-viewport-e2e.js 2>&1 | tee -a e2e-output.txt
+          BASE_URL=http://localhost:13581 node test-issue-1136-live-region-e2e.js 2>&1 | tee -a e2e-output.txt
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-rebrand-e2e.js 2>&1 | tee -a e2e-output.txt
          CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-logo-theme-e2e.js 2>&1 | tee -a e2e-output.txt

@@ -52,6 +52,28 @@
    return false;
  }
  function setObserverIataMap(m) { observerIataMap = m || {}; }
+
+  /**
+   * Build observer_id → IATA map from the /api/observers response.
+   * The endpoint returns `{ observers: [...], server_time: "..." }`
+   * (cmd/server/types.go ObserverListResponse). Defensive: also accepts
+   * a bare array in case the API shape ever changes back, and ignores
+   * observers without an IATA. Returns a plain object (used as a hash).
+   * Exported for tests via window._liveBuildObserverIataMap.
+   * Fixes #1136 (regression introduced in #1080 which assumed array shape).
+   */
+  function buildObserverIataMap(data) {
+    var list = null;
+    if (Array.isArray(data)) list = data;
+    else if (data && Array.isArray(data.observers)) list = data.observers;
+    var m = {};
+    if (!list) return m;
+    for (var i = 0; i < list.length; i++) {
+      var o = list[i];
+      if (o && o.id != null && o.iata) m[o.id] = o.iata;
+    }
+    return m;
+  }
  let rainCanvas = null, rainCtx = null, rainDrops = [], rainRAF = null;
  const propagationBuffer = new Map(); // hash -> {timer, packets[]}
  let _onResize = null;
@@ -1042,16 +1064,13 @@
    (function initLiveRegionFilter() {
      var rfEl = document.getElementById('liveRegionFilter');
      if (!rfEl || !window.RegionFilter) return;
-      // Fetch observer roster to build observer_id → IATA map
-      fetch('/api/observers').then(function(r) { return r.json(); }).then(function(list) {
-        var m = {};
-        if (Array.isArray(list)) {
-          for (var i = 0; i < list.length; i++) {
-            var o = list[i];
-            if (o && o.id != null && o.iata) m[o.id] = o.iata;
-          }
-        }
-        setObserverIataMap(m);
+      // Fetch observer roster to build observer_id → IATA map.
+      // /api/observers returns `{observers:[...], server_time:"..."}`
+      // (cmd/server/types.go ObserverListResponse) — NOT a top-level array.
+      // Bug #1136: previously parsed as array → map empty → region filter
+      // dropped every packet.
+      fetch('/api/observers').then(function(r) { return r.json(); }).then(function(data) {
+        setObserverIataMap(buildObserverIataMap(data));
      }).catch(function() { /* leave map empty; filter will hide all when active */ });
      RegionFilter.init(rfEl, { dropdown: true });
      regionFilterChangeHandler = RegionFilter.onChange(function() { /* selection persisted by RegionFilter; future packets reflect it */ });
@@ -2127,6 +2146,8 @@
  window._liveGetNodeFilterKeys = function() { return nodeFilterKeys; };
  window._livePacketMatchesRegion = packetMatchesRegion;
  window._liveSetObserverIataMap = setObserverIataMap;
+  window._liveBuildObserverIataMap = buildObserverIataMap;
+  window._liveGetObserverIataMap = function() { return observerIataMap; };
  window._liveSetNodeFilter = setNodeFilter;
  window._liveFormatLiveTimestampHtml = formatLiveTimestampHtml;
  window._liveResolveHopPositions = resolveHopPositions;
@@ -0,0 +1,134 @@
+/**
+ * E2E (#1136): Live page region filter must NOT wipe all packets and lines.
+ *
+ * Regression introduced in #1080 — `public/live.js` parsed `/api/observers`
+ * as if it were a top-level array, but the endpoint returns
+ * `{observers: [...], server_time: ...}`. Result: `observerIataMap` stayed
+ * empty and `packetMatchesRegion` returned false for EVERY packet whenever
+ * any region was selected — so no markers, no polylines, no feed entries.
+ *
+ * This test:
+ *   1. Loads /#/live against the fixture DB.
+ *   2. Waits for the observer roster to load and verifies the live module
+ *      has a populated observer_id → IATA map (proves the parse path works).
+ *   3. Programmatically selects a region (SJC) that we know maps to fixture
+ *      observers (test-fixtures/e2e-fixture.db has multiple observers in
+ *      SJC, OAK, MRY, SFO).
+ *   4. Synthesizes a packet whose observer_id IS in the SJC region and
+ *      pushes it through the same path live websocket packets take.
+ *   5. Asserts at least one `.live-feed-item` is rendered for that hash.
+ *
+ * Before the fix this test FAILS at assertion 2 (map empty) AND at
+ * assertion 5 (feed never renders the packet). After the fix both pass.
+ *
+ * Usage: BASE_URL=http://localhost:13581 node test-issue-1136-live-region-e2e.js
+ */
+'use strict';
+const { chromium } = require('playwright');
+
+const BASE = process.env.BASE_URL || 'http://localhost:13581';
+
+let passed = 0, failed = 0;
+async function step(name, fn) {
+  try { await fn(); passed++; console.log('  \u2713 ' + name); }
+  catch (e) { failed++; console.error('  \u2717 ' + name + ': ' + e.message); }
+}
+function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); }
+
+(async () => {
+  const browser = await chromium.launch({
+    headless: true,
+    executablePath: process.env.CHROMIUM_PATH || undefined,
+    args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'],
+  });
+  const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } });
+  const page = await ctx.newPage();
+  page.setDefaultTimeout(15000);
+  page.on('pageerror', (e) => console.error('[pageerror]', e.message));
+
+  console.log('\n=== #1136 live region filter E2E against ' + BASE + ' ===');
+
+  // Discover an observer_id in SJC from the API (drives test from real data).
+  let sjcObserverId = null;
+  let allObservers = [];
+  await step('GET /api/observers returns {observers:[...]} shape with SJC entries', async () => {
+    const res = await page.request.get(BASE + '/api/observers');
+    assert(res.ok(), 'API returned non-OK: ' + res.status());
+    const body = await res.json();
+    assert(body && Array.isArray(body.observers), 'response must have .observers array (the bug-1136 root cause)');
+    allObservers = body.observers;
+    const sjc = body.observers.filter(function (o) { return o && o.iata === 'SJC' && o.id; });
+    assert(sjc.length > 0, 'fixture must contain at least one SJC observer (got ' + sjc.length + ')');
+    sjcObserverId = sjc[0].id;
+  });
+
+  await step('navigate to /#/live and wait for live module to register', async () => {
+    // Pre-clear region selection so it starts unrestricted.
+    await page.addInitScript(() => {
+      try { localStorage.removeItem('meshcore-region-filter'); } catch (e) {}
+    });
+    await page.goto(BASE + '/#/live', { waitUntil: 'domcontentloaded' });
+    await page.waitForFunction(() => !!(window._liveBufferPacket && window.RegionFilter), { timeout: 15000 });
+  });
+
+  await step('observer iata map is POPULATED after init fetch (regression #1136)', async () => {
+    const exposed = await page.evaluate(() => typeof window._liveGetObserverIataMap);
+    assert(exposed === 'function', '_liveGetObserverIataMap must be exposed as a function (regression: not wired up)');
+    // Wait for fetch + setObserverIataMap to land.
+    await page.waitForFunction(() => {
+      const m = window._liveGetObserverIataMap && window._liveGetObserverIataMap();
+      return m && Object.keys(m).length > 0;
+    }, { timeout: 8000 }).catch(() => {});
+    const sample = await page.evaluate((oid) => {
+      const m = window._liveGetObserverIataMap();
+      return { size: Object.keys(m).length, iataForOid: m[oid] || null };
+    }, sjcObserverId);
+    assert(sample.size > 0, 'observerIataMap should be populated from /api/observers (was empty — #1136 bug)');
+    assert(sample.iataForOid === 'SJC', 'observerIataMap[' + sjcObserverId + '] should be "SJC", got ' + sample.iataForOid);
+  });
+
+  await step('select SJC region in RegionFilter, verify selection took effect', async () => {
+    await page.evaluate(() => {
+      window.RegionFilter.setSelected(['SJC']);
+    });
+    const sel = await page.evaluate(() => window.RegionFilter.getSelected());
+    assert(Array.isArray(sel) && sel.indexOf('SJC') !== -1, 'RegionFilter selected should include SJC, got ' + JSON.stringify(sel));
+  });
+
+  await step('packet with SJC observer renders to live feed when SJC region selected', async () => {
+    const targetHash = 'fixture-1136-' + Date.now().toString(16);
+    await page.evaluate(function (args) {
+      const pkt = {
+        id: 9999991136,
+        hash: args.hash,
+        raw_hex: '00',
+        path_json: '[]',
+        observer_id: args.oid,
+        observer_name: 'fixture-observer',
+        timestamp: new Date().toISOString(),
+        snr: 5, rssi: -90,
+        decoded: {
+          header: { payloadTypeName: 'GRP_TXT' },
+          payload: { text: 'region-1136-probe' },
+          path: { hops: [] },
+        },
+      };
+      // Push through the same buffer entry point the WS handler uses.
+      window._liveBufferPacket(pkt);
+    }, { hash: targetHash, oid: sjcObserverId });
+
+    // Allow the (non-realistic-propagation) immediate renderPacketTree to land.
+    await page.waitForFunction((h) => {
+      return !!document.querySelector('.live-feed-item[data-hash="' + h + '"]');
+    }, targetHash, { timeout: 5000 }).catch(() => {});
+
+    const found = await page.evaluate((h) => !!document.querySelector('.live-feed-item[data-hash="' + h + '"]'), targetHash);
+    assert(found, 'expected .live-feed-item[data-hash=' + targetHash + '] to render with SJC selected (#1136: filter wiped feed)');
+  });
+
+  await page.evaluate(() => { try { window.RegionFilter.setSelected([]); } catch(e) {} });
+  await browser.close();
+
+  console.log('\n--- ' + passed + ' passed, ' + failed + ' failed ---\n');
+  process.exit(failed > 0 ? 1 : 0);
+})().catch((e) => { console.error(e); process.exit(1); });
@@ -0,0 +1,133 @@
+/* Unit test (#1136): live.js must parse /api/observers correctly.
+ *
+ * Regression: PR #1080 wrote `if (Array.isArray(list))` and treated the
+ * response as a top-level array. The actual /api/observers shape is
+ * `{ observers: [...], server_time: "..." }` (cmd/server/types.go
+ * ObserverListResponse). Result: observerIataMap stays empty and ANY
+ * region selection drops every packet.
+ *
+ * This test loads live.js into a vm sandbox and asserts that the exposed
+ * builder helper produces a populated map from the realistic API shape.
+ */
+'use strict';
+const vm = require('vm');
+const fs = require('fs');
+const assert = require('assert');
+
+let passed = 0, failed = 0;
+function test(name, fn) {
+  try { fn(); passed++; console.log('  \u2705 ' + name); }
+  catch (e) { failed++; console.log('  \u274C ' + name + ': ' + e.message); }
+}
+
+function makeSandbox() {
+  const ctx = {
+    window: { addEventListener: () => {}, dispatchEvent: () => {}, devicePixelRatio: 1 },
+    document: {
+      readyState: 'complete',
+      createElement: () => ({ style: {}, classList: { add(){}, remove(){}, contains(){return false;} }, setAttribute(){}, addEventListener(){}, getContext: () => ({clearRect(){},fillRect(){},beginPath(){},arc(){},fill(){},scale(){},fillText(){}}) }),
+      head: { appendChild: () => {} },
+      getElementById: () => null,
+      addEventListener: () => {},
+      querySelectorAll: () => [], querySelector: () => null,
+      createElementNS: () => ({ setAttribute(){} }),
+      documentElement: { getAttribute: () => null, setAttribute: () => {}, dataset: {} },
+      body: { appendChild: () => {}, removeChild: () => {}, contains: () => false },
+      hidden: false,
+    },
+    console, Date, Infinity, Math, Array, Object, String, Number, JSON, RegExp,
+    Error, TypeError, Map, Set, Promise, URLSearchParams,
+    parseInt, parseFloat, isNaN, isFinite,
+    encodeURIComponent, decodeURIComponent,
+    setTimeout: () => 0, clearTimeout: () => {},
+    setInterval: () => 0, clearInterval: () => {},
+    fetch: () => Promise.resolve({ json: () => Promise.resolve({}) }),
+    performance: { now: () => Date.now() },
+    requestAnimationFrame: () => 0,
+    cancelAnimationFrame: () => {},
+    localStorage: (() => { const s = {}; return { getItem: k => s[k] !== undefined ? s[k] : null, setItem: (k,v) => { s[k] = String(v); }, removeItem: k => { delete s[k]; } }; })(),
+    location: { hash: '', protocol: 'https:', host: 'localhost' },
+    CustomEvent: class CustomEvent {},
+    addEventListener: () => {}, dispatchEvent: () => {},
+    getComputedStyle: () => ({ getPropertyValue: () => '' }),
+    matchMedia: () => ({ matches: false, addEventListener: () => {} }),
+    navigator: {}, visualViewport: null,
+    MutationObserver: function() { this.observe=()=>{}; this.disconnect=()=>{}; },
+    WebSocket: function() { this.close=()=>{}; },
+    IATA_COORDS_GEO: {},
+    L: {
+      circleMarker: () => ({addTo(){return this;},bindTooltip(){return this;},on(){return this;},setRadius(){},setStyle(){},setLatLng(){},getLatLng(){return{lat:0,lng:0};},remove(){}}),
+      polyline: () => ({addTo(){return this;},setStyle(){},remove(){}}),
+      polygon: () => ({addTo(){return this;},remove(){}}),
+      map: () => ({setView(){return this;},addLayer(){return this;},on(){return this;},getZoom(){return 11;},getCenter(){return{lat:0,lng:0};},getBounds(){return{contains:()=>true};},fitBounds(){return this;},invalidateSize(){},remove(){},hasLayer(){return false;},removeLayer(){}}),
+      layerGroup: () => ({addTo(){return this;},addLayer(){},removeLayer(){},clearLayers(){},hasLayer(){return true;},eachLayer(){}}),
+      tileLayer: () => ({addTo(){return this;}}),
+      control: { attribution: () => ({addTo(){}}) },
+      DomUtil: { addClass(){}, removeClass(){} },
+    },
+    registerPage: () => {}, onWS: () => {}, offWS: () => {}, connectWS: () => {},
+    api: () => Promise.resolve([]), invalidateApiCache: () => {},
+    favStar: () => '', bindFavStars: () => {},
+    getFavorites: () => [], isFavorite: () => false,
+    HopResolver: { init(){}, resolve: () => ({}), ready: () => false },
+    MeshAudio: null,
+    RegionFilter: { init(){}, getSelected: () => null, onChange: () => {}, offChange: () => {}, regionQueryString: () => '', getRegionParam: () => '' },
+  };
+  vm.createContext(ctx);
+  return ctx;
+}
+
+function load(ctx, file) {
+  vm.runInContext(fs.readFileSync(file, 'utf8'), ctx);
+  for (const k of Object.keys(ctx.window)) ctx[k] = ctx.window[k];
+}
+
+console.log('\n=== live.js: /api/observers parse (#1136) ===');
+const ctx = makeSandbox();
+load(ctx, 'public/roles.js');
+load(ctx, 'public/live.js');
+
+const build = ctx.window._liveBuildObserverIataMap;
+assert.ok(build, '_liveBuildObserverIataMap must be exposed (regression: missing parser helper)');
+
+const realShape = {
+  observers: [
+    { id: 'OBS1', iata: 'SJC', name: 'A' },
+    { id: 'OBS2', iata: 'OAK', name: 'B' },
+    { id: 'OBS3', iata: 'SFO', name: 'C' },
+    { id: 'OBS4', iata: null, name: 'no-iata' },
+  ],
+  server_time: '2026-05-07T00:00:00Z',
+};
+
+test('parses {observers:[...], server_time} response and populates map', () => {
+  const m = build(realShape);
+  assert.strictEqual(m.OBS1, 'SJC');
+  assert.strictEqual(m.OBS2, 'OAK');
+  assert.strictEqual(m.OBS3, 'SFO');
+});
+
+test('skips observers without iata', () => {
+  const m = build(realShape);
+  assert.ok(!('OBS4' in m), 'observers with null iata should not be in map');
+});
+
+test('returns empty map for null/undefined input', () => {
+  assert.strictEqual(Object.keys(build(null)).length, 0);
+  assert.strictEqual(Object.keys(build(undefined)).length, 0);
+});
+
+test('returns empty map when observers field is missing', () => {
+  assert.strictEqual(Object.keys(build({ server_time: 'x' })).length, 0);
+});
+
+test('back-compat: also accepts a top-level array (defensive)', () => {
+  // If the API shape ever changes back, don\'t silently break.
+  const m = build([{ id: 'X1', iata: 'LAX' }]);
+  assert.strictEqual(m.X1, 'LAX');
+});
+
+console.log('\n' + '='.repeat(40));
+console.log('  observer iata map tests: ' + passed + ' passed, ' + failed + ' failed');
+console.log('='.repeat(40) + '\n');
+if (failed > 0) process.exit(1);