diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
index 8e33da8a..e337fa74 100644
--- a/.github/workflows/deploy.yml
+++ b/.github/workflows/deploy.yml
@@ -356,6 +356,7 @@ jobs:
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
BASE_URL=http://localhost:13581 node test-observer-iata-1188-e2e.js 2>&1 | tee -a e2e-output.txt
+ BASE_URL=http://localhost:13581 node test-issue-1639-observers-sort-e2e.js 2>&1 | tee -a e2e-output.txt
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-fluid-1055-e2e.js 2>&1 | tee -a e2e-output.txt
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-priority-1102-e2e.js 2>&1 | tee -a e2e-output.txt
CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-nav-priority-1311-e2e.js 2>&1 | tee -a e2e-output.txt
diff --git a/public/observers.js b/public/observers.js
index f950f5d4..25b4eaeb 100644
--- a/public/observers.js
+++ b/public/observers.js
@@ -116,6 +116,7 @@ window.ObserversSummary = (function () {
let wsHandler = null;
let refreshTimer = null;
let regionChangeHandler = null;
+ let _obsSortCtl = null; // #1641 r1 finding #6: tracked TableSort controller for destroy-before-reinit
function init(app) {
app.innerHTML = `
@@ -181,6 +182,10 @@ window.ObserversSummary = (function () {
refreshTimer = null;
if (regionChangeHandler) RegionFilter.offChange(regionChangeHandler);
regionChangeHandler = null;
+ if (_obsSortCtl && typeof _obsSortCtl.destroy === 'function') {
+ try { _obsSortCtl.destroy(); } catch (_) { /* ignore */ }
+ }
+ _obsSortCtl = null;
observers = [];
obsSkewMap = {};
}
@@ -289,36 +294,71 @@ window.ObserversSummary = (function () {
`;
makeColumnsResizable('#obsTable', 'meshcore-obs-col-widths');
+ const obsTbl = document.getElementById('obsTable');
// #1056: fluid columns + +N hidden pill
- if (window.TableResponsive) {
- var _obsTbl = document.getElementById('obsTable');
- if (_obsTbl) window.TableResponsive.register(_obsTbl);
+ if (obsTbl && window.TableResponsive) {
+ window.TableResponsive.register(obsTbl);
+ }
+ // #1639 — wire TableSort AFTER tbody is in the DOM (avoid the #679 init
+ // race). Re-init on every render so the controller binds to the new
+ // header instance, and persist sort state under meshcore-observers-sort.
+ if (obsTbl && window.TableSort) {
+ // #1641 r1 finding #6: destroy the prior controller before re-init so
+ // we don't leak header click listeners across the 30s render cycle.
+ if (_obsSortCtl && typeof _obsSortCtl.destroy === 'function') {
+ try { _obsSortCtl.destroy(); } catch (_) { /* ignore */ }
+ }
+ _obsSortCtl = TableSort.init(obsTbl, {
+ defaultColumn: 'last_seen',
+ defaultDirection: 'desc',
+ storageKey: 'meshcore-observers-sort'
+ });
+ } else if (obsTbl && !window.TableSort) {
+ // #1641 r1 finding #14: surface bundler/loading regressions instead of
+ // silently rendering an unsortable table.
+ console.warn('[observers] window.TableSort missing — table will not be sortable');
}
}
diff --git a/public/table-sort.js b/public/table-sort.js
index aa4b956b..5a2aed26 100644
--- a/public/table-sort.js
+++ b/public/table-sort.js
@@ -114,7 +114,12 @@ window.TableSort = (function() {
state.direction = state.direction === 'asc' ? 'desc' : 'asc';
} else {
state.column = key;
- state.direction = options.defaultDirection || 'asc';
+ // #1639/#1641: switching to a new column picks a sensible default
+ // direction per type — text columns default to ascending (A→Z),
+ // numeric/date/dbm default to descending (largest/newest first).
+ // options.defaultDirection only seeds the INITIAL load.
+ var thType = th.getAttribute('data-type');
+ state.direction = (!thType || thType === 'text') ? 'asc' : 'desc';
}
doSort();
};
diff --git a/test-issue-1639-observers-sort-e2e.js b/test-issue-1639-observers-sort-e2e.js
new file mode 100644
index 00000000..7e5c68f0
--- /dev/null
+++ b/test-issue-1639-observers-sort-e2e.js
@@ -0,0 +1,174 @@
+/**
+ * E2E test (#1639): observers table at #/observers must be sortable.
+ *
+ * Same fix-pattern as #679 (nodes table) — wire TableSort with
+ * numeric / time column hints. Clicking a column header MUST reorder
+ * tbody rows.
+ *
+ * Round-1 test additions (PR #1641):
+ * - last_packet_at column reorder (regression guard for the round-0
+ * data-type=date → numeric fix; without this the one-line fix is
+ * unguarded).
+ * - strict monotonic non-increasing assertion (no more .some(changed),
+ * which silently passes even when order is mostly wrong).
+ * - toggle (second-click ascending) on packet_count.
+ * - second-column string sort (Name) to exercise localeCompare.
+ * - cells are addressed via data-testid (added in observers.js for
+ * testability) — finding #8 dropped the broader data-col-key on
+ * | s, and data-testid is the less-invasive replacement.
+ *
+ * Usage: BASE_URL=http://localhost:13581 node test-issue-1639-observers-sort-e2e.js
+ */
+const { chromium } = require('playwright');
+
+const BASE = process.env.BASE_URL || 'http://localhost:3000';
+
+async function test(name, fn) {
+ try {
+ await fn();
+ console.log(` \u2705 ${name}`);
+ } catch (err) {
+ console.log(` \u274c ${name}: ${err.message}`);
+ process.exit(1);
+ }
+}
+
+function assert(cond, msg) {
+ if (!cond) throw new Error(msg || 'Assertion failed');
+}
+
+// finding #5: strict monotonic non-increasing across ALL rows
+function assertNonIncreasing(arr, label) {
+ for (let i = 0; i < arr.length - 1; i++) {
+ // NaN slots are sorted last by the comparator — accept them at the tail.
+ const a = arr[i], b = arr[i + 1];
+ if (!Number.isFinite(a)) continue; // NaN tail
+ if (!Number.isFinite(b)) continue;
+ if (!(a >= b)) {
+ throw new Error(`${label}: not non-increasing at idx ${i}: ${a} < ${b}; full=${JSON.stringify(arr)}`);
+ }
+ }
+}
+
+function assertNonDecreasing(arr, label) {
+ for (let i = 0; i < arr.length - 1; i++) {
+ const a = arr[i], b = arr[i + 1];
+ if (!Number.isFinite(a)) continue;
+ if (!Number.isFinite(b)) continue;
+ if (!(a <= b)) {
+ throw new Error(`${label}: not non-decreasing at idx ${i}: ${a} > ${b}; full=${JSON.stringify(arr)}`);
+ }
+ }
+}
+
+async function gotoObservers(page) {
+ await page.goto(`${BASE}/#/observers`, { waitUntil: 'domcontentloaded' });
+ await page.evaluate(() => { try { localStorage.removeItem('meshcore-observers-sort'); } catch (_) {} });
+ await page.reload({ waitUntil: 'domcontentloaded' });
+ await page.waitForSelector('#obsTable', { timeout: 10000 });
+ await page.waitForSelector('#obsTable tbody tr', { timeout: 10000 });
+}
+
+async function readNumeric(page, testid) {
+ return await page.$$eval(`#obsTable tbody tr td[data-testid="${testid}"]`, (tds) => tds.map((td) => {
+ const dv = td.getAttribute('data-value');
+ const raw = dv != null && dv !== '' ? dv : td.textContent.replace(/[^0-9.-]/g, '');
+ return Number(raw);
+ }));
+}
+
+async function readStrings(page, testid) {
+ return await page.$$eval(`#obsTable tbody tr td[data-testid="${testid}"]`, (tds) =>
+ tds.map((td) => td.getAttribute('data-value') || ''));
+}
+
+async function run() {
+ 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);
+
+ console.log(`\nRunning #1639 observers-sort E2E tests against ${BASE}\n`);
+
+ await test('observers thead has data-sort-key + data-type on numeric columns', async () => {
+ await gotoObservers(page);
+ const totalPktsTh = await page.$('#obsTable thead th[data-sort-key="packet_count"]');
+ assert(totalPktsTh, 'Total Packets | must carry data-sort-key="packet_count"');
+ const type = await totalPktsTh.getAttribute('data-type');
+ assert(type === 'numeric',
+ `Total Packets | must have data-type="numeric", got "${type}"`);
+ });
+
+ await test('clicking Total Packets header reorders rows numerically (desc)', async () => {
+ await gotoObservers(page);
+ const before = await readNumeric(page, 'obs-cell-packet-count');
+ assert(before.length >= 2, `need >=2 rows, got ${before.length}`);
+
+ const th = await page.$('#obsTable thead th[data-sort-key="packet_count"]');
+ assert(th, 'Total Packets | not found');
+ await th.click();
+ await page.waitForTimeout(300);
+
+ const after = await readNumeric(page, 'obs-cell-packet-count');
+ assert(after.length === before.length, `row count changed`);
+ // finding #5: strict non-increasing across ALL rows, not just first/last
+ assertNonIncreasing(after, 'packet_count desc');
+ });
+
+ // finding #13: toggle (second click on the same header → ascending)
+ await test('second click on Total Packets header toggles to ascending', async () => {
+ await gotoObservers(page);
+ const th = await page.$('#obsTable thead th[data-sort-key="packet_count"]');
+ await th.click(); // desc
+ await page.waitForTimeout(200);
+ await th.click(); // asc
+ await page.waitForTimeout(300);
+ const after = await readNumeric(page, 'obs-cell-packet-count');
+ assert(after.length >= 2, 'need >=2 rows');
+ assertNonDecreasing(after, 'packet_count asc (after toggle)');
+ });
+
+ // finding #4: regression guard for round-0 data-type=date → numeric fix
+ await test('clicking Last Packet header reorders rows by timestamp (desc)', async () => {
+ await gotoObservers(page);
+ const th = await page.$('#obsTable thead th[data-sort-key="last_packet_at"]');
+ assert(th, 'Last Packet | not found');
+ const type = await th.getAttribute('data-type');
+ assert(type === 'numeric',
+ `Last Packet | must have data-type="numeric" (round-0 fix), got "${type}"`);
+ await th.click();
+ await page.waitForTimeout(300);
+ const after = await readNumeric(page, 'obs-cell-last-packet');
+ assert(after.length >= 2, `need >=2 rows`);
+ assertNonIncreasing(after, 'last_packet_at desc');
+ });
+
+ // finding #13: second-column test — Name string sort via localeCompare
+ await test('clicking Name header sorts alphabetically (string localeCompare)', async () => {
+ await gotoObservers(page);
+ const th = await page.$('#obsTable thead th[data-sort-key="name"]');
+ assert(th, 'Name | not found');
+ await th.click();
+ await page.waitForTimeout(300);
+ const after = await readStrings(page, 'obs-cell-name');
+ assert(after.length >= 2, `need >=2 rows`);
+ // Default direction for text columns is asc — sorted A→Z
+ for (let i = 0; i < after.length - 1; i++) {
+ if (after[i] === '' || after[i + 1] === '') continue;
+ assert(after[i].localeCompare(after[i + 1]) <= 0,
+ `Name asc broken at idx ${i}: ${JSON.stringify(after[i])} > ${JSON.stringify(after[i + 1])}`);
+ }
+ });
+
+ await browser.close();
+ console.log('\n✅ all #1639 sort tests passed\n');
+}
+
+run().catch((e) => {
+ console.error(e);
+ process.exit(1);
+});
diff --git a/test-xss-escape-sinks.js b/test-xss-escape-sinks.js
index c741adcb..984edbcf 100644
--- a/test-xss-escape-sinks.js
+++ b/test-xss-escape-sinks.js
@@ -179,9 +179,14 @@ test('observers.js renderRow: observer name cell escapes o.name', () => {
// Capture just the | ${ ... o.name || o.id ... }${chip} | .
const html = evalTemplate(
'public/observers.js',
- // Allow optional trailing content (e.g. listener/repeater badge added by #1290)
- // between the naive chip and the closing .
- /(\$\{[^}]*o\.name[^}]*\}\$\{window\.ObserversNaiveChip\.render\(o\)\}[^`]*?<\/td>)/,
+ // Allow optional | attributes before class="mono" (e.g. data-testid /
+ // data-value added by #1639 for sortable-column tests) AND optional trailing
+ // content (e.g. listener/repeater badge added by #1290) between the naive
+ // chip and the closing | . Capturing any data-value="${...}" attr makes
+ // this assertion STRICTER, not weaker: the eval now also renders those attr
+ // interpolations, so dropping escapeHtml() on either the attr OR the cell
+ // text trips assertNoXss (raw ![]()
]*\bclass="mono"[^>]*>\$\{[^}]*o\.name[^}]*\}\$\{window\.ObserversNaiveChip\.render\(o\)\}[^`]*?<\/td>)/,
{ o: { name: TAG_PAYLOAD + ATTR_PAYLOAD, id: 'obs-1' },
window: { ObserversNaiveChip: { render: () => '' } } }
);