fix(packets): preserve observation selection in detail URLs

public/packets.js

@@ -774,7 +774,11 @@
     if (m && m[1]) subpath = m[1];
     // Don't double-encode filters.hash when it's already the path segment.
     var skipHash = !!(filters.hash && subpath === '/' + filters.hash);
-    history.replaceState(null, '', '#/packets' + subpath + buildPacketsQuery(savedTimeWindowMin, RegionFilter.getRegionParam(), skipHash));
+    var query = buildPacketsQuery(savedTimeWindowMin, RegionFilter.getRegionParam(), skipHash);
+    // Observation selection belongs to the current detail route, not filters.
+    var obs = subpath ? getHashParams().get('obs') : null;
+    if (obs) query += (query ? '&' : '?') + 'obs=' + encodeURIComponent(obs);
+    history.replaceState(null, '', '#/packets' + subpath + query);
     // Update clear-filters button visibility
     var cb = document.getElementById('clearFiltersBtn');
     if (cb) {
@@ -1174,6 +1178,7 @@
 
     // Read URL params (router strips query from routeParam; read from location.hash)
     var _initUrlParams = getHashParams();
+    directObsId = _initUrlParams.get('obs');
     var _urlTimeWindow = Number(_initUrlParams.get('timeWindow'));
     if (Number.isFinite(_urlTimeWindow) && _urlTimeWindow > 0) {
       savedTimeWindowMin = _urlTimeWindow;
@@ -1277,10 +1282,13 @@
     // If linked directly to a packet by ID, load its detail and filter list
     if (directPacketId) {
       const pktId = Number(directPacketId);
+      const obsTarget = directObsId;
       directPacketId = null;
+      directObsId = null;
       try {
         const data = await api(`/packets/${pktId}`);
         if (gen !== initGeneration) return;
+        selectedObservationId = obsTarget;
         if (data.packet?.hash) {
           filters.hash = data.packet.hash;
           const hashInput = document.getElementById('fHash');
@@ -1300,7 +1308,7 @@
             const newHops = hops.filter(h => !(h in hopNameCache));
             if (newHops.length) await resolveHops(newHops);
           } catch {}
-          await renderDetail(content, data);
+          await renderDetail(content, data, obsTarget);
           initPanelResize();
         }
       } catch {}

tests/e2e/test-filter-ux-e2e.js

@@ -173,6 +173,66 @@ function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); }
     await page.evaluate(() => localStorage.removeItem('corescope_saved_filters_v1'));
   });
 
+  // Reuse the real three-observation transmission seeded by deploy.yml for
+  // #1486. Missing fixture data must fail, never silently skip this regression.
+  for (const routeKind of ['hash', 'id']) {
+    await step('#2091: ' + routeKind + ' observation deep link survives filters and refresh', async () => {
+      const response = await ctx.request.get(BASE + '/api/packets/fae0c9e6d357a814');
+      assert(response.ok(), '#1486 grouped packet fixture is required');
+      const detail = await response.json();
+      assert(detail.packet && detail.observations && detail.observations.length > 1,
+        'fixture must expose a packet with at least two observations');
+      const observation = detail.observations[1];
+      assert(String(observation.id) !== String(detail.observations[0].id), 'observation IDs must differ');
+
+      const detailContext = await browser.newContext({ viewport: { width: 1600, height: 1000 } });
+      try {
+        const detailPage = await detailContext.newPage();
+        const route = routeKind === 'hash' ? detail.packet.hash : 'id/' + detail.packet.id;
+        await detailPage.goto(BASE + '/#/packets/' + route + '?obs=' + observation.id,
+          { waitUntil: 'domcontentloaded' });
+
+        async function assertSelection(stage) {
+          await detailPage.waitForSelector('#pktRight .detail-obs-row.observation-current', { state: 'attached' });
+          const selected = await detailPage.locator('#pktRight .observation-current').getAttribute('data-obs-id');
+          assert(selected === String(observation.id), stage + ': selected observation changed to ' + selected);
+          const hash = new URL(detailPage.url()).hash;
+          const params = new URLSearchParams(hash.split('?')[1] || '');
+          assert(params.get('obs') === String(observation.id), stage + ': obs missing from ' + hash);
+          assert(params.getAll('obs').length === 1, stage + ': duplicated observation parameter');
+          if (routeKind === 'hash') assert(!params.has('hash'), stage + ': hash duplicated in query');
+        }
+
+        await assertSelection('initial load');
+        await detailPage.click('#typeTrigger');
+        await detailPage.locator('#typeMenu input[data-type-id="' + detail.packet.payload_type + '"]').check();
+        assert(await detailPage.evaluate(() => localStorage.getItem('meshcore-type-filter')) === String(detail.packet.payload_type),
+          'type filter must apply the requested selection');
+        await assertSelection('type filter');
+        await detailPage.click('#observerTrigger');
+        await detailPage.locator('#observerMenu input[data-obs-id=' + JSON.stringify(observation.observer_id) + ']').check();
+        assert(new URLSearchParams(new URL(detailPage.url()).hash.split('?')[1]).get('observer') === String(observation.observer_id),
+          'observer filter must update the URL to the requested observer');
+        await assertSelection('observer filter');
+        await detailPage.click('#observerTrigger');
+        await detailPage.selectOption('#fTimeWindow', '60');
+        assert(new URLSearchParams(new URL(detailPage.url()).hash.split('?')[1]).get('timeWindow') === '60',
+          'time-window filter must update the URL to 60 minutes');
+        await assertSelection('time-window filter');
+        await detailPage.reload({ waitUntil: 'load' });
+        await assertSelection('refresh');
+        await detailPage.click('#clearFiltersBtn');
+        await assertSelection('Clear filters');
+        const params = new URLSearchParams(new URL(detailPage.url()).hash.split('?')[1]);
+        assert(!params.has('observer') && !params.has('timeWindow'), 'Clear must still remove filters');
+        await detailPage.reload({ waitUntil: 'load' });
+        await assertSelection('refresh after Clear');
+      } finally {
+        await detailContext.close();
+      }
+    });
+  }
+
   await browser.close();
 
   console.log(`\n=== Results: passed ${passed} failed ${failed} ===`);

tests/unit/test-issue-2012-clear-filters-selection.js

@@ -39,6 +39,9 @@ const multiSelectSrc = slice('// --- Observer multi-select ---', '// --- Channel
 const clearSrc = slice('// --- Clear filters button ---', '// Show clear button if page loaded');
 // The real updatePacketsUrl(), which shows or hides the Clear button.
 const urlSrc = slice('function buildPacketsQuery(', 'let filtersBuilt = false;');
+const appSrc = fs.readFileSync(REPO_ROOT + '/public/app.js', 'utf-8');
+const hashParamsSrc = appSrc.match(/function getHashParams\(\) \{[\s\S]*?\n\}/)[0];
+const initParamsSrc = slice('// Parse ?obs=OBSERVER_ID from routeParam', 'app.innerHTML =');
 
 function makeEl(id) {
   const listeners = {};
@@ -69,7 +72,7 @@ function makeEl(id) {
   return el;
 }
 
-function setup() {
+function setup(hash = '#/packets', initialFilters = {}) {
   const elements = {};
   // #observerList and #observerSearchInput are not in packets.js yet. The
   // observer search PR (#1884) renders the observer rows into #observerList
@@ -93,21 +96,22 @@ function setup() {
   const observers = [{ id: 'obsA', name: 'Observer A' }, { id: 'obsB', name: 'Observer B' }];
   const observerMap = new Map(observers.map((o) => [o.id, o]));
   const SHORT_BY_ID = { 4: 'ADVERT', 5: 'GRP_TXT' };
-  const filters = { myNodes: false };
+  const filters = { myNodes: false, ...initialFilters };
   const escapeHtml = (s) => String(s);
   const RegionFilter = { setSelected() {}, getRegionParam: () => '' };
-  const location = { hash: '#/packets' };
-  const history = { replaceState() {} };
+  const location = { hash };
+  const history = { replaceState(_state, _title, url) { location.hash = url; } };
   const noop = () => {};
 
   const run = new Function(
     'filters', 'observers', 'observerMap', 'SHORT_BY_ID', 'escapeHtml', 'document', 'localStorage',
     'RegionFilter', 'renderTableRows', 'loadPackets',
     '_rebuildObserverMenu', '_observerFilterSet', 'savedTimeWindowMin', 'DEFAULT_TIME_WINDOW',
     'location', 'history', 'window', '_packetSortColumn', '_packetSortDirection',
-    urlSrc + '\n' + multiSelectSrc + '\n' + clearSrc
+    hashParamsSrc + '\n' + urlSrc + '\n' + multiSelectSrc + '\n' + clearSrc + '\n' +
+    'updatePacketsUrl(); return { updatePacketsUrl, buildPacketsQuery };'
   );
-  run(filters, observers, observerMap, SHORT_BY_ID, escapeHtml, document, localStorage,
+  const actions = run(filters, observers, observerMap, SHORT_BY_ID, escapeHtml, document, localStorage,
     RegionFilter, noop, noop, null, null, 15, 15,
     location, history, {}, null, null);
 
@@ -128,7 +132,7 @@ function setup() {
     return boxes(menuId).find((c) => c.dataset[attr] === '__all__');
   }
   return {
-    filters, elements, storage, boxes,
+    filters, elements, storage, boxes, location, ...actions,
     clear: () => elements.clearFiltersBtn.fire('click'),
     pickObserver: (id) => toggle('observerMenu', 'obsId', id, true),
     pickType: (id) => toggle('typeMenu', 'typeId', id, true),
@@ -204,6 +208,79 @@ test('observer only: Clear button is shown once an observer is picked, hidden af
   assert.strictEqual(s.elements.clearFiltersBtn.style.display, 'none', 'Clear button still shown after Clear');
 });
 
+for (const route of ['#/packets/aabbccddeeff0011', '#/packets/id/42']) {
+  test('#2091: initial URL rewrite preserves observation on ' + route, () => {
+    const s = setup(route + '?obs=123');
+    assert.strictEqual(s.location.hash, route + '?obs=123');
+    assert.strictEqual(s.elements.clearFiltersBtn.style.display, 'none', 'observation is not a filter');
+  });
+}
+
+test('#2091: type and observer changes preserve detail selection and canonical hash', () => {
+  const route = '#/packets/aabbccddeeff0011';
+  const s = setup(route + '?obs=123', { hash: 'aabbccddeeff0011' });
+  s.pickType('4');
+  s.pickObserver('obsA');
+  s.pickObserver('obsB');
+  const params = new URLSearchParams(s.location.hash.split('?')[1]);
+  assert.strictEqual(params.get('obs'), '123');
+  assert.strictEqual(params.getAll('obs').length, 1);
+  assert.strictEqual(params.get('observer'), 'obsA,obsB');
+  assert.strictEqual(params.has('hash'), false, 'path hash must not be duplicated as a filter');
+  assert.strictEqual(s.location.hash.split('?')[0], route);
+});
+
+test('#2091: Clear removes filters but keeps the current observation detail', () => {
+  const route = '#/packets/aabbccddeeff0011';
+  const s = setup(route + '?obs=123', { hash: 'aabbccddeeff0011' });
+  s.pickObserver('obsA');
+  s.pickType('4');
+  s.clear();
+  assert.strictEqual(s.location.hash, route + '?obs=123');
+  assert.strictEqual(s.elements.clearFiltersBtn.style.display, 'none');
+  assert.strictEqual(s.observerAllRow().checked, true);
+  assert.strictEqual(s.typeAllRow().checked, true);
+});
+
+test('#2091: encoded values survive repeated filter rewrites without duplicate obs', () => {
+  const s = setup('#/packets/id/42?obs=row%2B%26%3D%20%3F&obs=discarded', {
+    node: 'node +&=', channel: 'channel +&=', _filterExpr: 'name == "A & B"',
+  });
+  s.pickObserver('obsA');
+  s.updatePacketsUrl();
+  const params = new URLSearchParams(s.location.hash.split('?')[1]);
+  assert.deepStrictEqual(params.getAll('obs'), ['row+&= ?']);
+  assert.strictEqual(params.get('node'), s.filters.node);
+  assert.strictEqual(params.get('channel'), s.filters.channel);
+  assert.strictEqual(params.get('filter'), s.filters._filterExpr);
+  assert.strictEqual(params.get('observer'), 'obsA');
+});
+
+test('#2091: returning to list or selecting another packet does not resurrect stale obs', () => {
+  const s = setup('#/packets/aabbccddeeff0011?obs=123');
+  for (const route of ['#/packets', '#/packets?obs=123', '#/packets/1122334455667788']) {
+    s.location.hash = route;
+    s.pickType('4');
+    assert.strictEqual(new URLSearchParams(s.location.hash.split('?')[1]).has('obs'), false, route);
+  }
+});
+
+test('#2091: list query builder never inherits detail observation', () => {
+  const s = setup('#/packets/aabbccddeeff0011?obs=123');
+  assert.strictEqual(s.buildPacketsQuery(60, 'region +&=', false), '?timeWindow=60&region=region%20%2B%26%3D');
+});
+
+test('#2091: init restores obs from the hash after router strips the query', () => {
+  const readInitialObservation = new Function('location', 'routeParam',
+    'let directObsId = "stale", directPacketId = null, directPacketHash = null; ' +
+    'let savedTimeWindowMin = 15, _pendingUrlRegion = null; const filters = {}, window = {}; ' +
+    hashParamsSrc + '\n' + initParamsSrc + '\nreturn directObsId;');
+  for (const route of ['aabbccddeeff0011', 'id/42']) {
+    assert.strictEqual(readInitialObservation({ hash: '#/packets/' + route + '?obs=123' }, route), '123');
+    assert.strictEqual(readInitialObservation({ hash: '#/packets/' + route }, route), null);
+  }
+});
+
 console.log(`\n${passed} passed, ${failed} failed`);
 if (failed > 0) process.exit(1);
 console.log('All tests passed ✅');