fix(analytics): Prefix Tool hash-sizes query (#179) and stale tab on return (#183)

[dborup-agent]

Relates to #179, #183.

Two small, independent analytics fixes, each in its own commit with its tests.

#179: Prefix Tool sent hash-sizes&region=…

renderPrefixTool() appended the &region=…&area=… fragment directly to /analytics/hash-sizes, so with a filter active the server returned the SPA HTML (200 text/html) and the configured-hash-size counts (Kpa-clawbot#1270) fell back silently.

• New withQuery(path, frag) helper in public/analytics.js turns the &… fragments into a ?… query string.
• The three places that spelled out '?' + frag.slice(1) by hand (loadAnalytics, distance, neighbor-graph) now use the same helper. Their URLs are unchanged, and tests pin them.

#183: returning without ?tab= showed the old tab

_currentTab is module state, and init() only set it when the URL had a valid ?tab=. After you leave and come back, the markup marks Overview active, but loadAnalytics() rendered the previously selected tab.

• init() now derives the tab on every mount. It uses ?tab= when that names a tab button, otherwise Overview, and marks that button active.
• I did not add a reset in destroy(), which the issue suggested. With init() deriving the tab on every mount, the reset is redundant and no test could tell it apart.

Tests

New file test-analytics-tab-state-and-query.js (added to test-all.sh). It runs the real app.js api() and the real analytics page in a vm with a fake fetch, using the #172 fixture responses. Its fake DOM builds the tab buttons from the markup that init() writes. It has 9 tests:

• Prefix Tool URL with region + area, with area only, and with no filter.
• The shared loads and the distance tab keep their exact query strings, with and without filters.
• Leave and return without ?tab=, with ?tab=topology, and with an unknown ?tab=: the active button and the rendered content agree.

Mutants:

Mutant	Result
Original origin/master analytics.js	4 tests fail
Prefix Tool back to + rq	2 fail
withQuery returns path + frag	caught (asserts fail and the run crashes)
No Overview fallback for _currentTab in init()	2 fail
Overview button not marked active on fallback	2 fail

Related suites pass: test-1659-analytics-warmup.js (34), all test-analytics-*.js, test-node-analytics-hop-chart.js, test-frontend-helpers.js (707, 0 failed), test-packet-filter.js and test-aging.js. scripts/check-xss-sinks.sh --diff reports no findings.

Browser validation

Ran locally against a Go server with the migrated e2e-fixture.db in Playwright and Chromium.

	origin/master	this branch
Prefix Tool with region SJC	hash-sizes&region=SJC → 200 text/html (×2)	hash-sizes?region=SJC → 200 application/json
Topology → #/nodes → #/analytics	button Overview, content Topology	button Overview, content Overview

No page errors in either run.

Perf

There is no new work in any hot path. The change is one string concatenation per request build and one button lookup per mount.

Customizer

There are no new configurable values.

🤖 Generated with Claude Code

https://claude.ai/code/session_014m8dE6ocwCGg43tGWCPBoB

[dborup-agent]

Rapport — CS-pve-agent1 PR#191 #179+#183 — head 3a22d89

Status: Ready for review. Both issues are fixed in separate commits, tests were red before and green after, CI is green on 3a22d891, and the browser check passed.

Changes

• fix(analytics): Prefix Tool requests /analytics/hash-sizes&region=… (missing '?'), gets HTML instead of JSON #179 (5e1e34c9): The Prefix Tool now requests /analytics/hash-sizes?region=…&area=… instead of /analytics/hash-sizes&region=…. A new helper, withQuery(path, frag) in public/analytics.js, turns the &… filter fragments into a query string. The three call sites that built '?' + frag.slice(1) by hand (loadAnalytics, distance, neighbor-graph) now use it too, and their URLs are unchanged.
• fix(analytics): after leaving and returning to #/analytics, Overview is marked active but the old tab's content is shown #183 (3a22d891): init() now derives the tab on every mount: the URL's ?tab= when it names a tab button, otherwise Overview, and that button is marked active. The tab selected before leaving the page no longer carries over.
• test-analytics-tab-state-and-query.js is new and runs from test-all.sh.

Tests / CI

The new test runs the real app.js api() and the real analytics page in a vm, with a fake fetch serving the #172 fixture responses. Its fake DOM builds the tab buttons from the markup init() writes. It has 9 tests.

	Result
New test on origin/master analytics.js (red before)	5 passed, 4 failed. The failures are the malformed hash-sizes&… URLs and the stale Topology content.
New test on this branch (green after)	9 passed, 0 failed
Mutant: Prefix Tool back to + rq	caught (2 fail)
Mutant: withQuery returns path + frag	caught (asserts fail, run exits 1)
Mutant: no Overview fallback for _currentTab	caught (2 fail)
Mutant: Overview button not marked active on fallback	caught (2 fail)
Related suites	test-1659-analytics-warmup.js 34/0, all test-analytics-*.js, test-node-analytics-hop-chart.js, test-frontend-helpers.js 707/0, test-packet-filter.js, test-aging.js: all pass
scripts/check-xss-sinks.sh --diff	no findings
CI on 3a22d891	Go Build & Test, Playwright E2E and Docker build pass. Badges, Staging and Release are skipped, as expected for a draft.

Browser / E2E validation

I ran Playwright with Chromium against a local Go server using the migrated e2e-fixture.db. I ran the same script once with origin/master's analytics.js and once with this branch.

Scenario	origin/master	This branch
Prefix Tool with region SJC	hash-sizes&region=SJC → 200 text/html (×2)	hash-sizes?region=SJC → 200 application/json
Topology → #/nodes → #/analytics	active button Overview, content Topology	active button Overview, content Overview
Page errors	none	none

I checked the screenshots locally; they are not attached.

Known leftovers

• destroy() does not reset _currentTab, although fix(analytics): after leaving and returning to #/analytics, Overview is marked active but the old tab's content is shown #183 suggested it. Because init() now derives the tab on every mount, the reset would be redundant, and no test can tell whether it is there.
• Pre-existing and not changed here: ?tab= is interpolated into a querySelector attribute selector without escaping, so a value containing " would throw in init().
• The unit tests use a fake DOM, so they prove the logic, not the rendering. The rendering is covered only by the local browser run above; no new Playwright E2E test was added to CI.

[dborup]

Review — CS-MacBook PR#191 analytics — head 3a22d89

Dom: APPROVE med nits

Independent, read-only review of head 3a22d8917a59d26cde498cb89603a91902d89da5 (2 commits, 3 files: public/analytics.js, test-analytics-tab-state-and-query.js, test-all.sh). The branch is 16 commits behind master, so all tests and the browser run below used the merged tree from git merge-tree --write-tree origin/master <head> (clean, exit 0), not just the head. The head was unchanged on git ls-remote before and after the review.

Evidence tags: [T] test, probe, browser run or CI result I ran or saw myself, [A] analysis from reading code, [K] taken from the author's report and not run again.

Summary

Both fixes do what they say. withQuery produces byte-identical URLs to master for every existing call site (90 of 90 compared), the Prefix Tool request is now hash-sizes?region=… with application/json, and returning to #/analytics without ?tab= shows Overview in both the button and the content. I found nothing that blocks the merge. The remaining items are test gaps and a pre-existing, unescaped ?tab= selector that is worth its own small issue.

Findings

#	Priority	Where	Finding
1	Low (test gap)	test-analytics-tab-state-and-query.js	The PR body says tests pin the loadAnalytics, distance and neighbor-graph URLs. The neighbor-graph URL is not pinned: [T] a mutant that drops &min_count=1&min_score=0 from the neighbor-graph call passes all 9 tests. (I checked by hand that the URL is unchanged from master for every region value, so this is a gap, not a bug.)
2	Low (test gap)	same file, first Prefix Tool test	The test cannot tell the Prefix Tool's own hash-sizes request from the one loadAnalytics sends on the same mount (same URL). [T] A mutant that drops the area filter from the Prefix Tool's rq passes. The malformed-URL check (bad list) is what makes the test fail on master, and that part works.
3	Low (pre-existing, see point 6)	public/analytics.js:259	?tab= goes into a querySelector attribute selector without escaping. [T] A crafted link leaves the page on "Loading analytics…" or with an active button that does not match the content. No XSS found. Identical on master.
4	Nit	public/analytics.js:332 (withQuery)	The helper only works for &… fragments on a path without a query. [T] withQuery('/a', 'region=X') returns /a?egion=X (first character dropped), withQuery('/a?x=1', '&region=X') returns /a?x=1?region=X, and withQuery('/a', '&') returns /a?. No current caller hits these, and the comment states the contract, so this is latent only. A one-line guard or a comment on the path restriction would close it.
5	Nit	PR body	The description uses closing keywords for both issues. The repo convention is "Relates to #N", then closing the issues after the merge.
6	Info	area filter	[T] A mutant that removes setAreaFilterVisibility(_currentTab) from init() passes the tests (the fake DOM has no real display handling). That line is not part of this diff, but the new fallback relies on it for the Overview case, so a browser check matters. I did one: see Browser.

Answers to points 1–6

1. withQuery

• Edge cases [T] (probe against the merged analytics.js): empty, undefined and null fragments return the path unchanged. &region=X and &region=X&area=Y give ?region=X and ?region=X&area=Y. A fragment starting with ? also works, because slice(1) drops the ? instead of the &. A path that already has a query and a fragment without a leading & misbehave (finding 4).
• Byte-identical URLs [T]: I compared the master formulas with withQuery for hash-sizes, rf, channels, distance and neighbor-graph over region ∈ {none, SJC, A,B encoded} × area ∈ {none, north, a&b encoded} × window ∈ {none, 7d}: 90 URL pairs, 0 differences. The neighbor-graph form keeps min_count=1&min_score=0 and the right separator with and without a region.
• Filters on all calls [T]: loadAnalytics sends region+area (hash-sizes, hash-collisions), region+area+window (rf, topology, relay-airtime-share) and region+window (channels, no area, as before); distance and neighbor-graph send region only, as before; the Prefix Tool sends region+area. The browser run shows hash-sizes?region=SJC for the Prefix Tool. I also grepped the rest of analytics.js: the remaining concatenated api(...) URLs already have a ? in their base path, so the same bug class does not appear elsewhere in this file. [A]

2. #183 and the warm-up retry

All of this was run in Chromium on the merged tree against a local server [T]:

• Deep links: #/analytics?tab= with rf, topology, channels, hashsizes, collisions, subpaths, nodes, distance, neighbor-graph, prefix-tool, roles, areas each activate the matching button; results are identical to master's.
• Tab switching and history: click Collisions → leave → browser back returns to Collisions (the URL carries the tab); click RF, then Overview, leave and re-enter plainly gives Overview. Identical to master apart from the intended fix.
• fix(analytics): after leaving and returning to #/analytics, Overview is marked active but the old tab's content is shown #183 scenario: Topology → #/nodes → #/analytics: button and content are Overview (master: button Overview, content Topology).
• Unknown ?tab=: falls back to Overview (button and content).
• Warm-up (fix: #172 follow-ups after #169 (analytics warm-up retry, drawer close contrast, IATA oldest test, RX fit race) #175) and destroy(): I forced 503 + Retry-After: 1 on /analytics/rf. The first mount sends 4 requests in 3.5 s (initial + retries); after leaving the page there are 0 more in the next 4 s; a fresh mount again sends 4 in 3.5 s (no doubled chain); after four rapid leave/return cycles the final mount sends 3 in 3.5 s and 0 after the last leave. These numbers are the same on master's analytics.js, so the change adds no duplicate loads and no surviving timers. By code: init() calls loadAnalytics(), which bumps _loadGen through _cancelLoadRetry() and destroy() also cancels it. [A]

3. Tests

• sh test-all.sh on the merged tree: 201 passed, 0 failed (201 files). On clean master it is 200/0 (200 files), so the new file is the only difference. test-test-all.js (the registration guard) passes with the new file listed. [T]
• New test, master analytics.js against the new file: 5 passed, 4 failed (the two malformed Prefix Tool URLs, and the two stale-tab cases). Head and merged tree: 9 passed, 0 failed. [T]
• The unit test stubs setTimeout, so it does not exercise the retry timers; the browser run above covers that. [A]
• My own mutants (none of them in the author's table), each on a copy of the merged analytics.js:

Mutant	Result	Caught by
?tab= accepted without checking the button (urlTab || 'overview')	caught	"…mount with an unknown ?tab=: Overview"
?tab= ignored (always Overview)	caught	"…return with ?tab=topology…", "the distance tab asks for distance?region=…"
withQuery keeps the & (path + '?' + frag)	caught	Prefix Tool region+area, shared loads, distance
channels also gets the area filter	caught	"the shared loads keep their query strings"
relay-airtime-share loses the window	caught	"the shared loads keep their query strings"
distance loses the region	caught	"the distance tab asks for distance?region=…"
neighbor-graph drops min_count/min_score	survived	none (finding 1)
Prefix Tool drops the area filter	survived	none (finding 2)
setAreaFilterVisibility(_currentTab) removed from init()	survived	none (finding 6)

4. Browser

Local Go server built from the merged tree, e2e-fixture.db prepared as in CI (freshen, corescope-migrate, route-advert seed; I skipped the grouped-packet insert for Kpa-clawbot#1486, which the analytics page does not use). I confirmed by pid and cwd that the server running on the port was serving my merged copy, and a second one served master's public/ for comparison. Both servers were stopped by pid afterwards. [T]

	master public/	merged tree
Prefix Tool, region SJC	hash-sizes?region=SJC → 200 application/json (from the shared load) and hash-sizes&region=SJC → 200 text/html ×2	only hash-sizes?region=SJC → 200 application/json
Topology → #/nodes → #/analytics	button Overview, content Topology	button Overview, content Overview
Page errors on all normal scenarios	none	none

The only console errors in my run were the 503 resource lines from the deliberately forced warm-up responses, and the querySelector exception of finding 3 on a deliberately malformed ?tab=.

5. AGENTS.md rules

• No hardcoded colors in the added lines (the #179/#183 hits are comments). [T]
• No per-item API calls; the same six bulk calls per load and one bulk call in the Prefix Tool, as before. [A]
• scripts/check-xss-sinks.sh --diff against master (run on the diff in a scratch git repo): exit 0, no findings. [T]
• deploy.yml is not touched and has 9 × github.repository == 'Kpa-clawbot/CoreScope'. [T]
• Deep linking: the new state is derived from the existing ?tab= parameter; nothing new to add to the hash. [A]

6. Known leftovers

• destroy() does not reset _currentTab: I agree it is redundant. Nothing reads _currentTab between destroy() and the next init() (all refresh timers are stopped in destroy(), the theme handler is removed, and in-flight loads are cancelled by _loadGen), and init() assigns it unconditionally at the top of every mount. The browser scenarios above pass without the reset. [A][T]
• Unescaped ?tab=: pre-existing and identical on master [T], but it is more than an exception, and I recommend a separate small issue (or a two-line fix in this PR, since these lines were just rewritten):
  • A tab value containing " makes querySelector throw inside init(). The page shows the tab bar and "Loading analytics…" forever, and no analytics request is sent.
  • A value such as x"],[data-tab="overview produces a valid selector that matches a real button. _currentTab becomes the attacker-chosen string, no switch case matches it, and the page again sits on "Loading analytics…" while a button (Overview or RF in my runs) looks active. That is the same button/content mismatch class as fix(analytics): after leaving and returning to #/analytics, Overview is marked active but the old tab's content is shown #183, reachable from a shared link.
  • Not exploitable for script execution as far as I can see: _currentTab is only compared, used in a switch, set as an element id and used as a localStorage key suffix for column widths, and never interpolated into HTML. The section and window parameters in the same function are handled safely (getElementById, a <select> value, encodeURIComponent). [A]
  • Fix: look the button up by dataset.tab instead of building a selector (Array.from(analyticsTabs.querySelectorAll('.tab-btn')).find(b => b.dataset.tab === urlTab)), or wrap the value in CSS.escape.

CI (head 3a22d891)

Go Build & Test: pass (13m9s). Playwright E2E: pass (20m0s). Build & Publish Docker Image: pass (49s). Deploy Staging, Release Artifacts, Publish Badges & Summary: skipped, as expected for a draft. [T]

Not verified

• [K] The author's mutant table, the Playwright/Chromium screenshots and the xss-sinks run on the author's side; I re-ran the equivalent checks myself except the screenshots.
• Rendering beyond the DOM text and active-button checks (no visual or layout check of the tab bar or the area filter's visibility).
• The Packets page collapse button in the left column of the table opens the dialog. Kpa-clawbot/CoreScope#1486 grouped-packet fixture row from CI was not inserted; the analytics page does not depend on it.
• Tabs other than the twelve deep links listed, and mobile layouts.