[dborup]

Relates to #364.

Change

Preserve both existing light tile configuration shapes, tiles.light and map.tiles.lightUrl, in the shared registry resolver. Precedence is:

1. A valid explicit browser provider choice, including explicit OSM.
2. A valid configured/programmatic provider default.
3. The legacy light URL.
4. The built-in fallback.

The override is runtime metadata, not a new setting. A legacy URL has no registry vendor ID, overlay or inversion metadata, so inset maps retain their historical generic attribution rather than borrowing an unrelated provider's credit/filter. Dark resolution is unchanged.

Verification

• Identical final regression test against immutable base: 64 passed / seven failed. Candidate: 71/71 passed.
• Parent reran all 233 registered unit-test files: 233 passed, zero failed.
• Existing provider and inset helper suites: 39/39 and 6/6.
• Parent ran real Chromium with offline network interception: 21/21 checks. Both config shapes make actual synthetic legacy tile requests; an intercepted HTTP 503 followed by explicit OSM selection/reload recovers correctly.
• Additional 15-surface browser matrix passed for each legacy configuration shape.
• Syntax, ESLint (no errors; pre-existing warning), actual committed XSS diff gate and diff checks passed.
• Independent exact-tree review: PASS.

Limitations

No new automatic tile-error fallback is added. Explicit selection requires the existing remount/reload lifecycle. Unknown legacy vendors retain historical generic attribution. Zoom-cap handling and simultaneous mixed use of deprecated and modern configuration shapes are unchanged. No new config, dependencies or workflow changes.

Scope and safety

• One regular commit on verified master ff9a501c17396aada504e87d44c0f503e2740b8e.
• Draft; auto-merge is off. No merge, deployment, live-host access or upstream writes.
• Go server and ingestor suites passed on the verified base (58.9 s / 109.5 s); all backend/module files remain byte-identical in this change.

[dborup-agent]

Review — CS-MacBook PR#366 — head 4fa356a

Dom: APPROVE with nits

Independent, read-only review of the #364 follow-up ("preserve configured legacy light tile URL overrides"). Head verified via git ls-remote before and after the run (unchanged). Merge into current origin/master 8eb89720 is clean (git merge-tree --write-tree → single tree, no conflicts). All work done on git archive snapshots of the head and the merged tree in scratch; no checkout was pulled or reset. Evidence tags: [T] test/tool run, [A] architecture/code reasoning, [K] known/accepted.

Findings

#	Sev	Area	Finding
1	nit	attribution	A legacy raw URL carries no vendor metadata, so an operator who points map.tiles.lightUrl/tiles.light at e.g. OpenTopoMap gets the generic © OpenStreetMap contributors credit (no CC-BY-SA/SRTM), by design. Matches pre-#344 behavior and is explicitly accepted by the issue ("unknown legacy vendors retain historical generic attribution"). [K][A]
2	nit	zoom	The legacy-URL spec hard-codes maxZoom to osm-standard's 18. A custom light provider that supports z19 is clipped one level. Within the issue's "zoom-cap handling unchanged" scope; not separately tested. [A]
3	obs	scope	The dark legacy override (tiles.dark/map.tiles.darkUrl) is still masked by the registry default the same way light was — no legacy branch exists for dark. Out of scope for #364 (light-only); noted for completeness, not a defect in this PR. [A]

No correctness defects found. Items 1–2 are documented/accepted limitations; item 3 is an out-of-scope observation.

Answers to the review points

Precedence (browser choice → configured default → legacy URL → built-in fallback). Confirmed by code trace and tests. getSelectedLightId() resolves localStorage → _cfg.lightDefault → _serverDefaultLight → null; the legacy branch in getTileSpec('light') fires only when getSelectedLightId() is null and MC_LIGHT_TILE_URL_OVERRIDE is a non-empty string; otherwise getActiveLightId() = getSelectedLightId() || DEFAULT_ID_LIGHT. So each level strictly dominates the next. Explicit OSM selection is handled correctly: a stored osm-standard makes getSelectedLightId() non-null, so an explicit choice of the fallback id still beats the legacy URL. [A][T] My two unit mutants (below) independently exercise levels 1–2 and 3–4.

No silent change for existing installs. getActiveLightId() is a pure refactor — identical resolution chain to master. MC_LIGHT_TILE_URL_OVERRIDE defaults to null and is assigned only from cfg.tiles.light / cfg.map.tiles.lightUrl, mirroring the existing TILE_LIGHT assignment right above it (roles.js). An install that configures its light provider by id (lightDefault) never sets the override and never enters the legacy branch. Our own stg/prod shape (Esri dark via darkDefault, OpenTopoMap light via lightDefault) therefore resolves through the registry unchanged, with the inset getting the proper OpenTopoMap credit (id != null). Verified by reasoning [A]; the default/no-config path is covered live by the e2e (OSM both themes) [T]. I did not inspect the live prod/stg config file (no prod access) — that part is [A].

Inset (node-detail) maps and dark/light. In the legacy case getTileSpec returns id:null; getActiveTileProvider() then returns null so the inset keeps generic attribution and clears any filter. The unit suite drives the real nodes.js _applyTilesToNodeMap and asserts it does not borrow an unused provider's attribution/refUrl/invertFilter. Dark is untouched (no legacy branch for dark). [T][A]

Browser check against a local server with both legacy shapes. Done — see e2e below. Both tiles.light and map.tiles.lightUrl, in light and dark, drive real Chromium network requests; a synthetic 503 on the legacy host followed by explicit OSM selection + reload recovers. [T]

Hygiene. cmd/server untouched; no Go files touched at all, so no new map[string]interface{} outside tests. No hardcoded colors added (new literals are a URL attribution string and invertFilter:null). scripts/check-xss-sinks.sh --diff origin/master → exit 0, no DOM-sink lines added. No GitHub closing keywords (body: "Relates to #364"; commit headline/body carry none). Commit author and committer are dborup <kontakt@meshview.dk>. [T]

Tests (merged tree vs origin/master 8eb8972, all green)

Suite	Result
node test-frontend-helpers.js	712 passed / 0 [T]
node test-issue-332-tile-provider-fallback.js	71 passed / 0 (incl. 15 new #364 cases) [T]
sh test-all.sh	235 files, all passed [T]
cmd/server go test ./...	ok 41.8s [T]
cmd/ingestor go test ./...	ok 109s [T]
test-issue-332-no-carto-tiles-e2e.js (local Go server :13901, e2e-fixture.db prepared as in CI: freshen → migrate → seed-2073 → seed-199)	21 passed / 0 [T]

CI on the PR: Go Build & Test, Playwright E2E, and Docker build all pass; Deploy/Release/Badges skipped as expected for a non-release PR. No flaky reruns needed (#256/#267 not hit).

Mutants (mine)

1. Remove the legacy branch in getTileSpec (revert to master): unit suite → 6 red — exactly the "legacy URL applies" + fallback-recovery cases. [T]
2. Drop the !getSelectedLightId() guard so legacy would override an explicit/configured choice: unit suite → 6 red — the "browser choice wins", "configured provider default wins", and "programmatic server default wins" cases (including explicit osm-standard). [T]
3. Remove the legacy branch, real-browser (rebuilt public/, restarted server, reran e2e): 4 red — both config shapes' "node inset uses legacy URL" and both "explicit OSM recovery" steps; the dark-unchanged steps stayed green. Confirms the e2e is not passing vacuously. [T]

Together the mutants cover all four precedence levels at both the resolver and the real-browser layer. Source was restored to the head blob and the server stopped afterward.

Not verified / caveats

• Live stg/prod config file not inspected (no prod access); the "our config is unchanged" claim is architectural [A], resting on the provider-id (lightDefault) form.
• maxZoom clamp for legacy URLs (finding fix(frontend): replace legacy scope confirmation glyphs #2) not exercised.
• Mixed config (both tiles.light and map.tiles.lightUrl set at once): the override ternary prefers cfg.tiles and ignores map.tiles.lightUrl, consistent with the existing TILE_LIGHT logic directly above it — not a regression, not separately tested. [A]
• "Fork-guards 9 and 1" from the brief: I could not map that count to a concrete token in this diff. The only guard the PR adds is a defensive try/catch around MC_getTileSpec in getActiveTileProvider, alongside the existing typeof … === 'function' checks; no automation/CI guards were added or removed.