feat: one-click prune nodes outside geofilter (#669 M4)

[efiten]

Summary

• Adds POST /api/admin/prune-geo-filter endpoint — dry-run by default, ?confirm=true to permanently delete nodes outside the current geofilter polygon + buffer. Requires X-API-Key header.
• Adds Prune nodes section inside the GeoFilter customizer tab (write-access only, same writeEnabled gate as PUT). Preview lists affected nodes; Confirm delete removes them.
• Adds GetNodesForGeoPrune and DeleteNodesByPubkeys DB helpers.
• Updates docs/user-guide/geofilter.md — documents the UI button as primary workflow, CLI script as alternative.

Depends on M3 (feat/geofilter-m3-customizer, PR #736). Merge M3 first.

Test plan

• cd cmd/server && go test ./... — all pass
• Customizer GeoFilter tab without apiKey — Prune section not visible
• With apiKey + polygon active — Prune section visible
• Preview returns list of nodes outside polygon (no deletions)
• Confirm delete removes nodes, list clears
• POST /api/admin/prune-geo-filter without X-API-Key → 401
• POST /api/admin/prune-geo-filter with no polygon configured → 400

🤖 Generated with Claude Code

[Kpa-clawbot]

Review: one-click prune nodes outside geofilter (M4)

Good overall design: dry-run by default, API key required, no-GPS nodes always kept. Tests are thorough. A few items to address:

Must-fix

1. TOCTOU race between preview and confirm delete
The dry-run and confirm are two separate API calls. Between them, nodes could be ingested (or coordinates updated). The confirm call re-evaluates the filter independently — it doesn't use the preview list. This means:

• Nodes not shown in preview could be deleted on confirm (ingested between calls, with out-of-bounds coords)
• Nodes shown in preview might not be deleted (if their coords were updated to be inside)

The user sees a preview list, clicks "Delete N nodes", but a different set gets deleted. This violates the principle of least surprise for a destructive operation.

Fix: Either (a) pass the pubkey list from the preview to the confirm call (server-side or client-side), so only previewed nodes are deleted, or (b) return the actual deleted list in the confirm response so the UI shows what really happened, and document this behavior.

2. No cascade/cleanup of related data
DeleteNodesByPubkeys does DELETE FROM nodes WHERE public_key IN (...). If other tables reference these pubkeys (packets with source/dest fields, any future neighbor/edge table), orphaned references will remain. This isn't breaking today (no FK constraints), but it's technical debt that will bite when the schema evolves.

Fix: At minimum, add a code comment documenting that this only deletes from the nodes table and does not cascade. Ideally, also delete from any tables that reference the node pubkey (check transmissions/observations).

Should-fix

3. Irreversibility — no safety net
This is a permanent DELETE with no backup, no soft-delete, no undo. For a one-click operation in a UI panel, that's aggressive. The confirm() dialog is the only guard.

Suggestion: Log the deleted pubkeys + names at INFO level (not just the count) so an operator can at least see what was removed. The nodes will re-appear if the mesh re-advertises them, but historical metadata (first_seen, advert_count) is lost permanently.

4. Frontend prune confirm doesn't show the re-evaluated list
_gfPruneConfirm sends confirm=true but the response only contains { deleted: N }, not the list of what was deleted. If the set changed between preview and confirm (per issue #1), the user has no visibility into what actually happened.

Nits

• handlePruneGeoFilter calls NodePassesGeoFilter(lat, lon, ...) where lat/lon are concrete float64 (defaulting to 0 when nil). This works correctly because PassesFilter has the lat == 0 && lon == 0 escape hatch, but it's semantically confusing — a nil coordinate is being passed as 0.0 wrapped in interface{}. Consider checking n.Lat == nil directly and skipping the filter call entirely for no-GPS nodes.
• geofilter-builder.html loads Leaflet from unpkg CDN — consider using the same CDN/version as the rest of the app for consistency and offline scenarios.

What's good

• Dry-run default is the right call — safe by default
• requireAPIKey middleware + IsWeakAPIKey gate on the UI controls
• Tests cover all the important paths: dry-run, confirm, no-filter-400, no-key-401
• No-GPS nodes correctly preserved (verified through the full call chain)
• Atomic config write (temp file + rename)
• Documentation is comprehensive

Verdict: Requesting changes for the TOCTOU issue (#1). The rest can be addressed in follow-up, but #1 is a data safety concern for a destructive endpoint.

[Kpa-clawbot]

Re-review: PR #738 — geo-prune M4 (post b3d29be1)

Verdict: COMMENT — ready after rebase

Original review items (2026-04-15)

#	Item	Status
1	TOCTOU race between preview and confirm — different node set could be deleted	✅ Fixed. Confirm now requires pubkeys in request body; server intersects with current outside-filter set (routes.go:2565-2580). Frontend sends preview pubkeys in _gfPruneConfirm. Solid fix.
2	No cascade/cleanup comment on DeleteNodesByPubkeys	✅ Fixed. Comment added in db.go:2355-2357 documenting no FK cascade.
3	Irreversibility — no audit trail	✅ Fixed. Each deleted node logged individually at INFO: log.Printf("[geo-prune] deleted node %q (%s)") (routes.go:2596).
4	Confirm response didn't show what was deleted	✅ Fixed. Response now includes "nodes": toDelete with full list (routes.go:2600).
5	Nil coords passed as 0.0 to filter	✅ Fixed. NodeForGeoPrune uses *float64 pointers; handler checks n.Lat == nil before calling NodePassesGeoFilter (routes.go:2554).
6	Leaflet CDN in geofilter-builder.html	Minor — still uses unpkg, rest of app uses CDN link in index.html. Non-blocking.

New findings (djb + munger)

No must-fix items found. The TOCTOU fix is well-designed — the intersection approach means confirm can never delete more than what was previewed, even if the polygon or GPS data changes between calls.

Observations (informational, not blocking):

• confirm=true is a query param on a POST — acceptable since the body carries the pubkey payload and the endpoint requires X-API-Key. A future improvement could move it to the body.
• Single DELETE FROM nodes WHERE public_key IN (...) is atomic — no partial-delete risk.
• Same X-API-Key gates all admin endpoints. Acceptable for a single-operator tool; if multi-operator access is added later, consider per-endpoint permissions.
• No soft-delete/undo — nodes re-appear when the mesh re-advertises them, but historical metadata (first_seen, advert_count) is lost. Documented behavior; operator is warned via confirm() dialog.

Merge verdict

Ready after rebase. PR is currently CONFLICTING against master. Depends on PR #736 (M3) — merge that first, then rebase this. Zero must-fix items remain.

[Kpa-clawbot]

@efiten same comment as elsewhere - please clean up this PR and get it ready for merging or abandon it.