From a3a4052cd3b7180790614efc71d82ebe9f31780e Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:25:15 +0200
Subject: [PATCH 1/7] fix(proxy): activate a newly discovered node when the
 active one is dead

The network client auto-selects only the very first node it ever sees, and
removal-failover needs a surviving node to switch to. When the last node
vanishes (e.g. its process crashes) act_id keeps pointing at the dead node,
so a node added afterwards was never activated: the actonly subscriptions
(ACDATA/ROUTEDATA) stayed bound to the dead node, no traffic flowed, and the
data-flow timeout tore down an otherwise live connection.

_on_node_added now re-activates when the client's active node is gone, and
the local `import traceback` calls are hoisted to a module-level import.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 WebATM/proxy/managers/node_manager.py | 67 ++++++++++++++++-----------
 tests/test_node_manager.py            | 39 ++++++++++++++++
 2 files changed, 78 insertions(+), 28 deletions(-)

diff --git a/WebATM/proxy/managers/node_manager.py b/WebATM/proxy/managers/node_manager.py
index e3f69d2..2a618f2 100644
--- a/WebATM/proxy/managers/node_manager.py
+++ b/WebATM/proxy/managers/node_manager.py
@@ -2,6 +2,7 @@
 
 import threading
 import time
+import traceback
 
 from ...bluesky_client import safe_decode, seqid2idx, seqidx2id
 from ...logger import get_logger
@@ -65,8 +66,6 @@ def _emit_active_node_poly_data(self):
             )
         except Exception as e:
             logger.error(f" Error emitting active node POLY/POLYLINE data: {e}")
-            import traceback
-
             traceback.print_exc()
 
     def _emit_shapes(self, active_node_id, data_by_node, event):
@@ -89,27 +88,26 @@ def _emit_shapes(self, active_node_id, data_by_node, event):
             logger.debug(f"Emitted empty {event} data to clear")
 
     def _on_node_added(self, node_id):
-        """Callback when a new node is discovered."""
+        """Callback when a new node is discovered.
+
+        Tracks the node (keyed by hex string, matching SIMINFO sender IDs),
+        flips the proxy to connected on the first node, and re-activates the
+        client when its recorded active node no longer exists.
+        """
         try:
-            # Hex string keys for consistency with SIMINFO
             node_id_str = id2str(node_id)
 
             if node_id_str not in self.proxy.tracked_nodes:
                 server_id = node_id[:-1] + seqidx2id(0)
-                if (
-                    server_id not in self.proxy.bluesky_client.servers
-                ):  # Check standalone proxy's known servers
+                if server_id not in self.proxy.bluesky_client.servers:
                     server_id = b"0"  # Ungrouped
                 if server_id not in self.proxy.tracked_servers:
                     self._on_server_added(server_id)
 
-                node_num = seqid2idx(node_id[-1])
-
-                # Store using hex string key for consistency with SIMINFO
                 self.proxy.tracked_nodes[node_id_str] = {
-                    "node_id": node_id,  # Keep original binary for internal use
-                    "node_id_str": node_id_str,  # Hex string for display
-                    "node_num": node_num,
+                    "node_id": node_id,  # original binary, for internal use
+                    "node_id_str": node_id_str,
+                    "node_num": seqid2idx(node_id[-1]),
                     "server_id": server_id,
                     "status": "init",
                     "time": "00:00:00",
@@ -119,36 +117,49 @@ def _on_node_added(self, node_id):
                     f"Node {safe_decode(node_id)} added (total: {len(self.proxy.tracked_nodes)})"
                 )
 
-                # Update connection status immediately when nodes are detected
-                if not self.proxy.was_connected and len(self.proxy.tracked_nodes) > 0:
+                self._reactivate_if_active_node_gone(node_id)
+
+                if not self.proxy.was_connected:
                     self.proxy.was_connected = True
-                    # Start the data-flow timeout clock from "first node
-                    # appeared" (no data could arrive before a node existed).
-                    # This handler runs inside bluesky_client.update() and flips
-                    # was_connected first, so the network timer's own reset (only
-                    # runs while `not was_connected`) is skipped — reset here too,
-                    # else the stale start_client() timestamp times out at once.
+                    # Restart the data-flow timeout clock from "first node
+                    # appeared". This runs inside bluesky_client.update() and
+                    # flips was_connected before the network timer's own reset
+                    # (guarded on `not was_connected`) can run, so without this
+                    # a stale start_client() timestamp times out immediately.
                     self.proxy.last_successful_update = time.time()
                     logger.info(" Connection established")
                     self.proxy._emit_connection_status(True)
 
-                # The standalone proxy auto-selects the first node, so we don't need to do it manually here
-                # Emit updated node list to connected clients
                 if self.proxy.running:
                     self._emit_node_info()
         except Exception as e:
             logger.error(f" Error in _on_node_added: {e}")
-            import traceback
-
             traceback.print_exc()
 
+    def _reactivate_if_active_node_gone(self, node_id):
+        """Activate a newly discovered node if the client's active node is dead.
+
+        The network client auto-selects only the very first node it ever sees;
+        after that, ``_failover_active_node`` re-activates a survivor when the
+        active node is removed. But when the last node vanishes (e.g. its
+        process crashes) there is no survivor: ``act_id`` keeps pointing at the
+        dead node, and a node added afterwards would never be activated. The
+        actonly topics (ACDATA/ROUTEDATA) would stay subscribed to the dead
+        node, no traffic would flow, and the data-flow timeout would tear down
+        a live connection. Activating the newcomer here re-subscribes those
+        topics and resumes the data flow.
+        """
+        client = self.proxy.bluesky_client
+        if client.act_id is None or client.act_id not in client.nodes:
+            logger.info(
+                f"No live active node; activating new node {safe_decode(node_id)}"
+            )
+            client.actnode(node_id)
+
     def _on_server_added(self, server_id):
         """Callback when a server is discovered."""
         if server_id not in self.proxy.tracked_servers:
-            # Simple server tracking - just store the ID
             self.proxy.tracked_servers[server_id] = {"server_id": server_id}
-
-            # Emit updated server list to connected clients
             if self.proxy.running:
                 self._emit_node_info()
 
diff --git a/tests/test_node_manager.py b/tests/test_node_manager.py
index ed46fe7..6ace317 100644
--- a/tests/test_node_manager.py
+++ b/tests/test_node_manager.py
@@ -137,6 +137,45 @@ def test_check_node_shutdown_uses_proxy_connection_mgr(self, proxy, monkeypatch)
         assert reasons == ["All nodes removed (server shutdown)"]
 
 
+class TestReactivateOnNodeAdded:
+    """A node discovered while the client's active node is dead must become
+    the new active node.
+
+    Regression test: the network client auto-selects only the very first node
+    it ever sees, and removal-failover needs a survivor. When the last node
+    vanishes (e.g. its process crashes) and a node is added afterwards,
+    ``act_id`` kept pointing at the dead node, the actonly subscriptions
+    (ACDATA/ROUTEDATA) never moved to the new node, and the data-flow timeout
+    tore down a live connection ~10s later."""
+
+    def test_node_added_with_stale_act_id_reactivates(self, proxy, fake_client):
+        dead_bytes = b"\x01\x02\x03\x04\x81"
+        new_bytes = b"\x01\x02\x03\x04\x82"
+        proxy.bluesky_client = fake_client
+        proxy.running = True
+        proxy.was_connected = True
+        fake_client.act_id = dead_bytes  # removed node; not in client.nodes
+        fake_client.nodes = {new_bytes}
+
+        proxy.node_mgr._on_node_added(new_bytes)
+
+        assert fake_client.act_id == new_bytes
+
+    def test_node_added_with_live_act_id_keeps_it(self, proxy, fake_client):
+        active_bytes = b"\x01\x02\x03\x04\x81"
+        new_bytes = b"\x01\x02\x03\x04\x82"
+        proxy.bluesky_client = fake_client
+        proxy.running = True
+        proxy.was_connected = True
+        proxy.tracked_nodes[active_bytes.hex()] = {"node_id": active_bytes}
+        fake_client.act_id = active_bytes
+        fake_client.nodes = {active_bytes, new_bytes}
+
+        proxy.node_mgr._on_node_added(new_bytes)
+
+        assert fake_client.act_id == active_bytes
+
+
 class TestNodeRemoval:
     def test_on_node_removed_deletes_tracked_node(self, proxy):
         node_bytes = b"\x01\x02\x03\x04\x81"

From b5c627fe8cd4da5a12dec32cb54d5405c996010e Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:25:27 +0200
Subject: [PATCH 2/7] fix(proxy): serialize node_info in the initial_data
 snapshot

get_current_data() built its own node_info payload straight from the internal
stores, which keep binary node/server IDs. Socket.IO ships those as binary
attachments and the frontend NodeData type can't read them, so a client that
joined between node_info events saw unusable node data.

The serialization used by the node_info event is extracted into
NodeManager.serialize_node_info() and both paths now go through it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 WebATM/proxy/managers/data_manager.py | 12 +---
 WebATM/proxy/managers/node_manager.py | 95 +++++++++++++--------------
 tests/test_data_manager.py            | 21 ++++++
 3 files changed, 70 insertions(+), 58 deletions(-)

diff --git a/WebATM/proxy/managers/data_manager.py b/WebATM/proxy/managers/data_manager.py
index 4026b31..bf6e89a 100644
--- a/WebATM/proxy/managers/data_manager.py
+++ b/WebATM/proxy/managers/data_manager.py
@@ -204,8 +204,6 @@ def get_current_data(self) -> dict[str, Any]:
         else:
             logger.debug(" No active node - not including any shapes in initial data")
 
-        from ...bluesky_client import safe_decode
-
         return {
             "traffic_data": self.proxy.traffic_data,
             "sim_data": self.proxy.sim_data,
@@ -218,13 +216,7 @@ def get_current_data(self) -> dict[str, Any]:
                 "server_ip": self.proxy.server_ip,
                 "last_update": self.proxy.last_successful_update,
             },
-            "node_info": {
-                "nodes": self.proxy.tracked_nodes.copy(),
-                "servers": {
-                    safe_decode(k): v for k, v in self.proxy.tracked_servers.items()
-                },
-                "active_node": active_node_id,
-                "total_nodes": len(self.proxy.tracked_nodes),
-            },
+            # Same serialized shape as the node_info event (no raw bytes).
+            "node_info": self.proxy.node_mgr.serialize_node_info(),
             "timestamp": time.time(),
         }
diff --git a/WebATM/proxy/managers/node_manager.py b/WebATM/proxy/managers/node_manager.py
index 2a618f2..4a9e20d 100644
--- a/WebATM/proxy/managers/node_manager.py
+++ b/WebATM/proxy/managers/node_manager.py
@@ -38,8 +38,7 @@ def _get_safe_active_node(self):
             return None
 
         try:
-            # act_id is raw bytes in the network client; tracked_nodes is
-            # keyed by the hex-string form.
+            # act_id is raw bytes; tracked_nodes is keyed by hex strings.
             active_id_str = id2str(self.proxy.bluesky_client.act_id)
             if active_id_str in self.proxy.tracked_nodes:
                 return active_id_str
@@ -171,15 +170,14 @@ def _on_node_removed(self, node_id):
             self._failover_active_node(node_id)
             self._emit_node_info()
 
-        # Check if all nodes have been removed - this indicates server shutdown
-        # Add a small delay to avoid false positives during normal node transitions
+        # All nodes gone usually means server shutdown; re-check after a short
+        # delay to avoid false positives during normal node transitions.
         if (
             len(self.proxy.tracked_nodes) == 0
             and self.proxy.was_connected
             and self.proxy.running
         ):
             logger.warning(" All nodes removed - checking for server shutdown...")
-            # Use a timer to check again in a moment to confirm it's really a shutdown
             threading.Timer(1.0, self._check_node_shutdown).start()
 
     def _failover_active_node(self, removed_node_id):
@@ -224,51 +222,52 @@ def _on_server_removed(self, server_id):
             del self.proxy.tracked_servers[server_id]
             self._emit_node_info()
 
+    def serialize_node_info(self):
+        """Build the JSON-serializable ``node_info`` payload.
+
+        Decodes the binary node/server IDs kept in ``tracked_nodes`` and
+        ``tracked_servers`` into the string forms the frontend expects
+        (``NodeData`` in ``frontend/src/data/types.ts``). Used for both the
+        ``node_info`` Socket.IO event and the ``initial_data`` snapshot.
+
+        Returns:
+            dict: ``nodes``, ``servers``, ``active_node`` and ``total_nodes``.
+        """
+        nodes_data = {}
+        for node_id_str, tracked in self.proxy.tracked_nodes.items():
+            node_data = tracked.copy()
+            if "node_id" in node_data:
+                node_data["node_id"] = safe_decode(node_data["node_id"])
+            if "server_id" in node_data:
+                raw_server_id = node_data["server_id"]
+                node_data["server_id"] = safe_decode(raw_server_id)
+                node_data["server_id_hex"] = id2str(raw_server_id)
+                node_data["server_id_raw"] = str(raw_server_id)
+            nodes_data[node_id_str] = node_data
+
+        servers_data = {}
+        for server_id, tracked in self.proxy.tracked_servers.items():
+            server_data = tracked.copy()
+            if "server_id" in server_data:
+                server_data["server_id"] = safe_decode(server_data["server_id"])
+            servers_data[safe_decode(server_id)] = server_data
+
+        return {
+            "nodes": nodes_data,
+            "servers": servers_data,
+            "active_node": self._get_safe_active_node(),
+            "total_nodes": len(self.proxy.tracked_nodes),
+        }
+
     def _emit_node_info(self):
         """Emit current node and server information to connected clients."""
-        if self.proxy.socketio and self.proxy.connected_clients > 0:
-            try:
-                # Convert node data for JSON serialization
-                nodes_data = {}
-                for k, v in self.proxy.tracked_nodes.items():
-                    # k is already a hex string, v contains node data
-                    # Make a copy and ensure all values are JSON serializable
-                    node_data = v.copy()
-                    if "node_id" in node_data:
-                        node_data["node_id"] = safe_decode(node_data["node_id"])
-                    if "server_id" in node_data:
-                        # Include decoded, hex, and raw server ID
-                        raw_server_id = node_data["server_id"]
-                        node_data["server_id"] = safe_decode(raw_server_id)
-                        node_data["server_id_hex"] = id2str(raw_server_id)
-                        node_data["server_id_raw"] = str(
-                            raw_server_id
-                        )  # Raw byte string representation
-                    nodes_data[k] = node_data  # k is already the hex string
-
-                servers_data = {}
-                for k, v in self.proxy.tracked_servers.items():
-                    key = safe_decode(k)
-                    server_data = v.copy()
-                    if "server_id" in server_data:
-                        server_data["server_id"] = safe_decode(server_data["server_id"])
-                    servers_data[key] = server_data
-
-                # Get active node safely
-                active_node = self._get_safe_active_node()
-
-                node_info = {
-                    "nodes": nodes_data,
-                    "servers": servers_data,
-                    "active_node": active_node,
-                    "total_nodes": len(self.proxy.tracked_nodes),
-                }
-                self.proxy.socketio.emit("node_info", node_info)
-            except Exception as e:
-                logger.error(f" Error emitting node info: {e}")
-                import traceback
-
-                traceback.print_exc()
+        if not (self.proxy.socketio and self.proxy.connected_clients > 0):
+            return
+        try:
+            self.proxy.socketio.emit("node_info", self.serialize_node_info())
+        except Exception as e:
+            logger.error(f" Error emitting node info: {e}")
+            traceback.print_exc()
 
     def actnode(self, node_id):
         """Select the active simulation node via the network client.
diff --git a/tests/test_data_manager.py b/tests/test_data_manager.py
index 8ee8074..cda0155 100644
--- a/tests/test_data_manager.py
+++ b/tests/test_data_manager.py
@@ -124,3 +124,24 @@ def test_get_current_data_uses_proxy_node_mgr(self, proxy, monkeypatch):
         )
         data = proxy.data_mgr.get_current_data()
         assert data["node_info"]["active_node"] == "deadbeef81"
+
+    def test_get_current_data_node_info_is_serialized(self, proxy):
+        """The initial_data snapshot must carry the same JSON-safe node_info
+        payload as the node_info event — no raw bytes (which Socket.IO would
+        ship as binary attachments and the frontend NodeData type can't use)."""
+        node_bytes = b"\x01\x02\x03\x04\x81"
+        proxy.tracked_nodes[node_bytes.hex()] = {
+            "node_id": node_bytes,
+            "server_id": b"SRV\x80\x80",
+        }
+        proxy.tracked_servers[b"SRV\x80\x80"] = {"server_id": b"SRV\x80\x80"}
+
+        node_info = proxy.data_mgr.get_current_data()["node_info"]
+
+        node_entry = node_info["nodes"][node_bytes.hex()]
+        assert isinstance(node_entry["node_id"], str)
+        assert isinstance(node_entry["server_id"], str)
+        assert "server_id_hex" in node_entry
+        assert all(isinstance(k, str) for k in node_info["servers"])
+        # The proxy's internal store keeps its binary IDs untouched.
+        assert proxy.tracked_nodes[node_bytes.hex()]["node_id"] == node_bytes

From b999efc30369e33c58dd7d290bdd843b308c7438 Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:25:37 +0200
Subject: [PATCH 3/7] fix(connection): suppress phantom reconnects and stale
 state after a disconnect

Three related connection-status defects:

- A deliberate disconnect (QUIT, server_disconnected, Stop in the integrated
  build) left the armed no-data timer running, so it fired up to 5s later and
  echoed a spurious "No data received - connection may be lost" warning.
  setBlueSkyConnected(false) now cancels the timer and clears receivingData,
  which also fixes receivingData sticking true forever after a WebSocket drop.
- Data events already in flight when the disconnect landed flipped the status
  straight back to connected, re-arming that timer and flashing a phantom
  "Connected". expectDisconnect() opens a short grace window in which data is
  ignored; SocketManager also drops in-flight siminfo/acdata frames so they
  can't repaint traffic the disconnect just cleared. Data still arriving after
  the window (i.e. the disconnect request failed) reconnects as usual.
- SettingsModal never propagated the configured host, so the header status
  string reported its 'localhost' default forever.

Also drops the unused ConnectionState duplication (webSocketState /
blueSkyState mirrored the booleans) and dead accessors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/core/ConnectionStatusService.test.ts  |  77 ++++++-
 frontend/src/core/ConnectionStatusService.ts  | 218 ++++++------------
 frontend/src/core/SocketManager.ts            |   9 +-
 frontend/src/data/CommandHandler.test.ts      |   6 +-
 frontend/src/data/CommandHandler.ts           |   4 +-
 frontend/src/ui/SettingsModal.ts              |   3 +
 6 files changed, 170 insertions(+), 147 deletions(-)

diff --git a/frontend/src/core/ConnectionStatusService.test.ts b/frontend/src/core/ConnectionStatusService.test.ts
index 07831ad..ae64365 100644
--- a/frontend/src/core/ConnectionStatusService.test.ts
+++ b/frontend/src/core/ConnectionStatusService.test.ts
@@ -76,7 +76,6 @@ describe('ConnectionStatusService', () => {
             service.setWebSocketConnected(true);
             const status = service.getStatus();
             expect(status.webSocketConnected).toBe(true);
-            expect(status.webSocketState).toBe('connected');
             expect(echoManager.success).toHaveBeenCalledWith('Connected to WebATM server');
         });
 
@@ -98,6 +97,17 @@ describe('ConnectionStatusService', () => {
             expect(service.isBlueSkyConnected()).toBe(false);
             expect(service.isFullyConnected()).toBe(false);
         });
+
+        it('a WebSocket disconnect also clears receivingData', () => {
+            // Regression: receivingData used to stay true forever after a
+            // WebSocket drop, because the guarded timeout path skipped it.
+            service.setWebSocketConnected(true);
+            service.onSimInfoReceived();
+            expect(service.isReceivingData()).toBe(true);
+
+            service.setWebSocketConnected(false);
+            expect(service.isReceivingData()).toBe(false);
+        });
     });
 
     describe('BlueSky state via data reception', () => {
@@ -126,6 +136,71 @@ describe('ConnectionStatusService', () => {
                 expect.stringContaining('No data received'));
         });
 
+        it('an explicit disconnect cancels the pending no-data timer', () => {
+            // Regression: after a deliberate disconnect (QUIT,
+            // server_disconnected, Stop in the integrated build) the armed
+            // 5s timer used to fire anyway and echo a spurious
+            // "No data received ... connection may be lost" warning.
+            service.onSimInfoReceived(); // arms the 5s timer
+
+            service.setBlueSkyConnected(false); // e.g. QUIT
+            expect(service.isReceivingData()).toBe(false);
+
+            (echoManager.warning as ReturnType<typeof vi.fn>).mockClear();
+            vi.advanceTimersByTime(10000);
+            expect(echoManager.warning).not.toHaveBeenCalledWith(
+                expect.stringContaining('No data received'));
+        });
+
+        it('a deliberate disconnect ignores in-flight data during the grace window', () => {
+            // Regression: after QUIT, data events already buffered in the
+            // socket flipped the status straight back to "Connected" and
+            // re-armed the timer, producing a phantom connect flash and a
+            // spurious warning 5s later.
+            const nowSpy = vi.spyOn(Date, 'now');
+            nowSpy.mockReturnValue(1000);
+            service.onSimInfoReceived();
+
+            service.expectDisconnect(); // QUIT
+            service.setBlueSkyConnected(false);
+
+            // A trailing acdata event arrives 100ms later - ignored.
+            nowSpy.mockReturnValue(1100);
+            service.onAircraftDataReceived();
+            expect(service.isBlueSkyConnected()).toBe(false);
+
+            (echoManager.warning as ReturnType<typeof vi.fn>).mockClear();
+            vi.advanceTimersByTime(10000);
+            expect(echoManager.warning).not.toHaveBeenCalledWith(
+                expect.stringContaining('No data received'));
+        });
+
+        it('data still flowing after the grace window reconnects as usual', () => {
+            const nowSpy = vi.spyOn(Date, 'now');
+            nowSpy.mockReturnValue(1000);
+            service.onSimInfoReceived();
+
+            service.expectDisconnect(); // disconnect request that failed
+            service.setBlueSkyConnected(false);
+
+            // Data keeps arriving well past the 2s grace window.
+            nowSpy.mockReturnValue(4000);
+            service.onAircraftDataReceived();
+            expect(service.isBlueSkyConnected()).toBe(true);
+        });
+
+        it('exposes the grace window so data consumers can drop in-flight frames', () => {
+            const nowSpy = vi.spyOn(Date, 'now');
+            nowSpy.mockReturnValue(1000);
+            expect(service.isInDisconnectGrace()).toBe(false);
+
+            service.expectDisconnect();
+            expect(service.isInDisconnectGrace()).toBe(true);
+
+            nowSpy.mockReturnValue(4000); // past the 2s default
+            expect(service.isInDisconnectGrace()).toBe(false);
+        });
+
         it('continuous data keeps the connection alive past the timeout window', () => {
             service.onSimInfoReceived();
             for (let i = 0; i < 5; i++) {
diff --git a/frontend/src/core/ConnectionStatusService.ts b/frontend/src/core/ConnectionStatusService.ts
index ecabfa7..7aec558 100644
--- a/frontend/src/core/ConnectionStatusService.ts
+++ b/frontend/src/core/ConnectionStatusService.ts
@@ -1,16 +1,9 @@
 /**
- * Connection Status Service
+ * Centralized singleton service holding every connection state: the WebSocket
+ * to the WebATM server, the BlueSky server connection, and data reception.
  *
- * Centralized singleton service for managing all connection states in the application.
- * This service tracks:
- * - WebSocket connection to WebATM server
- * - BlueSky server connection status (derived from receiving data)
- * - Data reception status
- *
- * The connection status should be available across all TypeScript classes.
- *
- * Key concept: As long as we're receiving any data (nodeinfo, siminfo, or acdata),
- * we're connected to BlueSky server. If no data is received for DATA_TIMEOUT_MS,
+ * Key concept: receiving any server data (nodeinfo, siminfo, acdata, shapes)
+ * proves the BlueSky connection is up. If nothing arrives for DATA_TIMEOUT_MS,
  * the connection is considered lost.
  */
 
@@ -18,16 +11,12 @@ import { echoManager } from '../ui/EchoManager';
 import { logger } from '../utils/Logger';
 import { EventEmitter } from '../utils/events';
 
-export type ConnectionState = 'disconnected' | 'connecting' | 'connected' | 'error';
-
 export interface ConnectionStatusData {
     // WebSocket connection to WebATM
     webSocketConnected: boolean;
-    webSocketState: ConnectionState;
 
-    // BlueSky server connection (determined by nodeinfo receipt)
+    // BlueSky server connection (derived from receiving data)
     blueSkyConnected: boolean;
-    blueSkyState: ConnectionState;
 
     // Data flow indicators
     receivingData: boolean;
@@ -45,18 +34,14 @@ type ConnectionStatusListener = (status: ConnectionStatusData) => void;
 type ConnectionEventCallback = (connected: boolean) => void;
 
 /**
- * Centralized Connection Status Service
- *
- * This service maintains the single source of truth for all connection states.
+ * Single source of truth for all connection states.
  */
 export class ConnectionStatusService {
     private static instance: ConnectionStatusService | null = null;
 
     private status: ConnectionStatusData = {
         webSocketConnected: false,
-        webSocketState: 'disconnected',
         blueSkyConnected: false,
-        blueSkyState: 'disconnected',
         receivingData: false,
         lastDataReceived: null,
         lastNodeInfoReceived: null,
@@ -68,23 +53,23 @@ export class ConnectionStatusService {
     private dataTimeoutId: number | null = null;
     private readonly DATA_TIMEOUT_MS = 5000; // Consider disconnected if no data for 5 seconds
 
-    // Wall-clock time the active no-data timeout was armed, plus how many times
-    // we have deferred a disconnect because that timeout fired late. A
-    // setTimeout that fires far past its delay means the event loop was blocked
-    // (a heavy synchronous task such as loading a large GeoJSON, a long GC
-    // pause, or a backgrounded tab) - during that window the socket cannot
-    // deliver data even when the server is perfectly healthy, so "no data" is
-    // not evidence of a real disconnect. We defer (bounded) and let live data
-    // confirm the connection instead of flashing a false "BlueSky disconnected".
+    // A setTimeout that fires far past its delay means the event loop was
+    // blocked (heavy synchronous work, GC pause, backgrounded tab). During
+    // that window the socket couldn't deliver data anyway, so "no data" is
+    // not evidence of a real disconnect - we defer (bounded) and let live
+    // data confirm the connection instead of flashing a false disconnect.
     private dataTimeoutArmedAt = 0;
     private stallDeferrals = 0;
     private readonly MAX_STALL_DEFERRALS = 2;
     private readonly STALL_OVERSHOOT_MS = 1500;
 
+    // End of the deliberate-disconnect grace window (see expectDisconnect).
+    private ignoreDataUntil = 0;
+
     // Initial connection tracking
     private isInitialLoadComplete: boolean = false;
     private initialConnectionCheckTimer: number | null = null;
-    private readonly INITIAL_CONNECTION_CHECK_DELAY_MS = 500; // Wait 0.5s before checking initial connection
+    private readonly INITIAL_CONNECTION_CHECK_DELAY_MS = 500;
 
     // Connection event callbacks
     private blueSkyDisconnectEmitter = new EventEmitter<boolean>('ConnectionStatus.disconnect');
@@ -123,9 +108,6 @@ export class ConnectionStatusService {
         return unsubscribe;
     }
 
-    /**
-     * Notify all listeners of status change
-     */
     private notifyListeners(): void {
         this.statusEmitter.emit(this.getStatus());
     }
@@ -147,12 +129,10 @@ export class ConnectionStatusService {
     public setWebSocketConnected(connected: boolean): void {
         const changed = this.status.webSocketConnected !== connected;
         this.status.webSocketConnected = connected;
-        this.status.webSocketState = connected ? 'connected' : 'disconnected';
 
         if (changed) {
             logger.info('ConnectionStatus', `WebSocket: ${connected ? 'connected' : 'disconnected'}`);
 
-            // Log to echo messages
             if (connected) {
                 echoManager.success('Connected to WebATM server');
             } else {
@@ -168,44 +148,35 @@ export class ConnectionStatusService {
         }
     }
 
-    /**
-     * Set WebSocket to connecting state
-     */
-    public setWebSocketConnecting(): void {
-        this.status.webSocketState = 'connecting';
-        echoManager.info('⟳ Connecting to WebATM server...');
-        this.notifyListeners();
-    }
-
-    /**
-     * Set WebSocket to error state
-     */
-    public setWebSocketError(): void {
-        this.status.webSocketState = 'error';
-        this.status.webSocketConnected = false;
-        echoManager.error('✗ WebATM server connection error');
-        this.notifyListeners();
-    }
-
     // ========================================
     // BlueSky Server Connection Methods
     // ========================================
 
     /**
-     * Update BlueSky server connection status
-     * This should be called when nodeinfo is received or connection is explicitly lost
+     * Update BlueSky server connection status.
+     * Called when data is received, or when the connection is known to be
+     * down (explicit disconnect, server_disconnected, WebSocket drop).
      */
     public setBlueSkyConnected(connected: boolean): void {
         const changed = this.status.blueSkyConnected !== connected;
         const wasConnected = this.status.blueSkyConnected;
 
         this.status.blueSkyConnected = connected;
-        this.status.blueSkyState = connected ? 'connected' : 'disconnected';
+
+        if (!connected) {
+            // The connection is known to be down: the pending no-data timer
+            // is now meaningless. Without this it fires up to DATA_TIMEOUT_MS
+            // later and echoes a false "no data received" warning after a
+            // deliberate disconnect (e.g. QUIT), and receivingData would
+            // stay stale after a WebSocket drop.
+            this.clearDataTimeout();
+            this.stallDeferrals = 0;
+            this.setReceivingData(false);
+        }
 
         if (changed) {
             logger.info('ConnectionStatus', `BlueSky: ${connected ? 'connected' : 'disconnected'}`);
 
-            // Log to echo messages
             if (connected) {
                 echoManager.success('Connected to BlueSky server');
             } else {
@@ -222,12 +193,24 @@ export class ConnectionStatusService {
     }
 
     /**
-     * Set BlueSky to connecting state
+     * A deliberate disconnect (QUIT, server_disconnected) is happening: data
+     * events already in flight would otherwise flip the status straight back
+     * to connected and re-arm the no-data timer, which then fires a spurious
+     * "connection may be lost" warning 5s after a clean disconnect. Ignore
+     * data for a short grace window; data still flowing after it (e.g. the
+     * disconnect request failed) reconnects as usual.
      */
-    public setBlueSkyConnecting(): void {
-        this.status.blueSkyState = 'connecting';
-        echoManager.info('⟳ Connecting to BlueSky server...');
-        this.notifyListeners();
+    public expectDisconnect(graceMs: number = 2000): void {
+        this.ignoreDataUntil = Date.now() + graceMs;
+    }
+
+    /**
+     * True while the deliberate-disconnect grace window is open. Lets data
+     * consumers (e.g. SocketManager) drop in-flight frames that would
+     * repaint the traffic the disconnect just cleared.
+     */
+    public isInDisconnectGrace(): boolean {
+        return Date.now() < this.ignoreDataUntil;
     }
 
     /**
@@ -237,6 +220,10 @@ export class ConnectionStatusService {
      * the receivingData flag.
      */
     private onServerDataReceived(kind: string, marksReceivingData: boolean): void {
+        if (Date.now() < this.ignoreDataUntil) {
+            logger.debug('ConnectionStatus', `${kind} ignored - disconnect grace window`);
+            return;
+        }
         logger.debug('ConnectionStatus', `${kind} received`);
 
         if (!this.status.blueSkyConnected) {
@@ -254,12 +241,10 @@ export class ConnectionStatusService {
 
     /**
      * Called when nodeinfo is received
-     * This is an indicator that we're connected to BlueSky server
      */
     public onNodeInfoReceived(): void {
         const now = Date.now();
 
-        // Calculate interval between nodeinfo messages
         if (this.status.lastNodeInfoReceived !== null) {
             this.status.nodeInfoInterval = now - this.status.lastNodeInfoReceived;
         }
@@ -270,7 +255,6 @@ export class ConnectionStatusService {
 
     /**
      * Called when simulation info (siminfo) is received
-     * This is a strong indicator that we're connected and receiving data
      */
     public onSimInfoReceived(): void {
         this.status.lastDataReceived = Date.now();
@@ -279,7 +263,6 @@ export class ConnectionStatusService {
 
     /**
      * Called when aircraft data (acdata) is received
-     * This is a strong indicator that we're connected and receiving data
      */
     public onAircraftDataReceived(): void {
         this.status.lastDataReceived = Date.now();
@@ -288,7 +271,6 @@ export class ConnectionStatusService {
 
     /**
      * Called when shape data (poly/polyline) is received
-     * This is an indicator that we're connected to BlueSky server
      */
     public onShapeDataReceived(): void {
         this.status.lastDataReceived = Date.now();
@@ -297,7 +279,6 @@ export class ConnectionStatusService {
 
     /**
      * Reset the timeout that detects when we stop receiving any data
-     * This is called when we receive nodeinfo, siminfo, or acdata
      */
     private resetDataTimeout(): void {
         // Fresh data clears any stall-deferral budget and re-arms the timer.
@@ -307,9 +288,7 @@ export class ConnectionStatusService {
 
     /** (Re)arm the no-data timeout, recording when it was armed. */
     private armDataTimeout(): void {
-        if (this.dataTimeoutId !== null) {
-            window.clearTimeout(this.dataTimeoutId);
-        }
+        this.clearDataTimeout();
         this.dataTimeoutArmedAt = Date.now();
         this.dataTimeoutId = window.setTimeout(
             () => this.onDataTimeoutExpired(),
@@ -317,16 +296,23 @@ export class ConnectionStatusService {
         );
     }
 
+    /** Cancel the pending no-data timeout, if any. */
+    private clearDataTimeout(): void {
+        if (this.dataTimeoutId !== null) {
+            window.clearTimeout(this.dataTimeoutId);
+            this.dataTimeoutId = null;
+        }
+    }
+
     /**
-     * Called when the no-data timeout fires. Distinguishes a genuine server
-     * silence from a local main-thread stall: if the callback ran far later
-     * than its scheduled delay, the page was frozen (so no data could have been
-     * received regardless of server health). In that case we defer the
-     * disconnect and re-arm, giving live data a chance to reset the timer.
-     * Bounded by MAX_STALL_DEFERRALS so a persistently blocked loop still
-     * eventually reports a real outage.
+     * Called when the no-data timeout fires. A callback that ran far later
+     * than its scheduled delay means the page was frozen, so the silence is
+     * a local stall rather than a server outage: defer the disconnect and
+     * re-arm (bounded by MAX_STALL_DEFERRALS). Otherwise treat the silence
+     * as a real disconnect.
      */
     private onDataTimeoutExpired(): void {
+        this.dataTimeoutId = null;
         const overshoot = Date.now() - this.dataTimeoutArmedAt - this.DATA_TIMEOUT_MS;
 
         if (overshoot > this.STALL_OVERSHOOT_MS && this.stallDeferrals < this.MAX_STALL_DEFERRALS) {
@@ -340,14 +326,11 @@ export class ConnectionStatusService {
             return;
         }
 
-        // Either the timer fired on schedule (a real silence) or we have
-        // deferred as long as we are willing to - treat it as a disconnect.
         this.stallDeferrals = 0;
-        logger.warn('ConnectionStatus', 'No data received (nodeinfo, siminfo, or acdata) - BlueSky may be disconnected');
-        echoManager.warning('⚠ No data received from BlueSky server - connection may be lost');
         if (this.status.blueSkyConnected) {
+            logger.warn('ConnectionStatus', 'No data received (nodeinfo, siminfo, or acdata) - BlueSky may be disconnected');
+            echoManager.warning('⚠ No data received from BlueSky server - connection may be lost');
             this.setBlueSkyConnected(false);
-            this.setReceivingData(false);
         }
     }
 
@@ -357,7 +340,6 @@ export class ConnectionStatusService {
 
     /**
      * Update data reception status
-     * Called when simulation data (siminfo, acdata) is received
      */
     public setReceivingData(receiving: boolean): void {
         const changed = this.status.receivingData !== receiving;
@@ -373,17 +355,6 @@ export class ConnectionStatusService {
         }
     }
 
-    /**
-     * Called when any simulation data is received
-     */
-    public onDataReceived(): void {
-        this.status.lastDataReceived = Date.now();
-
-        if (!this.status.receivingData) {
-            this.setReceivingData(true);
-        }
-    }
-
     // ========================================
     // Server Configuration Methods
     // ========================================
@@ -410,13 +381,6 @@ export class ConnectionStatusService {
         return this.status.webSocketConnected && this.status.blueSkyConnected;
     }
 
-    /**
-     * Check if WebSocket is connected
-     */
-    public isWebSocketConnected(): boolean {
-        return this.status.webSocketConnected;
-    }
-
     /**
      * Check if BlueSky is connected
      */
@@ -460,15 +424,9 @@ export class ConnectionStatusService {
             return 'Disconnected from BlueSky server. Please visit settings and make sure that (1) BlueSky server has been started and (2) You are connected to server.';
         }
 
-        if (this.status.blueSkyConnected && this.status.receivingData) {
-            return `Connected to BlueSky server at ${this.status.serverIP}.`;
-        }
-
-        if (this.status.blueSkyConnected && !this.status.receivingData) {
-            return `Connected to BlueSky server at ${this.status.serverIP} (No Data).`;
-        }
-
-        return 'Unknown connection status';
+        return this.status.receivingData
+            ? `Connected to BlueSky server at ${this.status.serverIP}.`
+            : `Connected to BlueSky server at ${this.status.serverIP} (No Data).`;
     }
 
     /**
@@ -479,9 +437,7 @@ export class ConnectionStatusService {
 
         this.status = {
             webSocketConnected: false,
-            webSocketState: 'disconnected',
             blueSkyConnected: false,
-            blueSkyState: 'disconnected',
             receivingData: false,
             lastDataReceived: null,
             lastNodeInfoReceived: null,
@@ -489,30 +445,24 @@ export class ConnectionStatusService {
             nodeInfoInterval: null
         };
 
-        if (this.dataTimeoutId !== null) {
-            window.clearTimeout(this.dataTimeoutId);
-            this.dataTimeoutId = null;
-        }
+        this.clearDataTimeout();
         this.stallDeferrals = 0;
         this.dataTimeoutArmedAt = 0;
+        this.ignoreDataUntil = 0;
 
         logger.info('ConnectionStatus', 'Reset all connection states');
         this.notifyListeners();
     }
 
     /**
-     * Get detailed status for debugging
+     * Get detailed status for debugging (window.connectionStatus helper)
      */
     public getDetailedStatus(): string {
         const status = this.getStatus();
         return JSON.stringify({
-            webSocket: {
-                connected: status.webSocketConnected,
-                state: status.webSocketState
-            },
+            webSocket: { connected: status.webSocketConnected },
             blueSky: {
                 connected: status.blueSkyConnected,
-                state: status.blueSkyState,
                 lastNodeInfo: status.lastNodeInfoReceived ?
                     `${Date.now() - status.lastNodeInfoReceived}ms ago` : 'never',
                 interval: status.nodeInfoInterval ? `${status.nodeInfoInterval}ms` : 'unknown'
@@ -532,16 +482,13 @@ export class ConnectionStatusService {
     // Initial Connection Checking Methods
     // ========================================
 
-    /**
-     * Load initial load state from sessionStorage
-     */
     private loadInitialLoadState(): void {
         this.isInitialLoadComplete = sessionStorage.getItem('bluesky-initial-load-complete') === 'true';
     }
 
     /**
-     * Start initial connection check
-     * This checks if we're connected after a delay on first page load
+     * On the first page load of a session, invoke onNotConnected if no
+     * BlueSky connection is established shortly after startup.
      */
     public startInitialConnectionCheck(onNotConnected: () => void): void {
         if (this.isInitialLoadComplete) {
@@ -582,21 +529,11 @@ export class ConnectionStatusService {
         });
     }
 
-    /**
-     * Mark initial load as complete
-     */
     private markInitialLoadComplete(): void {
         this.isInitialLoadComplete = true;
         sessionStorage.setItem('bluesky-initial-load-complete', 'true');
     }
 
-    /**
-     * Check if initial load is complete
-     */
-    public isInitialLoad(): boolean {
-        return !this.isInitialLoadComplete;
-    }
-
     // ========================================
     // Connection Event Callbacks
     // ========================================
@@ -609,9 +546,6 @@ export class ConnectionStatusService {
         return this.blueSkyDisconnectEmitter.subscribe(callback);
     }
 
-    /**
-     * Trigger all disconnect callbacks
-     */
     private triggerDisconnectCallbacks(): void {
         logger.debug('ConnectionStatus', 'Triggering disconnect callbacks');
         this.blueSkyDisconnectEmitter.emit(false);
diff --git a/frontend/src/core/SocketManager.ts b/frontend/src/core/SocketManager.ts
index c55b50d..b7931e3 100644
--- a/frontend/src/core/SocketManager.ts
+++ b/frontend/src/core/SocketManager.ts
@@ -119,10 +119,14 @@ export class SocketManager {
                 }
             },
             onSimInfo: (data: SimInfo) => {
+                if (connectionStatus.isInDisconnectGrace()) return;
                 this.stateManager.updateSimInfo(data);
                 connectionStatus.onSimInfoReceived();
             },
             onAircraftData: (data: AircraftData) => {
+                // In-flight frames arriving right after a deliberate
+                // disconnect would repaint the traffic that was just cleared.
+                if (connectionStatus.isInDisconnectGrace()) return;
                 this.stateManager.updateAircraftData(data);
                 connectionStatus.onAircraftDataReceived();
             },
@@ -161,8 +165,11 @@ export class SocketManager {
                 }
             },
             onServerDisconnected: () => {
+                // Also clears receivingData and the pending no-data timer;
+                // the grace window keeps in-flight data events from flipping
+                // the status straight back to connected.
+                connectionStatus.expectDisconnect();
                 connectionStatus.setBlueSkyConnected(false);
-                connectionStatus.setReceivingData(false);
             },
             onReset: () => {
                 // RESET clears simulation data only - we stay connected to BlueSky.
diff --git a/frontend/src/data/CommandHandler.test.ts b/frontend/src/data/CommandHandler.test.ts
index e70751b..821e4db 100644
--- a/frontend/src/data/CommandHandler.test.ts
+++ b/frontend/src/data/CommandHandler.test.ts
@@ -15,7 +15,7 @@ vi.mock('../ui/EchoManager', () => ({
 }));
 
 vi.mock('../core/ConnectionStatusService', () => ({
-    connectionStatus: { setBlueSkyConnected: vi.fn() },
+    connectionStatus: { setBlueSkyConnected: vi.fn(), expectDisconnect: vi.fn() },
 }));
 
 const addMessage = vi.mocked(echoManager.addMessage);
@@ -230,7 +230,9 @@ describe('CommandHandler', () => {
                 '/api/server/disconnect',
                 expect.objectContaining({ method: 'POST' }),
             );
-            // ...reflects it immediately in the shared connection status...
+            // ...reflects it immediately in the shared connection status,
+            // shielding it from data events already in flight...
+            expect(connectionStatus.expectDisconnect).toHaveBeenCalled();
             expect(setBlueSkyConnected).toHaveBeenCalledWith(false);
             // ...and crucially does NOT drop the browser↔WebATM socket.
             expect(mocks.socketManager.disconnect).not.toHaveBeenCalled();
diff --git a/frontend/src/data/CommandHandler.ts b/frontend/src/data/CommandHandler.ts
index 6ef4002..2534c1f 100644
--- a/frontend/src/data/CommandHandler.ts
+++ b/frontend/src/data/CommandHandler.ts
@@ -635,7 +635,9 @@ export class CommandHandler {
         // Reflect the explicit disconnect immediately in the single source of
         // truth the header — and, in the integrated build, the server-control
         // status — both read, so the indicators agree without waiting on the
-        // data-flow timeout.
+        // data-flow timeout. expectDisconnect() keeps data events already in
+        // flight from flipping the status right back to connected.
+        connectionStatus.expectDisconnect();
         connectionStatus.setBlueSkyConnected(false);
 
         this.sendEcho('Disconnected from BlueSky server (the server itself is left running)', 'info');
diff --git a/frontend/src/ui/SettingsModal.ts b/frontend/src/ui/SettingsModal.ts
index 05dfbb8..199f6e2 100644
--- a/frontend/src/ui/SettingsModal.ts
+++ b/frontend/src/ui/SettingsModal.ts
@@ -410,6 +410,9 @@ export class SettingsModal {
 
             if (result.success) {
                 this.saveServerIP(serverIp);
+                // Reflect the real host in the header status string, which
+                // otherwise reports its 'localhost' default forever.
+                connectionStatus.setServerIP(serverIp);
                 this.close();
 
                 // Move focus back to command console

From 24165bb86d4f02d8dd67ffa2184d8eebca8eebe3 Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:25:47 +0200
Subject: [PATCH 4/7] fix(map): make the click-to-draw tools mutually exclusive

Starting shape drawing while a route draw was in progress (or either while
placing an aircraft) left both tools listening, so one map click was consumed
twice: waypoints landed in a route the user thought they had left, and the
shared drawing banner showed whichever tool started last.

A small drawingExclusion module holds the single active claim; claiming
cancels the previous owner's draw. Route and shape drawing claim the map
before showing their banner so the outgoing tool's teardown can't hide it.

Alongside:
- AircraftInteractionManager's route-only predicate becomes a general
  setDrawingToolActiveCheck, so empty-map clicks during shape drawing and
  aircraft placement no longer deselect the selected aircraft.
- Teardown no longer bails out when the map is already gone, which used to
  strand the document-level keydown/Escape listener and the navaid highlight.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 frontend/src/core/App.ts                      | 14 ++--
 .../src/ui/map/BaseDrawingManager.test.ts     | 64 +++++++++++++++++--
 frontend/src/ui/map/BaseDrawingManager.ts     | 35 +++++-----
 .../map/aircraft/AircraftCreationManager.ts   | 43 +++++++------
 .../aircraft/AircraftInteractionManager.ts    | 24 +++----
 frontend/src/ui/map/drawingExclusion.ts       | 30 +++++++++
 .../src/ui/map/routes/RouteDrawingManager.ts  |  4 +-
 .../src/ui/map/shapes/ShapeDrawingManager.ts  |  4 +-
 8 files changed, 159 insertions(+), 59 deletions(-)
 create mode 100644 frontend/src/ui/map/drawingExclusion.ts

diff --git a/frontend/src/core/App.ts b/frontend/src/core/App.ts
index 1ca02b1..ba3c380 100644
--- a/frontend/src/core/App.ts
+++ b/frontend/src/core/App.ts
@@ -373,13 +373,15 @@ export class App {
             this.navaidSnapper
         );
 
-        // Let AircraftInteractionManager know when route drawing is active so
-        // it can skip its empty-map-click "unselect aircraft" behavior while
-        // waypoints are being placed.
-        if (this.aircraftInteractionManager && this.routeDrawingManager) {
+        // Let AircraftInteractionManager know when a drawing tool is active
+        // so it can skip its empty-map-click "unselect aircraft" behavior
+        // while points are being placed.
+        if (this.aircraftInteractionManager) {
             const rdm = this.routeDrawingManager;
-            this.aircraftInteractionManager.setRouteDrawingActiveCheck(
-                () => rdm.isDrawing()
+            const sdm = this.shapeDrawingManager;
+            const acm = this.aircraftCreationManager;
+            this.aircraftInteractionManager.setDrawingToolActiveCheck(
+                () => !!(rdm?.isDrawing() || sdm?.isDrawing() || acm?.isDrawing())
             );
         }
 
diff --git a/frontend/src/ui/map/BaseDrawingManager.test.ts b/frontend/src/ui/map/BaseDrawingManager.test.ts
index 5fb1d65..537e038 100644
--- a/frontend/src/ui/map/BaseDrawingManager.test.ts
+++ b/frontend/src/ui/map/BaseDrawingManager.test.ts
@@ -57,7 +57,11 @@ class TestDrawingManager extends BaseDrawingManager {
     protected onPointAdded(point: DrawingPoint): void { this.points.push(point); }
     protected onCursorMove(point: DrawingPoint): void { this.cursorMoves.push(point); }
     protected finishDrawing(): void { this.finished(); }
-    protected cancelDrawing(): void { this.cancelled(); }
+    // Mirrors the real subclasses: cancelling stops the draw.
+    protected cancelDrawing(): void {
+        this.cancelled();
+        this.stop();
+    }
 }
 
 describe('BaseDrawingManager', () => {
@@ -68,10 +72,14 @@ describe('BaseDrawingManager', () => {
     const clickEvent = (lat: number, lng: number) =>
         ({ lngLat: { lat, lng }, preventDefault: vi.fn() }) as unknown as MapMouseEvent;
 
-    function createManager(finishOnEnter = false): TestDrawingManager {
-        map = createFakeMap();
+    function createManager(
+        finishOnEnter = false,
+        getMap?: () => ReturnType<typeof createFakeMap> | null
+    ): TestDrawingManager {
+        const ownMap = createFakeMap();
+        map = ownMap;
         snapper = { snap: vi.fn(() => null), highlight: vi.fn(), clearHighlight: vi.fn() };
-        const mapDisplay = { getMap: () => map } as unknown as MapDisplay;
+        const mapDisplay = { getMap: getMap ?? (() => ownMap) } as unknown as MapDisplay;
         return new TestDrawingManager(mapDisplay, snapper as unknown as NavaidSnapper, finishOnEnter);
     }
 
@@ -178,4 +186,52 @@ describe('BaseDrawingManager', () => {
         idle.destroy();
         expect(idle.cancelled).not.toHaveBeenCalled();
     });
+
+    it('starting a second drawing tool cancels the first (mutual exclusion)', () => {
+        const first = createManager();
+        const firstMap = map;
+        first.start();
+
+        const second = createManager();
+        second.start();
+
+        expect(first.cancelled).toHaveBeenCalledTimes(1);
+        expect(first.isDrawing()).toBe(false);
+        expect(second.isDrawing()).toBe(true);
+
+        // Clicks reach only the second tool now.
+        firstMap.fire('click', clickEvent(51, 3));
+        map.fire('click', clickEvent(52, 4));
+        expect(first.points).toEqual([]);
+        expect(second.points).toEqual([{ lat: 52, lng: 4 }]);
+    });
+
+    it('stopping again after a draw was cancelled does not re-cancel the new tool', () => {
+        const first = createManager();
+        first.start();
+        const second = createManager();
+        second.start();
+
+        // The first tool releasing its (already lost) claim must not affect
+        // the second tool's active draw.
+        first.stop();
+        expect(second.isDrawing()).toBe(true);
+        expect(second.cancelled).not.toHaveBeenCalled();
+    });
+
+    it('stop with the map already gone still releases the keydown listener and hooks', () => {
+        let liveMap: ReturnType<typeof createFakeMap> | null = null;
+        const gone = createManager(false, () => liveMap);
+        liveMap = map;
+        gone.start();
+
+        const removeSpy = vi.spyOn(document, 'removeEventListener');
+        liveMap = null;
+        gone.stop();
+
+        expect(gone.disabled).toHaveBeenCalledTimes(1);
+        expect(removeSpy).toHaveBeenCalledWith('keydown', expect.any(Function));
+        expect(snapper.clearHighlight).toHaveBeenCalled();
+        removeSpy.mockRestore();
+    });
 });
diff --git a/frontend/src/ui/map/BaseDrawingManager.ts b/frontend/src/ui/map/BaseDrawingManager.ts
index df21fe3..443445f 100644
--- a/frontend/src/ui/map/BaseDrawingManager.ts
+++ b/frontend/src/ui/map/BaseDrawingManager.ts
@@ -3,6 +3,7 @@ import type { MapMouseEvent } from 'maplibre-gl';
 import type { NavaidSnapper } from './navdata/NavaidSnapper';
 import { DRAWING_CURSOR } from '../../utils/maplibre';
 import { isTextEntryTarget } from '../../utils/dom';
+import { claimDrawing, releaseDrawing } from './drawingExclusion';
 
 /**
  * BaseDrawingManager - shared interactive point-drawing lifecycle for the
@@ -97,6 +98,10 @@ export abstract class BaseDrawingManager {
         const map = this.mapDisplay.getMap();
         if (!map) return;
 
+        // Cancel any other tool's in-progress draw (shape vs route vs
+        // aircraft placement) so two tools never consume the same clicks.
+        claimDrawing(this, () => this.cancelDrawing());
+
         map.getCanvas().style.cursor = DRAWING_CURSOR;
 
         this.onDrawingEnabled();
@@ -116,11 +121,9 @@ export abstract class BaseDrawingManager {
      * Restore the map to normal mode and tear down all drawing handlers.
      */
     protected disableMapDrawing(): void {
-        const map = this.mapDisplay.getMap();
-        if (!map) return;
-
         this.suspendMapInteraction();
         this.onDrawingDisabled();
+        releaseDrawing(this);
     }
 
     /**
@@ -131,23 +134,19 @@ export abstract class BaseDrawingManager {
      * Idempotent.
      */
     protected suspendMapInteraction(): void {
+        // The map can already be gone at teardown; the document-level keydown
+        // listener and the navaid highlight must be released regardless.
         const map = this.mapDisplay.getMap();
-        if (!map) return;
-
-        map.getCanvas().style.cursor = '';
-
-        if (this.mapClickHandler) {
-            map.off('click', this.mapClickHandler);
-            this.mapClickHandler = null;
-        }
-        if (this.mapRightClickHandler) {
-            map.off('contextmenu', this.mapRightClickHandler);
-            this.mapRightClickHandler = null;
-        }
-        if (this.mapMouseMoveHandler) {
-            map.off('mousemove', this.mapMouseMoveHandler);
-            this.mapMouseMoveHandler = null;
+        if (map) {
+            map.getCanvas().style.cursor = '';
+            if (this.mapClickHandler) map.off('click', this.mapClickHandler);
+            if (this.mapRightClickHandler) map.off('contextmenu', this.mapRightClickHandler);
+            if (this.mapMouseMoveHandler) map.off('mousemove', this.mapMouseMoveHandler);
         }
+        this.mapClickHandler = null;
+        this.mapRightClickHandler = null;
+        this.mapMouseMoveHandler = null;
+
         if (this.keyDownHandler) {
             document.removeEventListener('keydown', this.keyDownHandler);
             this.keyDownHandler = null;
diff --git a/frontend/src/ui/map/aircraft/AircraftCreationManager.ts b/frontend/src/ui/map/aircraft/AircraftCreationManager.ts
index c5db1f6..5ec3ba4 100644
--- a/frontend/src/ui/map/aircraft/AircraftCreationManager.ts
+++ b/frontend/src/ui/map/aircraft/AircraftCreationManager.ts
@@ -13,6 +13,7 @@ import {
 import { pointFeature, lineStringFeature } from '../../../utils/geojson';
 import { roundedBearing } from '../../../utils/geo';
 import { isTextEntryTarget } from '../../../utils/dom';
+import { claimDrawing, releaseDrawing } from '../drawingExclusion';
 import {
     AircraftCreationForm,
     AircraftCreationData,
@@ -62,6 +63,14 @@ export class AircraftCreationManager {
         this.form.showModal();
     }
 
+    /**
+     * Whether click-to-place aircraft creation is currently active. Consumed
+     * by AircraftInteractionManager to suppress empty-map-click deselection.
+     */
+    public isDrawing(): boolean {
+        return this.aircraftDrawingMode;
+    }
+
     /**
      * Begin the map drawing state machine with validated form data.
      * Invoked by AircraftCreationForm after the modal closes.
@@ -118,6 +127,10 @@ export class AircraftCreationManager {
             return;
         }
 
+        // Cancel any in-progress shape/route draw so two tools never consume
+        // the same map clicks.
+        claimDrawing(this, () => this.stopAircraftDrawing());
+
         // Match the crosshair cursor used by the console map picker and the
         // shape/route drawing modes so every drawing mode looks the same.
         map.getCanvas().style.cursor = DRAWING_CURSOR;
@@ -154,26 +167,19 @@ export class AircraftCreationManager {
      * Disable aircraft map drawing
      */
     private disableAircraftMapDrawing(): void {
+        // The map can already be gone at teardown; the document-level Escape
+        // listener and the drawing claim must be released regardless.
         const map = this.mapDisplay.getMap();
-        if (!map) return;
-
-        // Restore MapLibre's default cursor when leaving drawing mode.
-        map.getCanvas().style.cursor = '';
-
-        if (this.aircraftMapClickHandler) {
-            map.off('click', this.aircraftMapClickHandler);
-            this.aircraftMapClickHandler = null;
-        }
-
-        if (this.aircraftMouseMoveHandler) {
-            map.off('mousemove', this.aircraftMouseMoveHandler);
-            this.aircraftMouseMoveHandler = null;
-        }
-
-        if (this.aircraftSnapHoverHandler) {
-            map.off('mousemove', this.aircraftSnapHoverHandler);
-            this.aircraftSnapHoverHandler = null;
+        if (map) {
+            // Restore MapLibre's default cursor when leaving drawing mode.
+            map.getCanvas().style.cursor = '';
+            if (this.aircraftMapClickHandler) map.off('click', this.aircraftMapClickHandler);
+            if (this.aircraftMouseMoveHandler) map.off('mousemove', this.aircraftMouseMoveHandler);
+            if (this.aircraftSnapHoverHandler) map.off('mousemove', this.aircraftSnapHoverHandler);
         }
+        this.aircraftMapClickHandler = null;
+        this.aircraftMouseMoveHandler = null;
+        this.aircraftSnapHoverHandler = null;
         this.navaidSnapper.clearHighlight();
 
         if (this.aircraftEscapeHandler) {
@@ -183,6 +189,7 @@ export class AircraftCreationManager {
 
         this.aircraftPosition = null;
         this.clearTemporaryAircraftDrawing();
+        releaseDrawing(this);
     }
 
     /**
diff --git a/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts b/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts
index 6eb2ffa..81de6af 100644
--- a/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts
+++ b/frontend/src/ui/map/aircraft/AircraftInteractionManager.ts
@@ -39,9 +39,10 @@ export class AircraftInteractionManager {
     private lastExplicitPosCommand: string | null = null;
     private lastExplicitPosTime: number = 0;
 
-    // Optional reference to the route drawing manager - when route drawing is
-    // active, empty-map clicks are waypoint placements, not aircraft deselects.
-    private isRouteDrawingActive: (() => boolean) | null = null;
+    // Optional predicate reporting whether any map drawing tool (route,
+    // shape, aircraft placement) is active - those clicks are point
+    // placements, not aircraft deselects.
+    private isDrawingToolActive: (() => boolean) | null = null;
 
     // Document-level listeners and state subscriptions, released in destroy()
     // (map listeners die with the map, but these would outlive it).
@@ -108,9 +109,9 @@ export class AircraftInteractionManager {
             setTimeout(() => {
                 if (!this.map) return;
 
-                // If the route drawing tool is active, an empty-map click is a
-                // waypoint placement, not a request to unselect the aircraft.
-                if (this.isRouteDrawingActive && this.isRouteDrawingActive()) {
+                // If a drawing tool is active, an empty-map click is a point
+                // placement, not a request to unselect the aircraft.
+                if (this.isDrawingToolActive && this.isDrawingToolActive()) {
                     return;
                 }
 
@@ -478,12 +479,13 @@ export class AircraftInteractionManager {
     }
 
     /**
-     * Register a predicate that reports whether interactive route drawing is
-     * currently in progress. When true, empty-map clicks are suppressed from
-     * the "unselect aircraft" path so they can be consumed as waypoint drops.
+     * Register a predicate that reports whether any interactive drawing tool
+     * (route, shape, aircraft placement) is in progress. When true, empty-map
+     * clicks are suppressed from the "unselect aircraft" path so they can be
+     * consumed as point drops.
      */
-    public setRouteDrawingActiveCheck(check: () => boolean): void {
-        this.isRouteDrawingActive = check;
+    public setDrawingToolActiveCheck(check: () => boolean): void {
+        this.isDrawingToolActive = check;
     }
 
     /**
diff --git a/frontend/src/ui/map/drawingExclusion.ts b/frontend/src/ui/map/drawingExclusion.ts
new file mode 100644
index 0000000..a6a3873
--- /dev/null
+++ b/frontend/src/ui/map/drawingExclusion.ts
@@ -0,0 +1,30 @@
+/**
+ * Mutual exclusion between the map's click-to-draw tools (route drawing,
+ * shape drawing, aircraft placement). Each tool claims the map when its
+ * interactive mode starts; claiming cancels whichever tool held it, so two
+ * tools can never consume the same map clicks at once.
+ */
+
+let activeOwner: object | null = null;
+let activeCancel: (() => void) | null = null;
+
+/**
+ * Claim interactive drawing for `owner`, cancelling any other tool's
+ * in-progress draw first. `cancel` must fully stop the owner's draw; it is
+ * invoked when another tool claims the map.
+ */
+export function claimDrawing(owner: object, cancel: () => void): void {
+    if (activeOwner && activeOwner !== owner) {
+        activeCancel?.();
+    }
+    activeOwner = owner;
+    activeCancel = cancel;
+}
+
+/** Release the claim if `owner` still holds it (no-op otherwise). */
+export function releaseDrawing(owner: object): void {
+    if (activeOwner === owner) {
+        activeOwner = null;
+        activeCancel = null;
+    }
+}
diff --git a/frontend/src/ui/map/routes/RouteDrawingManager.ts b/frontend/src/ui/map/routes/RouteDrawingManager.ts
index 86ed0a8..bcc970d 100644
--- a/frontend/src/ui/map/routes/RouteDrawingManager.ts
+++ b/frontend/src/ui/map/routes/RouteDrawingManager.ts
@@ -212,10 +212,12 @@ export class RouteDrawingManager extends BaseDrawingManager {
             drawBtn.classList.add('active');
         }
 
+        // Claim the map first (cancelling any other tool's draw, which hides
+        // the shared banner), then show this draw's banner.
+        this.enableMapDrawing();
         this.showDrawingBanner(
             `Drawing route for ${this.targetAircraftId} (leader from ${this.leaderAnchorLabel}) - Click to add waypoints, right-click or Enter to finish, Esc to cancel`
         );
-        this.enableMapDrawing();
         this.preview.updateDrawing(this.routePoints, this.leaderAnchor);
 
         logger.info(
diff --git a/frontend/src/ui/map/shapes/ShapeDrawingManager.ts b/frontend/src/ui/map/shapes/ShapeDrawingManager.ts
index 9619cb6..4a39ad0 100644
--- a/frontend/src/ui/map/shapes/ShapeDrawingManager.ts
+++ b/frontend/src/ui/map/shapes/ShapeDrawingManager.ts
@@ -192,10 +192,12 @@ export class ShapeDrawingManager extends BaseDrawingManager {
             drawBtn.classList.add('active');
         }
 
+        // Claim the map first (cancelling any other tool's draw, which hides
+        // the shared banner), then show this draw's banner.
+        this.enableMapDrawing();
         this.showDrawingBanner(
             `${this.getBannerPrefix()} - ${START_INSTRUCTIONS[this.currentShapeType]}`
         );
-        this.enableMapDrawing();
 
         logger.info('ShapeDrawingManager', `Started drawing ${this.currentShapeType}: ${this.currentShapeName}`);
     }

From 4c50401526d54c002ccafe18de775fbe0686eab9 Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:25:56 +0200
Subject: [PATCH 5/7] fix(3d): keep invalid aircraft coordinates out of the
 scene origin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

BlueSky can deliver NaN or out-of-range lat/lon (e.g. after a MOVE past
lat 90). The scene origin was seeded from — and recentred on — every record,
so one bad aircraft poisoned the origin shared by all mesh transforms and the
mercator camera matrix, and MercatorCoordinate.fromLngLat then threw on every
data tick and every frame.

updateSceneOrigin now filters through isValidCoordinate before seeding or
recentring, mirroring the per-aircraft guard the renderers already apply.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../map/aircraft/Aircraft3DTransforms.test.ts | 98 +++++++++++++++++++
 .../ui/map/aircraft/Aircraft3DTransforms.ts   | 73 ++++++--------
 2 files changed, 130 insertions(+), 41 deletions(-)
 create mode 100644 frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts

diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts
new file mode 100644
index 0000000..a55ed3e
--- /dev/null
+++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.test.ts
@@ -0,0 +1,98 @@
+/**
+ * Tests for Aircraft3DTransforms scene-origin management. The scene origin
+ * is shared by every mesh transform and the mercator camera matrix, so an
+ * invalid aircraft record (NaN or out-of-range lat/lon, which BlueSky can
+ * deliver, e.g. after MOVE beyond lat 90) must never leak into it:
+ * MercatorCoordinate.fromLngLat throws on invalid input, which crashes the
+ * whole data tick and the per-frame render.
+ */
+import { describe, it, expect } from 'vitest';
+import * as THREE from 'three';
+import { Aircraft3DTransforms } from './Aircraft3DTransforms';
+import type { AircraftMeshData } from './Aircraft3DTransforms';
+import type { AircraftData, DisplayOptions } from '../../../data/types';
+
+function makeTransforms(): Aircraft3DTransforms {
+    return new Aircraft3DTransforms({
+        getMap: () => null,
+        getCamera: () => new THREE.PerspectiveCamera(),
+        getDisplayOptions: () => ({ aircraft3DScale: 2 } as unknown as DisplayOptions),
+        createFallbackMatrix: () => new THREE.Matrix4(),
+        stateManager: null,
+    });
+}
+
+/** AircraftData carrying only the positions the origin logic reads. */
+function acData(positions: Array<[number, number]>): AircraftData {
+    return {
+        id: positions.map((_, i) => `AC${i}`),
+        lat: positions.map((p) => p[0]),
+        lon: positions.map((p) => p[1]),
+    } as unknown as AircraftData;
+}
+
+function meshData(lat: number, lon: number): AircraftMeshData {
+    return { lat, lon, alt: 3000, hdg: 90, selected: false, inconf: false, actype: 'A320' };
+}
+
+describe('Aircraft3DTransforms scene origin vs invalid coordinates', () => {
+    it('initializes the origin from the first valid aircraft, skipping invalid ones', () => {
+        const t = makeTransforms();
+        t.updateSceneOrigin(acData([[NaN, NaN], [52, 4]]));
+
+        const mesh = new THREE.Object3D();
+        t.updateMeshTransform(mesh, meshData(52, 4));
+
+        // Origin is the valid aircraft, so its own offsets are ~zero.
+        expect(mesh.position.x).toBeCloseTo(0);
+        expect(mesh.position.z).toBeCloseTo(0);
+    });
+
+    it('returns false (no origin) when no valid aircraft and no map exist', () => {
+        const t = makeTransforms();
+        expect(t.updateSceneOrigin(acData([[91, 181]]))).toBe(false);
+    });
+
+    it('an aircraft moved beyond lat 90 does not drag the origin away from valid traffic', () => {
+        const t = makeTransforms();
+        t.updateSceneOrigin(acData([[52.2, 4.7]]));
+        // BAD sits at lat 91 (as ACDATA reports after `MOVE BAD 91,181`)
+        // next to an in-range aircraft.
+        t.updateSceneOrigin(acData([[91, 181], [52.3, 4.62]]));
+
+        const mesh = new THREE.Object3D();
+        t.updateMeshTransform(mesh, meshData(52.3, 4.62));
+
+        // The origin must stay near the valid traffic: offsets are small
+        // and finite (a poisoned centroid would put it thousands of km away).
+        expect(Number.isFinite(mesh.position.x)).toBe(true);
+        expect(Math.abs(mesh.position.x)).toBeLessThan(50_000);
+        expect(Math.abs(mesh.position.z)).toBeLessThan(50_000);
+    });
+
+    it('a NaN record does not poison the reposition centroid', () => {
+        const t = makeTransforms();
+        t.updateSceneOrigin(acData([[52.2, 4.7]]));
+        // The valid aircraft is >10 km out, so the origin repositions; the
+        // NaN record must be excluded from the centroid.
+        t.updateSceneOrigin(acData([[NaN, 4], [53.5, 6]]));
+
+        const mesh = new THREE.Object3D();
+        t.updateMeshTransform(mesh, meshData(53.5, 6));
+
+        expect(Number.isFinite(mesh.position.x)).toBe(true);
+        expect(Math.abs(mesh.position.x)).toBeLessThan(50_000);
+        expect(Math.abs(mesh.position.z)).toBeLessThan(50_000);
+    });
+
+    it('still repositions onto the centroid of valid traffic', () => {
+        const t = makeTransforms();
+        t.updateSceneOrigin(acData([[52, 4]]));
+        expect(t.updateSceneOrigin(acData([[53.5, 6]]))).toBe(true);
+
+        const mesh = new THREE.Object3D();
+        t.updateMeshTransform(mesh, meshData(53.5, 6));
+        expect(mesh.position.x).toBeCloseTo(0);
+        expect(mesh.position.z).toBeCloseTo(0);
+    });
+});
diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts
index b70f680..b5fa640 100644
--- a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts
+++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts
@@ -1,6 +1,8 @@
 import * as THREE from 'three';
 import type { Map as MapLibreMap } from 'maplibre-gl';
 import { altitudeScaledForOrigin, mercatorCameraMatrix, relativePositionMeters } from '../rendering/mercatorUtils';
+import type { LngLatPoint } from '../rendering/mercatorUtils';
+import { isValidCoordinate } from '../../../utils/maplibre';
 import { getGlobeModelMatrix } from '../rendering/globeMatrix';
 import type { Render3DArgs } from '../rendering/CustomLayer3D';
 import type { AircraftData, DisplayOptions } from '../../../data/types';
@@ -76,75 +78,64 @@ export class Aircraft3DTransforms {
      * Initialize or update scene origin based on aircraft positions.
      * Returns true when the origin was repositioned, in which case the
      * caller must re-apply mercator transforms to existing meshes.
+     *
+     * Invalid records (NaN or out-of-range lat/lon — BlueSky delivers
+     * these, e.g. after a MOVE beyond lat 90) are ignored, mirroring the
+     * per-aircraft guard in the renderers. A poisoned origin would make
+     * MercatorCoordinate.fromLngLat throw on every mesh and camera update.
      */
     updateSceneOrigin(aircraftData: AircraftData): boolean {
+        const valid: LngLatPoint[] = [];
+        for (let i = 0; i < aircraftData.lat.length; i++) {
+            if (isValidCoordinate(aircraftData.lat[i], aircraftData.lon[i])) {
+                valid.push({ lng: aircraftData.lon[i], lat: aircraftData.lat[i] });
+            }
+        }
+
         if (!this.sceneOrigin) {
-            // Initialize scene origin with first aircraft or map center
-            if (aircraftData.lat.length > 0) {
-                this.sceneOrigin = {
-                    lng: aircraftData.lon[0],
-                    lat: aircraftData.lat[0]
-                };
+            if (valid.length > 0) {
+                this.sceneOrigin = { ...valid[0] };
             } else {
                 // Fallback to map center if available
                 const center = this.deps.getMap()?.getCenter();
                 if (!center) return false;
-                this.sceneOrigin = {
-                    lng: center.lng,
-                    lat: center.lat
-                };
+                this.sceneOrigin = { lng: center.lng, lat: center.lat };
             }
             logger.debug('Aircraft3DTransforms', `Scene origin set to: ${this.sceneOrigin.lng.toFixed(6)}, ${this.sceneOrigin.lat.toFixed(6)}`);
         }
 
-        // Check if any aircraft is too far from current origin
-        let needsRepositioning = false;
-        for (let i = 0; i < aircraftData.lat.length; i++) {
-            const distance = this.calculateDistance(
-                this.sceneOrigin.lat, this.sceneOrigin.lng,
-                aircraftData.lat[i], aircraftData.lon[i]
-            );
-            if (distance > this.maxDistanceFromOrigin) {
-                needsRepositioning = true;
-                break;
-            }
-        }
+        const origin = this.sceneOrigin;
+        const needsRepositioning = valid.some((p) =>
+            this.calculateDistance(origin.lat, origin.lng, p.lat, p.lng) > this.maxDistanceFromOrigin
+        );
 
-        if (needsRepositioning) {
-            return this.repositionSceneOrigin(aircraftData);
-        }
-        return false;
+        return needsRepositioning ? this.repositionSceneOrigin(valid) : false;
     }
 
     /**
-     * Reposition scene origin to aircraft centroid. Returns true when the
-     * origin actually moved (changes under 10 m are skipped).
+     * Reposition scene origin to the centroid of the given (valid) aircraft
+     * positions. Returns true when the origin actually moved (changes under
+     * 10 m are skipped).
      */
-    private repositionSceneOrigin(aircraftData: AircraftData): boolean {
-        if (aircraftData.lat.length === 0) return false;
+    private repositionSceneOrigin(coords: LngLatPoint[]): boolean {
+        if (coords.length === 0) return false;
 
-        // Calculate centroid of all aircraft
         let sumLat = 0;
         let sumLng = 0;
-        for (let i = 0; i < aircraftData.lat.length; i++) {
-            sumLat += aircraftData.lat[i];
-            sumLng += aircraftData.lon[i];
+        for (const c of coords) {
+            sumLat += c.lat;
+            sumLng += c.lng;
         }
-
         const newOrigin = {
-            lng: sumLng / aircraftData.lon.length,
-            lat: sumLat / aircraftData.lat.length
+            lng: sumLng / coords.length,
+            lat: sumLat / coords.length
         };
 
-        // Check if the new origin is significantly different from current origin
-        // Only reposition if difference is meaningful (> 10 meters)
         const distanceToNewOrigin = this.calculateDistance(
             this.sceneOrigin!.lat, this.sceneOrigin!.lng,
             newOrigin.lat, newOrigin.lng
         );
-
         if (distanceToNewOrigin < 10) {
-            // Origin change is too small to be meaningful, skip repositioning
             return false;
         }
 

From 7b1ef873186625911d314ad60bdd0e9de546dc33 Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:25:56 +0200
Subject: [PATCH 6/7] fix(3d): discard GLB loads that complete after teardown

A model request still in flight when clearAll() ran resurrected the caches it
had just dropped: the model was cached and onModelLoaded fired against a torn
down renderer, leaking the GLB's geometry and materials. The load callback now
checks that its entry is still registered and disposes the result otherwise.

clearCache() also dropped its recorded raw dimensions, which previously
survived the models they described.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../aircraft/Aircraft3DModelLoader.test.ts    | 46 ++++++++++++++++
 .../ui/map/aircraft/Aircraft3DModelLoader.ts  | 54 ++++++++++---------
 2 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts
index f2b52e5..38141fd 100644
--- a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts
+++ b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts
@@ -66,6 +66,52 @@ describe('Aircraft3DModelLoader cache disposal', () => {
         matSpies.forEach((spy) => expect(spy).toHaveBeenCalledTimes(1));
     });
 
+    it('drops the recorded raw dimensions on clearCache()', () => {
+        const loader = makeLoader();
+        const { model } = multiMaterialModel();
+
+        loader.load('A320.glb');
+        captured.onLoad?.({ scene: model });
+        expect(loader.rawMaxDim('A320.glb')).toBeGreaterThan(0);
+
+        loader.clearCache();
+
+        expect(loader.rawMaxDim('A320.glb')).toBeUndefined();
+    });
+
+    it('lets an in-flight load re-populate the cache after clearCache()', () => {
+        const loader = makeLoader();
+        const { model } = multiMaterialModel();
+
+        loader.load('A320.glb');
+        loader.clearCache(); // load still in flight
+
+        captured.onLoad?.({ scene: model });
+
+        expect(loader.get('A320.glb')).toBe(model);
+    });
+
+    it('discards a load that completes after clearAll()', () => {
+        const onModelLoaded = vi.fn();
+        const loader = new Aircraft3DModelLoader({
+            getMaxAnisotropy: () => 1,
+            onModelLoaded,
+        });
+        const { model, geometry, materials } = multiMaterialModel();
+        const geomSpy = vi.spyOn(geometry, 'dispose');
+        const matSpies = materials.map((m) => vi.spyOn(m, 'dispose'));
+
+        loader.load('A320.glb');
+        loader.clearAll(); // teardown while the load is in flight
+
+        captured.onLoad?.({ scene: model });
+
+        expect(loader.get('A320.glb')).toBeUndefined();
+        expect(onModelLoaded).not.toHaveBeenCalled();
+        expect(geomSpy).toHaveBeenCalledTimes(1);
+        matSpies.forEach((spy) => expect(spy).toHaveBeenCalledTimes(1));
+    });
+
     it('disposes cached model resources on clearAll()', () => {
         const loader = makeLoader();
         const { model, geometry, materials } = multiMaterialModel();
diff --git a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts
index 32eaa31..9934e54 100644
--- a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts
+++ b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts
@@ -63,6 +63,13 @@ export class Aircraft3DModelLoader {
         this.loader.load(
             path,
             (gltf) => {
+                // clearAll() ran while this request was in flight: the
+                // owner tore everything down, so discard the result
+                // instead of resurrecting the cache.
+                if (!this.loadingModels.has(path)) {
+                    this.disposeModel(gltf.scene);
+                    return;
+                }
                 this.loadingModels.delete(path);
                 this.normalizeModel(gltf.scene, path);
                 this.loadedModels.set(path, gltf.scene);
@@ -92,38 +99,37 @@ export class Aircraft3DModelLoader {
      * re-populate the cache.
      */
     public clearCache(): void {
-        this.disposeCachedModels();
+        this.loadedModels.forEach((model) => this.disposeModel(model));
         this.loadedModels.clear();
+        this.rawMaxDims.clear();
         this.animationClips.clear();
     }
 
-    /** Full teardown: dispose and drop the cache, forget in-flight loads. */
+    /**
+     * Full teardown: dispose and drop the cache, forget in-flight loads
+     * (their completions are discarded in load()'s callback).
+     */
     public clearAll(): void {
-        this.disposeCachedModels();
-        this.loadedModels.clear();
-        this.animationClips.clear();
+        this.clearCache();
         this.loadingModels.clear();
     }
 
     /**
-     * Dispose the geometry and materials of every cached model. Aircraft
-     * meshes are clones that share these resources, so callers must detach
-     * all live meshes before clearing the cache. Materials may be a single
-     * instance or an array (multi-material meshes), so handle both.
+     * Dispose a model's geometry and materials. Aircraft meshes are clones
+     * that share these resources, so callers must detach all live meshes
+     * first. Materials may be a single instance or an array.
      */
-    private disposeCachedModels(): void {
-        this.loadedModels.forEach((model) => {
-            model.traverse((child) => {
-                if (child instanceof THREE.Mesh) {
-                    child.geometry.dispose();
-                    const material = child.material;
-                    if (Array.isArray(material)) {
-                        material.forEach((m) => m.dispose());
-                    } else if (material instanceof THREE.Material) {
-                        material.dispose();
-                    }
+    private disposeModel(model: THREE.Group): void {
+        model.traverse((child) => {
+            if (child instanceof THREE.Mesh) {
+                child.geometry.dispose();
+                const material = child.material;
+                if (Array.isArray(material)) {
+                    material.forEach((m) => m.dispose());
+                } else if (material instanceof THREE.Material) {
+                    material.dispose();
                 }
-            });
+            }
         });
     }
 
@@ -205,10 +211,8 @@ export class Aircraft3DModelLoader {
             }
         });
 
-        // Don't set model.rotation here - it gets ignored anyway because we use mesh.matrix
-        // The rotation is applied in updateMeshTransform() via the transform matrix
-        // This ensures heading rotation works correctly
-
+        // No rotation here: heading is applied per-aircraft in the
+        // updateMeshTransform* methods.
         logger.debug('Aircraft3DModelLoader', `Model normalized: path=${path}, rawMax=${rawMax.toFixed(2)}, size=${size.x.toFixed(1)}x${size.y.toFixed(1)}x${size.z.toFixed(1)}, anisotropy=${maxAnisotropy}`);
     }
 }

From bb7e576058e935958fa8b202c44cecf7e6bddb5b Mon Sep 17 00:00:00 2001
From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:26:02 +0200
Subject: [PATCH 7/7] refactor(ui): trim duplication in Aircraft3DFleet and
 MapControlsPanel

Aircraft3DFleet repeated the globe/mercator transform branch in four places;
it now goes through one applyTransform helper. MapControlsPanel's three
near-identical button handlers collapse into a withManager helper, and the
zoom/reset paths lose their duplicated null checks and narration-only logs.

No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../src/ui/map/aircraft/Aircraft3DFleet.ts    |  48 ++----
 .../src/ui/panels/left/MapControlsPanel.ts    | 138 ++++--------------
 2 files changed, 41 insertions(+), 145 deletions(-)

diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts
index 03095fc..b56ea0d 100644
--- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts
+++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts
@@ -60,6 +60,15 @@ export class Aircraft3DFleet {
         this.aircraft.forEach(cb);
     }
 
+    /** Apply the transform matching the projection the mesh lives in. */
+    private applyTransform(mesh: THREE.Object3D, data: AircraftMeshData, isGlobe: boolean): void {
+        if (isGlobe) {
+            this.deps.transforms.updateMeshTransformForGlobe(mesh, data);
+        } else {
+            this.deps.transforms.updateMeshTransform(mesh, data);
+        }
+    }
+
     /**
      * Scale factor that converts this model's raw GLB units into
      * real-world meters for the given ICAO. Multiply by the user's
@@ -108,14 +117,8 @@ export class Aircraft3DFleet {
         }
 
         const isGlobe = this.deps.isGlobeProjection();
-
-        if (isGlobe) {
-            this.deps.transforms.updateMeshTransformForGlobe(mesh, data);
-            this.deps.getGlobeGroup()?.add(mesh);
-        } else {
-            this.deps.transforms.updateMeshTransform(mesh, data);
-            this.deps.getMercatorGroup()?.add(mesh);
-        }
+        this.applyTransform(mesh, data, isGlobe);
+        (isGlobe ? this.deps.getGlobeGroup() : this.deps.getMercatorGroup())?.add(mesh);
 
         this.aircraft.set(id, {
             mesh,
@@ -147,13 +150,9 @@ export class Aircraft3DFleet {
             );
         }
 
-        // Update transform appropriate for the current group the mesh is in
-        // Note: If projection changed, switchGroups handles moving
-        if (aircraftMesh.currentGroup === 'globe') {
-            this.deps.transforms.updateMeshTransformForGlobe(aircraftMesh.mesh, data);
-        } else {
-            this.deps.transforms.updateMeshTransform(aircraftMesh.mesh, data);
-        }
+        // Transform for the group the mesh is currently in; if the
+        // projection changed, switchGroups handles moving it.
+        this.applyTransform(aircraftMesh.mesh, data, aircraftMesh.currentGroup === 'globe');
 
         aircraftMesh.data = data;
         aircraftMesh.lastUpdate = Date.now();
@@ -249,11 +248,7 @@ export class Aircraft3DFleet {
     reapplyAllTransforms(): void {
         const isGlobe = this.deps.isGlobeProjection();
         this.aircraft.forEach((aircraftMesh) => {
-            if (isGlobe) {
-                this.deps.transforms.updateMeshTransformForGlobe(aircraftMesh.mesh, aircraftMesh.data);
-            } else {
-                this.deps.transforms.updateMeshTransform(aircraftMesh.mesh, aircraftMesh.data);
-            }
+            this.applyTransform(aircraftMesh.mesh, aircraftMesh.data, isGlobe);
         });
     }
 
@@ -288,20 +283,9 @@ export class Aircraft3DFleet {
         logger.debug('Aircraft3DFleet', `Switching ${this.aircraft.size} aircraft to ${toGlobe ? 'globe' : 'mercator'} group`);
 
         this.aircraft.forEach((aircraftMesh) => {
-            // Remove from current group
             sourceGroup.remove(aircraftMesh.mesh);
-
-            // Add to target group
             targetGroup.add(aircraftMesh.mesh);
-
-            // Update the transform for the new projection mode
-            if (toGlobe) {
-                this.deps.transforms.updateMeshTransformForGlobe(aircraftMesh.mesh, aircraftMesh.data);
-            } else {
-                this.deps.transforms.updateMeshTransform(aircraftMesh.mesh, aircraftMesh.data);
-            }
-
-            // Update tracking
+            this.applyTransform(aircraftMesh.mesh, aircraftMesh.data, toGlobe);
             aircraftMesh.currentGroup = toGlobe ? 'globe' : 'mercator';
         });
     }
diff --git a/frontend/src/ui/panels/left/MapControlsPanel.ts b/frontend/src/ui/panels/left/MapControlsPanel.ts
index efe3d25..aadbc7c 100644
--- a/frontend/src/ui/panels/left/MapControlsPanel.ts
+++ b/frontend/src/ui/panels/left/MapControlsPanel.ts
@@ -29,61 +29,25 @@ export class MapControlsPanel extends BasePanel {
     protected onInit(): void {
         logger.debug('MapControlsPanel', 'MapControlsPanel initialized');
 
-        // Set up button event handlers
-        this.setupButtonHandlers();
+        this.bindClick('create-aircraft-btn', () =>
+            this.withManager(this.aircraftCreationManager, 'AircraftCreationManager', m => m.showModal()));
+        this.bindClick('draw-shape-btn', () =>
+            this.withManager(this.shapeDrawingManager, 'ShapeDrawingManager', m => m.toggleDrawing()));
+        this.bindClick('draw-route-btn', () =>
+            this.withManager(this.routeDrawingManager, 'RouteDrawingManager', m => m.toggleDrawing()));
 
-        // Set up map info update handlers
         this.setupMapInfoHandlers();
     }
 
     /**
-     * Set up button event handlers
+     * Run a drawing-button action against its manager, or tell the user the
+     * map is still loading when the manager hasn't been wired up yet.
      */
-    private setupButtonHandlers(): void {
-        this.bindClick('create-aircraft-btn', () => this.onCreateAircraftClick());
-        this.bindClick('draw-shape-btn', () => this.onDrawShapeClick());
-        this.bindClick('draw-route-btn', () => this.onDrawRouteClick());
-    }
-
-    /**
-     * Handle Create Aircraft button click
-     */
-    private onCreateAircraftClick(): void {
-        logger.debug('MapControlsPanel', 'Create Aircraft button clicked');
-        // Will be connected to AircraftCreationManager
-        if (this.aircraftCreationManager) {
-            this.aircraftCreationManager.showModal();
-        } else {
-            logger.warn('MapControlsPanel', 'AircraftCreationManager not set - map may still be loading');
-            // Optionally show a user-friendly message
-            alert('Map is still loading. Please wait a moment and try again.');
-        }
-    }
-
-    /**
-     * Handle Draw Shape button click
-     */
-    private onDrawShapeClick(): void {
-        logger.debug('MapControlsPanel', 'Draw Shape button clicked');
-        // Will be connected to ShapeDrawingManager
-        if (this.shapeDrawingManager) {
-            this.shapeDrawingManager.toggleDrawing();
-        } else {
-            logger.warn('MapControlsPanel', 'ShapeDrawingManager not set - map may still be loading');
-            // Optionally show a user-friendly message
-            alert('Map is still loading. Please wait a moment and try again.');
-        }
-    }
-
-    /**
-     * Handle Draw Route button click
-     */
-    private onDrawRouteClick(): void {
-        logger.debug('MapControlsPanel', 'Draw Route button clicked');
-        if (this.routeDrawingManager) {
-            this.routeDrawingManager.toggleDrawing();
+    private withManager<T>(manager: T | null, name: string, action: (manager: T) => void): void {
+        if (manager) {
+            action(manager);
         } else {
-            logger.warn('MapControlsPanel', 'RouteDrawingManager not set - map may still be loading');
+            logger.warn('MapControlsPanel', `${name} not set - map may still be loading`);
             alert('Map is still loading. Please wait a moment and try again.');
         }
     }
@@ -163,121 +127,69 @@ export class MapControlsPanel extends BasePanel {
         logger.debug('MapControlsPanel', 'Map event listeners attached for zoom and bbox updates');
     }
 
-    /**
-     * Update the zoom level display
-     */
     private updateZoomDisplay(): void {
-        if (!this.mapDisplay) return;
-
-        const map = this.mapDisplay.getMap();
+        const map = this.mapDisplay?.getMap();
         if (!map) return;
 
         this.setText('current-zoom', map.getZoom().toFixed(1));
     }
 
-    /**
-     * Update the bounding box display
-     */
     private updateBoundingBoxDisplay(): void {
-        if (!this.mapDisplay) return;
-
-        const map = this.mapDisplay.getMap();
+        const map = this.mapDisplay?.getMap();
         if (!map) return;
 
         const bounds = map.getBounds();
-
         this.setText('bbox-north', bounds.getNorth().toFixed(2));
         this.setText('bbox-south', bounds.getSouth().toFixed(2));
         this.setText('bbox-east', bounds.getEast().toFixed(2));
         this.setText('bbox-west', bounds.getWest().toFixed(2));
     }
 
-    /**
-     * Set the AircraftCreationManager instance
-     */
     public setAircraftCreationManager(manager: AircraftCreationManager): void {
         this.aircraftCreationManager = manager;
-        logger.debug('MapControlsPanel', 'MapControlsPanel connected to AircraftCreationManager');
     }
 
-    /**
-     * Set the ShapeDrawingManager instance
-     */
     public setShapeDrawingManager(manager: ShapeDrawingManager): void {
         this.shapeDrawingManager = manager;
-        logger.debug('MapControlsPanel', 'MapControlsPanel connected to ShapeDrawingManager');
     }
 
-    /**
-     * Set the RouteDrawingManager instance
-     */
     public setRouteDrawingManager(manager: RouteDrawingManager): void {
         this.routeDrawingManager = manager;
-        logger.debug('MapControlsPanel', 'MapControlsPanel connected to RouteDrawingManager');
     }
 
-    /**
-     * Zoom in by one level
-     */
+    /** Zoom in by one level */
     public zoomIn(): void {
-        if (!this.mapDisplay) {
-            logger.warn('MapControlsPanel', 'Cannot zoom in: MapDisplay not set');
-            return;
-        }
-
-        const map = this.mapDisplay.getMap();
+        const map = this.mapDisplay?.getMap();
         if (!map) {
-            logger.warn('MapControlsPanel', 'Cannot zoom in: Map not initialized');
+            logger.warn('MapControlsPanel', 'Cannot zoom in: map not ready');
             return;
         }
-
-        const currentZoom = map.getZoom();
-        map.easeTo({ zoom: currentZoom + 1, duration: 300 });
-        logger.debug('MapControlsPanel', 'Zooming in to level:', currentZoom + 1);
+        map.easeTo({ zoom: map.getZoom() + 1, duration: 300 });
     }
 
-    /**
-     * Zoom out by one level
-     */
+    /** Zoom out by one level */
     public zoomOut(): void {
-        if (!this.mapDisplay) {
-            logger.warn('MapControlsPanel', 'Cannot zoom out: MapDisplay not set');
-            return;
-        }
-
-        const map = this.mapDisplay.getMap();
+        const map = this.mapDisplay?.getMap();
         if (!map) {
-            logger.warn('MapControlsPanel', 'Cannot zoom out: Map not initialized');
+            logger.warn('MapControlsPanel', 'Cannot zoom out: map not ready');
             return;
         }
-
-        const currentZoom = map.getZoom();
-        map.easeTo({ zoom: currentZoom - 1, duration: 300 });
-        logger.debug('MapControlsPanel', 'Zooming out to level:', currentZoom - 1);
+        map.easeTo({ zoom: map.getZoom() - 1, duration: 300 });
     }
 
-    /**
-     * Reset view to default center and zoom
-     */
+    /** Reset view to the default center and zoom */
     public resetView(): void {
-        if (!this.mapDisplay) {
-            logger.warn('MapControlsPanel', 'Cannot reset view: MapDisplay not set');
-            return;
-        }
-
-        const map = this.mapDisplay.getMap();
+        const map = this.mapDisplay?.getMap();
         if (!map) {
-            logger.warn('MapControlsPanel', 'Cannot reset view: Map not initialized');
+            logger.warn('MapControlsPanel', 'Cannot reset view: map not ready');
             return;
         }
-
         map.flyTo({
             center: this.DEFAULT_CENTER,
             zoom: this.DEFAULT_ZOOM,
             duration: 1000,
             essential: true
         });
-        logger.debug('MapControlsPanel', 'Resetting view to default center and zoom');
     }
 
     /**