fix(map): ignore stale async work and keep Path Inspector controls usable

[dborup]

Relates to #123

Plan and design

The user asked for autonomous work, so the plan is written here instead of waiting for sign-off (AGENTS.md rule 5).

1. Test first (c68accbd): a new Playwright test, test-issue-123-map-lifecycle-e2e.js, registered in the CI Playwright step. It fails on master with 0/12.
2. Fix (df235cb7): public/map.js and public/style.css.
3. Review round (da47a0b0): completes criterion 3. The route view's delayed timers are inert after destroy, and destroy removes the theme observer and the tile-provider listener. Touches public/route-view.js, public/map.js and the E2E.

Lifecycle token per mount

• mapGeneration is bumped by both init() and destroy().
• mapToken() returns an alive() check that stays true only while the mount that created it is current.
• Every async path checks alive() after its awaits:
  • init() after /api/config/map. This await happens before L.map(), so leaving during it used to throw Map container not found, and a quick return threw Map container is already initialized.
  • The geo-filter overlay and the area outline refresh. The outline refresh also has its own refresh generation, so only the newest refresh draws.
  • loadNodes().then(...).
  • drawPacketRoute and drawPacketRouteMulti after /api/resolve-hops.
  • loadRouteFromDeepLink and loadEstimatedNodesFromDeepLink.
  • selectReferenceNode, which now fills a local set and publishes it only if the mount and the selection are both still current.
  • The affinity overlay.
• A bare if (map) is not enough. After a quick return it sees the next mount's map, which is the core point of the issue.

Request generation for loadNodes()

• Every call gets a request number.
• Only the newest request of the current mount assigns nodes/observers, renders, and sets data-loaded.
• A superseded call resolves together with the newest one, so the deep-link chain in init() still runs with loaded nodes.

Timers and listeners

• mapTimeout() replaces the loose setTimeouts: the 100 ms invalidateSize, the target-node popup, and the two pane invalidateSize calls. Its timers are mount-scoped and tracked in a set that removes itself when a timer fires.
• destroy() clears those timers and the zoom/resize timer.
• destroy() removes the two AreaFilter.onChange listeners, which were previously added again on every mount and never removed.
• destroy() also disconnects the data-theme MutationObserver and removes the window mc-tile-provider-changed listener (da47a0b0). Before that, every mount added both for good, and each kept calling setUrl on its removed map's tile layer: 6 mounts left 6 listeners.
• Route view (public/route-view.js, da47a0b0): leaving the map with an open route threw TypeError: Cannot read properties of undefined (reading '_leaflet_pos'), because the teardown ran invalidateSize 50 ms later on the map that destroy() had removed.
  • The route view now marks a map as removed on Leaflet's public unload event, which map.remove() fires.
  • Every route-view timer that touches the map goes through a local mapTimeout(mapRef, fn, ms) that skips a removed map. That covers the teardown/close/collapse invalidateSize, the mobile-sheet refit, the spider fan and the resize refit.
• initMapSidePane() was called by every loadNodes(), including WS advert reloads, and added a new click listener each time. After an even number of reloads one click toggled the pane twice, so it looked dead. The pane is now wired once per mount (data-wired).

Layout

• From 641 px and up, .map-controls and .map-controls-toggle are offset by the Path Inspector's width: 32 px collapsed, 320 px expanded.
• Below 641 px the pane is hidden and the existing mobile rules apply unchanged.

How this differs from upstream Kpa-clawbot/CoreScope#2048

Upstream is read as a reference only; nothing was cherry-picked.

• The CSS offsets match upstream.
• Upstream guards on map identity (map === initializedMap). That does not cover the await in init() before L.map() exists, which is the navigate-during-config and quick-return case, and it does not cover out-of-order loadNodes() calls on one instance. Both are acceptance criteria here, so this PR uses a mount generation plus a request generation.
• This PR also removes the duplicate pane, AreaFilter, theme-observer and tile-provider listeners that the fork accumulates. Upstream does not touch them.

Acceptance criteria

Criterion	Status	Evidence
Every mount has a lifecycle token, and every same-instance node load has a request generation	Met	mapGeneration / mapToken(), nodesLoadGeneration
Async results and delayed callbacks affect only their own generation; only the newest same-instance load renders	Met	E2E "out-of-order node loads", "leaving right after init"
Destroy cancels timers and listeners where practical and makes remaining work inert	Met	destroy() clears mapTimers and the zoom/resize timer, removes the AreaFilter listeners, the theme observer and the tile-provider listener; route-view timers are inert on a removed map. E2E "leaving the map with an open route" (desktop 1440x900, tablet 1024x768) and "six mounts leave one tile-provider listener and one theme observer". The fetches themselves are not aborted; their continuations become no-ops.
Old work cannot clear, recentre or populate a later map	Met	E2E "quick return" (one .leaflet-container, the same map object), plus the resolve-hops test
Path Inspector controls stay visible, do not overlap, and are clickable at tablet and desktop widths	Met (normal map mode)	E2E at 1440x900, 1280x600, 1024x768, 800x900 and 641x800, collapsed and expanded, with real (unforced) clicks. Route mode: see "Known limitation".
Route rendering, filters, saved viewport and selected-node behaviour are preserved	Met (tests)	The existing map E2E and unit suites are unchanged (numbers below)

Tests

New test test-issue-123-map-lifecycle-e2e.js (Playwright, run locally against a server on test-fixtures):

	Passed	Failed
master d264716c	0	12
df235cb7	12	0
df235cb7 + the 3 review-round steps (before the fix)	11	4
da47a0b0	15	0
da47a0b0 merged with origin/master (local)	15	0

Before the review-round fix, these steps failed:

• Desktop: pageerror: Cannot read properties of undefined (reading '_leaflet_pos').
• Tablet: the same error.
• Six mounts: 6 map tile-provider listeners after 6 mounts, want 1.
• The aggregate no-errors step.

Review-round mutants, each caught:

• mapTimeout in route-view ignores the removal: 12/3.
• destroy() keeps the theme observer: 14/1 ("6 map theme observers").
• destroy() keeps the tile-provider listener: 14/1 ("6 map tile-provider listeners").

The six-mount step also checks the effect: two theme switches plus one provider event cause as many setUrl calls after 6 mounts as after 1, and leaving the map leaves 0 listeners and 0 observers.

Errors on master, as reported by the test:

• The controls overlap the inspector at every width.
• One click after three node reloads does nothing.
• Map container not found.
• Map container is already initialized.
• Cannot read properties of null (reading 'invalidateSize')
• Cannot read properties of null (reading 'clearLayers')
• An older node load overwrote the newer one.

Existing suites on da47a0b0 (local):

Suite	Result
test-issue-1374-route-map-a11y-e2e.js	20/20
test-issue-1236-map-mobile-e2e.js	3/3
test-issue-1329-map-controls-accordion-e2e.js	5/5
test-map-modal-fluid-e2e.js	10/10
test-path-inspector-coverage-e2e.js	10/10
test-map-nodes-pagination-e2e.js	3/3
test-issue-1418-* (6 files, including spider-fan and polish-review)	0 failed
test-top-routes-overlay.js	28/28
test-map-scope-filter.js	17/17
test-packet-path-map.js	42/42
test-xss-escape-sinks.js	34/34
test-packet-filter.js	92/92
test-aging.js	19/19
test-issue-1633-hide-1byte-hops.js	21/21
test-issue-1420-tile-providers.js	39/39
test-issue-1614-tile-url-function.js	3/3
test-frontend-helpers.js	705 passed / 2 failed, identical on master

Earlier runs on df235cb7:

• test-e2e-playwright.js with fail-fast disabled: branch 130 passed, 4 skipped, 1 failed; master 131 passed, 3 skipped, 1 failed.
  • The single failure is identical on master: Packets type filter includes Group Data (#1791).
  • The skip difference is Short URL: 8-char prefix ..., skipped on a prefix collision in the freshened fixture copy.
• test-map-clustering.js: 4 passed / 1 failed, identical on master.

Other checks:

• scripts/check-xss-sinks.sh --diff origin/master: clean on da47a0b0.
• No workflow guard lines changed; only one line was added to the Playwright step.

Browser check

• Screenshots were taken locally at 1280x600 and 800x900 with the inspector expanded. On master the controls panel covers the inspector's input field and its toggle; on this branch the controls sit inside the map area and the inspector is free.
• The review-round cases run in local Chromium through the E2E above, at desktop 1440x900 and tablet 1024x768.
• Map tiles and the Leaflet CDN are blocked in the sandbox, so the local runs served Leaflet from the npm package and a stub tile (harness only, nothing in the repo).

Perf

• Not a hot path. The added work is one integer comparison per await continuation or timer, and one WeakSet lookup per route-view timer.
• mapTimers holds at most the few pending mount timers and empties itself as they fire.
• Removing the leaked AreaFilter, theme-observer and tile-provider listeners reduces work: on master every theme or provider change called setUrl once per earlier mount.

Known limitation (pre-existing, not changed here)

• In route mode, public/route-view.css sets #leaflet-map to left: 320px; width: calc(100% - 320px), so the map covers the Path Inspector column and its toggle #mapPaneToggle.
• This was already the case on master.
• It is not a trivial fix: it needs a decision on how route mode and the inspector share the width (hide the inspector in route mode, or subtract its width), plus tests. So it is left out of this PR.

Not verified

• Real device and browser widths beyond the Playwright viewports, and touch input on a tablet.
• Behaviour against the production database size (the node loads are the same requests as before).
• "Show on map" from the Path Inspector: an earlier version of this text said it does nothing when already on #/map. That is refuted in issue cleanup(path-inspector): remove unreachable showOnMap branch that calls undefined window.drawPacketRoute #143. The branch in public/path-inspector.js that calls the undefined window.drawPacketRoute cannot be reached in normal navigation: the standalone inspector always stores a pending route and navigates to #/map, and the embedded pane calls the local drawPacketRoute. It is dead code, and cleanup(path-inspector): remove unreachable showOnMap branch that calls undefined window.drawPacketRoute #143 tracks its removal.

Overlap with other open PRs

• .github/workflows/deploy.yml (Playwright step, one added line): the same step was also edited by PR fix(live): wire every persisted view toggle before Live init awaits #135 (fix(live): wire persisted view toggles before initialization awaits #125, merged). Neighbouring lines only; the branch merges cleanly with origin/master.
• No other open PR touches public/map.js, public/route-view.js or public/style.css.

🤖 Generated with Claude Code

https://claude.ai/code/session_011FcyXW5RdFzLZhuL1ntAsY

[dborup]

Independent review of df235cb7

Verdict: APPROVE with nits. This is a recommendation only; merging is the owner's call.

Reviewed head: df235cb7f4c4f341446baa1bec397e17703e4e1c (unchanged before and after the review). Work done on git archive trees of the head, of commit A c68accbd, of origin/master ad011021, and of the clean merge result (git merge-tree tree b2279299). Each tree was served by a local server on ports 13730-13733.

Labels: [F] freshly verified by me · [T] taken from the PR text · [A] assumption · [K] known limitation.

Findings

1. P3. The new E2E does not pin several of the guards the PR claims. test-issue-123-map-lifecycle-e2e.js:193, public/map.js:1321, public/map.js:2567-2568, public/map.js:2356. These mutants survive the whole new E2E (12/12 green):

   • M6: remove the post-resolve-hops guard in drawPacketRouteMulti. The test passes a single path, and a single path delegates to drawPacketRoute (map.js:1283-1285), so the multi-path branch is never exercised.
   • M8: do not remove the AreaFilter listeners in destroy().
   • M10: drop the stale-selection guard in selectReferenceNode.

   I wrote a probe for each of these (a reviewer probe, kept locally and not committed). All three probes pass on the head, fail on master, and kill their mutant:

   • S1 (multi-path route, leave while resolve-hops is pending). On master and on M6 it throws Cannot read properties of null (reading 'addLayer').
   • S2 (live map.js AreaFilter listeners after 4 mounts): head 2, master 8, M8 8.
   • S3 (neighbour lookup for A arrives late after B was selected): head d, master dc, M10 c.

   So the code is correct, but the regression protection covers only part of what the PR claims. Also, the step "leaving right after init: the delayed invalidateSize is inert" goes red on master because of the loadNodes render after destroy, not because of the 100 ms timer. M4/M5 (mapTimeout without alive(), or without clearing in destroy()) survive. They behave the same as the fix in practice: a stale invalidateSize on the next map is harmless [A]. [F]

2. P3 (process). The CI Playwright job on this head is red, on an unrelated test. Run 36557644736 fails in test-analytics-fluid-charts.js AC3 with precondition failed: expected ≥2 cols at 1300px; got 1, after the same precondition passed earlier in that run. Nothing in the diff can affect that page: the only CSS added is map-scoped, inside @media (min-width: 641px). Locally it passes 5/5 on the head and 5/5 on master. It looks like a flake and needs a re-run before merge. The new fix(map): ignore stale async work and keep Path Inspector controls usable #123 E2E passed in CI (--- 12 passed, 0 failed ---). [F]

3. nit. The pane widths are duplicated as magic numbers. public/style.css:4584-4587 hardcode 32px/320px, which must match .map-side-pane flex: 0 0 32px / 320px at public/style.css:4574-4575. Resizing the pane later would silently bring the overlap back. A CSS custom property (e.g. --map-side-pane-w) shared by both rules would remove the coupling. There are no colour literals, so the CSS-variable rule is respected. [F]

Metadata

• CI on df235cb7 is complete: Go Build & Test SUCCESS, Playwright E2E FAILURE (unrelated flake, finding 2), Docker SKIPPED because of it. The new fix(map): ignore stale async work and keep Path Inspector controls usable #123 E2E itself ran and passed in that job (passed 12 failed 0). A re-run is needed before merge. [F]

• Head at start and end: df235cb7f4c4f341446baa1bec397e17703e4e1c [F]

• Commits in origin/master..head (both have author and committer dborup <kontakt@meshview.dk>) [F]:

  • c68accbd test(map): reproduce stale async work and Path Inspector overlap (fix(map): ignore stale async work and keep Path Inspector controls usable #123)
  • df235cb7 fix(map): ignore stale async work and keep Path Inspector controls clear (fix(map): ignore stale async work and keep Path Inspector controls usable #123)

• Files: .github/workflows/deploy.yml (+1), public/map.js (+108/-17), public/style.css (+10), test-issue-123-map-lifecycle-e2e.js (+236, new) [F]

• merge-base d264716c. That is 3 commits behind origin/master ad011021, which since then touched only public/analytics.js, public/app.js, test-all.sh and a fix(analytics): treat lazy distance-index 202 responses as transient #120 test. git merge-tree --write-tree origin/master df235cb7 is clean (tree b2279299). On the merge tree: the new E2E is 12/12, the 1329 accordion E2E 5/5, map-modal-fluid 10/10, path-inspector-coverage 10/10, and my probe 5/5. [F]

• PR body [F]:

  • no closing keyword; Relates to #123
  • no @mentions
  • upstream appears only as `Kpa-clawbot/CoreScope#2048` in code format
  • no github.com/Kpa-clawbot URL
  • it is a draft

• Workflow: the only change is one added registration line in the Playwright step: CHROMIUM_REQUIRE=1 BASE_URL=http://localhost:13581 node test-issue-123-map-lifecycle-e2e.js 2>&1 | tee -a e2e-output.txt. No github.repository == 'Kpa-clawbot/CoreScope' line, trigger, permission or job changed. [F]

Acceptance criteria (issue #123)

Criterion	Result
Lifecycle token per mount, request generation per same-instance node load	Met [F]. mapGeneration/mapToken() at map.js:32-60, bumped in init() (:208) and destroy() (:2562). nodesLoadGeneration with current()/stale() at map.js:1722-1730.
Async results/delayed callbacks affect only their generation; only newest same-instance load renders	Met [F]. The E2E "out-of-order node loads" is green on the head and red on master (nodes: a). M2 (drop the request generation) and M15 (assign nodes before the observers await) are both caught. My S5 (initial load superseded by a filter change while /api/observers is delayed) is green: data-loaded appears only once nodes are assigned.
Destroy cancels timers/listeners where practical; remaining work inert	Met [F]. destroy() clears mapTimers and _zoomResizeTimer and calls AreaFilter.offChange for both handlers (map.js:2561-2569). S2: 2 live map.js listeners after 4 mounts on the head, 8 on master. The fetches are not aborted, only made inert, as the PR states [T/F].
Old work cannot clear, recentre or populate a later map	Met [F]. The E2E "quick return" (one .leaflet-container, the same __mc_map) is green on the head and red on master (Map container is already initialized). My S4 adds a random 0-1500 ms delay to every /api/* request and does 5 rapid map↔packets cycles, then a final mount. On the head: 1 container, __mc_map is the visible map, 202 nodes, 0 errors. On master: Map container not found and null.invalidateSize.
Path Inspector controls visible, non-overlapping, clickable at tablet/desktop widths	Met [F]. The E2E covers 1440x900, 1280x600, 1024x768, 800x900 and 641x800, collapsed and expanded, with real clicks: green on the head, 5/5 red on master. My bounding boxes with the pane expanded, head: controls [718..948] inside map [0..960], pane from 960 at 1280x600; [129..309] inside [0..321] at 641x800; no overlap with the Leaflet zoom control [10..58]. Master: controls [1038..1268] over the pane. I checked the screenshots visually. CSS mutants M16/M17/M18 are all caught.
Preserve route rendering, filters, saved viewport, selected-node behaviour	Met on existing tests [F]. The existing map E2Es and map unit tests give identical results on master and the head (below). The map-view save code is untouched. Selected-node behaviour beyond the existing tests: [A].

Test-first and mutants

• Commit A c68accbd adds only the test and the workflow line. The test file is byte-identical between A and the head (git diff c68accbd df235cb7 touches only public/). A's public/map.js/style.css equal master's (master's later drift is in analytics.js/app.js). [F]
• New E2E on the master server: 0 passed, 12 failed. On the head server: 12 passed, 0 failed. Same result as the PR table. [F]

All mutants were applied to a separate archive tree (a separate local tree and server). After each mutant I restored the files and checked them with shasum against git show df235cb7:<path>: map.js b6d79c2a…, style.css 774c509b…, both match. [F]

#	Mutant	New E2E	My probe (S1-S5)	Result
M1	drop alive() after /api/config/map in init	9/3	–	caught
M2	drop request generation from current()	11/1	–	caught
M3	drop data-wired once-guard on side pane	11/1	–	caught
M4	mapTimeout without alive() and destroy() not clearing timers	12/0	5/0	survived: equivalent in practice [A] (only a harmless invalidateSize on the next map)
M5	mapTimeout without alive() only	12/0	5/0	survived: equivalent (destroy still clears)
M6	drop post-resolve-hops guard in drawPacketRouteMulti	12/0	S1 fails	test gap (Finding 1)
M7	drop post-resolve-hops guard in drawPacketRoute	10/2	–	caught
M8	destroy() keeps AreaFilter listeners	12/0	S2 fails (8 listeners)	test gap (Finding 1)
M9	drop alive() in loadNodes().then in init	12/0	5/0	survived: near-equivalent [A]. The deep-link loaders read the current hash and have their own guards
M10	drop stale-selection guard in selectReferenceNode	12/0	S3 fails (c)	test gap (Finding 1)
M11	area outline: drop refreshGen check	12/0	5/0	survived: untested, low impact (polygons are cached after the first fetch) [A]
M12	superseded loadNodes resolves immediately (stale() → undefined)	12/0	5/0	survived: untested; only matters for a deep link combined with an early reload [A]
M13	data-loaded set by any request	12/0	5/0	survived: untested; affects only E2E synchronisation
M14	drop both alive() guards in loadRouteFromDeepLink	12/0	5/0	survived: near-equivalent [A]. drawPacketRoute has its own !map guard
M15	assign nodes before the observers await (old order)	11/1	–	caught
M16	CSS: drop expanded offsets	7/5	–	caught
M17	CSS: media min-width 641 → 1025	9/3	–	caught
M18	CSS: drop toggle offsets	7/5	–	caught

Suites run locally

Frontend-only PR, so I ran no Go suites; cmd/server and cmd/ingestor are untouched. [F]

Suite	master ad011021	head df235cb7
test-issue-123-map-lifecycle-e2e.js	0/12	12/0
test-issue-1236-map-mobile-e2e.js	3/3	3/3
test-issue-1329-map-controls-accordion-e2e.js	5/5	5/5
test-issue-1374-route-map-a11y-e2e.js	20/0	20/0
test-map-modal-fluid-e2e.js	10/0	10/0
test-map-nodes-pagination-e2e.js	3/0	3/0
test-path-inspector-coverage-e2e.js	10/0	10/0
test-issue-1146-path-link-contrast-e2e.js	11/0	11/0
test-issue-1705-subpath-contrast-e2e.js, test-a11y-axe-routes-coverage.js	rc 0	rc 0
test-e2e-playwright.js (my copy with fail-fast disabled)	130/135, 4 skip, 1 fail	131/135, 3 skip, 1 fail
test-analytics-fluid-charts.js (the CI failure)	8/0 ×5	8/0 ×5

• The single test-e2e-playwright.js failure is identical on both: "Version info lives on Perf dashboard", which is known environment noise. The skip difference is "Short URL: 8-char prefix" (fixture prefix collision after freshen).

• Unit tests, same results on master and the head:

  • test-top-routes-overlay 28/0
  • test-map-scope-filter 17/0
  • test-packet-path-map 42/0
  • test-area-nodes-map 14/0
  • test-issue-1356-map-a11y 40/0
  • test-issue-1136 5/0
  • test-issue-1574 4/0
  • test-area-filter 16/0
  • test-xss-escape-sinks 34/0
  • test-packet-filter 92/0
  • test-aging 19/0
  • test-issue-1293 23/0
  • test-issue-1360 9/0
  • test-issue-1407 61/0
  • test-issue-1418 ×6, all 0 failed
  • test-issue-1438-marker-css-vars 14/0
  • test-issue-1488 24/0
  • test-issue-1633 21/0

• Pre-existing failures, identical on both:

  • test-map-clustering 4/1
  • test-frontend-helpers 705/2
  • test-issue-1438-customizer-mcrole 7/1
  • test-issue-1648-m3, -m6-final-sweep and -m6-lint-self fail (emoji scans of other files)

  The failure sets are identical, so the PR introduces no regressions. [F]

Browser

• I ran the new E2E against master, the head and the merge tree (above). [F]
• My own Playwright scenarios (reviewer probe, kept locally), with route interception to delay, hold and reorder responses and to navigate away mid-flight. The head passes 5/5, master fails 4/5 (S5 passes on master). [F]
  • S1: multi-path drawPacketRouteMulti, leave while resolve-hops is held.
  • S2: mock one area, mount the map 4 times, count live map.js AreaFilter listeners, then pick the area.
  • S3: selectReferenceNode race (A held, B immediate, then release A).
  • S4: random 0-1500 ms delay on every /api/* request plus 5 rapid leave/return cycles.
  • S5: initial node load superseded by a filter change while the first /api/observers is delayed.
• Screenshots with the pane expanded at 641x800, 1024x768 and 1280x600, on master and the head (kept locally). On master the controls cover the inspector input and toggle; on the head they sit inside the Leaflet area. At 641 px the 180 px-wide controls clip the "3-byte" pill. That is the same width and the same clipping as master, so it is not a regression. [F]

Performance and security

• Not a hot path. The added work is one integer comparison per await continuation or timer. Superseded node loads still complete their network requests, as before; only their results are dropped. [F]
• Removing the leaked AreaFilter listeners (S2) and the pane click listeners that piled up on every reload reduces work. As a side effect, a ?prefix= deep link is no longer re-submitted on every WS node reload (initMapSidePane now wires once per mount, map.js:2447-2448). [F]
• mapTimers is bounded: it removes each id as its timer fires and is cleared in destroy(). nodesLoadPromise is reset in destroy(). [F]
• No new innerHTML sink with data. The only innerHTML in the diff context is the existing static caret SVG. No new Go code, no map[string]interface{}, and cmd/server is untouched. [F]
• No new UI state, so no deep-link requirement. [F]
• Unrelated, pre-existing, out of scope:
  • packets.js:1679 also registers an AreaFilter.onChange listener on every mount without removing it (S2 shows 3 after 3 packets mounts).
  • area-filter.js interpolates a.label/a.key into innerHTML unescaped (operator config, not mesh data).
  • The PR's own note is correct: path-inspector.js:191 calls window.drawPacketRoute, but map.js exports only drawPacketRouteMulti (map.js:1413). [F]

Not verified

• I did not check real touch devices or tablets, or widths outside 641/800/1024/1280/1440. [K]
• I did not reproduce the CI flake of test-analytics-fluid-charts.js AC3 (5/5 green locally on both trees). I did not look for a root cause. [K]
• I tested deep-link combinations (?packet=, ?node=, ?prefix=) during a quick return only by reading the code (see M9/M12/M14), not in a browser. [A]
• I did not run scripts/check-xss-sinks.sh or eslint myself. I relied on reading the diff; the PR reports both as clean [T].
• The PR's "Not verified" section is honest as far as it goes. It does not mention that the listener cleanup, the selectReferenceNode guard and the multi-path guard have no automated test (Finding 1).

[dborup]

CI-note: Playwright-kørslen fejlede i test-charts-fluid-1058-e2e.js (AC3: layout reflows on resize … expected ≥2 cols at 1300px; got 1). Det er samme test, der fejlede intermitterende på PR #135 (dengang i 2560px-casen). Denne PR's egen E2E-test (test-issue-123-map-lifecycle-e2e.js) bestod i samme kørsel.

Testen dækker analytics-layout. Denne PR's CSS er afgrænset til #mapSidePane ~ .map-controls(-toggle) inden for @media (min-width: 641px) og rører ikke analytics. Lokalt bestod testen 20/20 mod denne branch og 20/20 mod origin/master d264716c. Jeg har genkørt det fejlede job én gang. Den kan ikke reproduceres på master, så jeg kalder den intermitterende, ikke baseline.

---

Generated by Claude Code

[dborup]

Review feedback addressed (commit da47a0b0)

1. Route view timer after destroy. Leaving the map with an open route threw TypeError: Cannot read properties of undefined (reading '_leaflet_pos'). The teardown timer in public/route-view.js ran invalidateSize on the map that destroy() had removed.
   • The route view now marks a map as removed on Leaflet's unload event, which map.remove() fires.
   • All of its map-touching timers go through mapTimeout(mapRef, fn, ms), which skips a removed map. That covers teardown/close/collapse invalidateSize, the mobile-sheet refit, the spider fan and the resize refit.
   • New E2E steps for desktop 1440x900 and tablet 1024x768 were red before the fix (the _leaflet_pos pageerror) and are green after.
2. Listener leaks in destroy(). destroy() in public/map.js now disconnects the data-theme MutationObserver and removes the mc-tile-provider-changed window listener.
   • The new E2E step mounts the map 6 times and counts only map.js's own callbacks (the ones that call _syncDarkTiles).
   • Before the fix it failed with 6 map tile-provider listeners after 6 mounts, want 1. After the fix there is 1 listener and 1 observer.
   • Two theme switches plus a provider event cause as many setUrl calls after 6 mounts as after 1, and leaving the map leaves 0 of each.
3. PR text, "Show on map". The "Not verified" item now says the claim is refuted in cleanup(path-inspector): remove unreachable showOnMap branch that calls undefined window.drawPacketRoute #143: the branch is unreachable dead code.
4. PR text, #mapPaneToggle. It is covered in route mode (route-view.css left: 320px). This is now listed as a known, pre-existing limitation, because it needs a layout decision and is not a trivial fix.

Numbers:

• test-issue-123-map-lifecycle-e2e.js: 11 passed / 4 failed before, 15/0 after. It is also 15/0 on a local merge with origin/master.
• Mutants, each caught:
  • route-view mapTimeout without the removal check: 12/3.
  • Observer kept: 14/1.
  • Listener kept: 14/1.
• Existing map suites unchanged:
  • 1374 route a11y 20/20
  • 1236 3/3
  • 1329 5/5
  • map-modal-fluid 10/10
  • path-inspector-coverage 10/10
  • nodes-pagination 3/3
  • 1418 ×6, 0 failed
  • 1420 tile providers 39/39
• check-xss-sinks.sh --diff: clean.

---

Generated by Claude Code