fix mcp-server review feedback from thread

Agent-Logs-Url: https://github.com/meshtastic/firmware/sessions/91dc128a-ed50-4d07-8bb2-3dc6623a05f7

Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com>

.mcp.json

@@ -1,10 +1,10 @@
 {
   "mcpServers": {
     "meshtastic": {
-      "command": "/Users/benmeadors/Documents/GitHub/firmware/mcp-server/.venv/bin/python",
+      "command": "./mcp-server/.venv/bin/python",
       "args": ["-m", "meshtastic_mcp"],
       "env": {
-        "MESHTASTIC_FIRMWARE_ROOT": "/Users/benmeadors/Documents/GitHub/firmware"
+        "MESHTASTIC_FIRMWARE_ROOT": "."
       }
     }
   }

mcp-server/README.md

@@ -61,7 +61,7 @@ Replace `<firmware-repo>` with the absolute path, e.g. `/Users/you/GitHub/firmwa
 
 Same `mcpServers` block, but in `~/Library/Application Support/Claude/claude_desktop_config.json` (macOS) or `%APPDATA%\Claude\claude_desktop_config.json` (Windows).
 
-## Tools (32)
+## Tools (38)
 
 ### Discovery & metadata
 
@@ -100,7 +100,7 @@ Backed by long-running `pio device monitor` subprocesses with a 10k-line ring bu
 | `device_info` | my_node_num, long/short name, firmware version, region, channel, node count |
 | `list_nodes`  | Full node database with position, SNR, RSSI, last_heard, battery            |
 
-_The tool tables below document 35 currently registered MCP server tools._
+_The tool tables below document 38 currently registered MCP server tools._
 
 ### Device writes
 

mcp-server/src/meshtastic_mcp/flash.py

@@ -10,6 +10,7 @@ artifacts to exist, so these tools build first if needed.
 from __future__ import annotations
 
 import subprocess
+import threading
 import time
 from pathlib import Path
 from typing import Any
@@ -308,7 +309,7 @@ def _do_1200bps_touch(port: str, settle_ms: int, touch_timeout_s: float = 3.0) -
     worker that's still blocked in the background after timeout has already
     delivered the signal.
     """
-    import concurrent.futures
+    errors: list[BaseException] = []
 
     def _inner() -> None:
         try:
@@ -325,12 +326,19 @@ def _do_1200bps_touch(port: str, settle_ms: int, touch_timeout_s: float = 3.0) -
             except Exception:
                 pass
 
-    with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
-        future = pool.submit(_inner)
+    def _runner() -> None:
         try:
-            future.result(timeout=touch_timeout_s)
-        except concurrent.futures.TimeoutError:
-            pass  # signal already delivered; worker thread leaks harmlessly
+            _inner()
+        except BaseException as exc:  # re-raised on caller thread after join
+            errors.append(exc)
+
+    worker = threading.Thread(target=_runner, daemon=True)
+    worker.start()
+    worker.join(timeout=touch_timeout_s)
+    if worker.is_alive():
+        return  # signal already delivered; allow daemon worker to finish/exit
+    if errors:
+        raise errors[0]
 
 
 # Adafruit nRF52 bootloader VID/PID (BOTH RAK4631 and most Feather nRF52 boards).

mcp-server/src/meshtastic_mcp/registry.py

@@ -54,18 +54,17 @@ def all_sessions() -> list[SerialSession]:
 
 def sweep_dead() -> int:
     """Remove sessions whose subprocess has exited. Returns count removed."""
-    removed = 0
+    removed_sessions: list[SerialSession] = []
     with _LOCK:
-        for sid in list(_sessions):
-            s = _sessions[sid]
+        for sid, s in list(_sessions.items()):
             if s.proc.poll() is not None:
-                # Leave the session object around for one read so callers can
-                # see eof=True. We only remove after stopped_at is older than
-                # a few seconds, so an immediate poll-after-close still sees
-                # it. For v1 simpler: just leave dead sessions until explicit
-                # close. No-op.
-                pass
-    return removed
+                removed_sessions.append(_sessions.pop(sid))
+    for session in removed_sessions:
+        try:
+            close_session(session)
+        except Exception:
+            pass
+    return len(removed_sessions)
 
 
 def shutdown_all() -> None:

mcp-server/src/meshtastic_mcp/serial_session.py

@@ -19,7 +19,7 @@ import uuid
 from dataclasses import dataclass, field
 from typing import Any
 
-from . import config, pio
+from . import boards, config, pio
 
 _BUFFER_MAX_LINES = 10_000
 _POLL_NEW_PORT_TIMEOUT_S = 3.0
@@ -76,7 +76,43 @@ def open_session(
     effective_baud: int = baud
     if env is not None:
         args.extend(["-e", env])
-        effective_filters = filters or []  # pio will fill in from env
+        raw_config: dict[str, Any] = {}
+        try:
+            raw = boards.get_board(env).get("raw_config")
+            if isinstance(raw, dict):
+                raw_config = raw
+        except Exception:
+            raw_config = {}
+
+        monitor_speed = raw_config.get("monitor_speed")
+        has_board_speed = False
+        if monitor_speed is not None:
+            try:
+                effective_baud = int(str(monitor_speed).strip())
+                has_board_speed = True
+            except (TypeError, ValueError):
+                pass
+
+        monitor_filters_raw = raw_config.get("monitor_filters")
+        parsed_board_filters: list[str] = []
+        if isinstance(monitor_filters_raw, str):
+            for token in monitor_filters_raw.replace("\n", ",").split(","):
+                item = token.strip()
+                if item:
+                    parsed_board_filters.append(item)
+        elif isinstance(monitor_filters_raw, list):
+            parsed_board_filters = [
+                str(item).strip() for item in monitor_filters_raw if str(item).strip()
+            ]
+
+        has_board_filters = len(parsed_board_filters) > 0
+        effective_filters = parsed_board_filters if has_board_filters else (filters or [])
+
+        if not has_board_speed:
+            args.extend(["--baud", str(effective_baud)])
+        if not has_board_filters:
+            for f in effective_filters:
+                args.extend(["--filter", f])
     else:
         args.extend(["--baud", str(baud)])
         effective_filters = filters or ["direct"]

mcp-server/src/meshtastic_mcp/server.py

@@ -1,4 +1,4 @@
-"""FastMCP server wiring — 32 tools across 6 categories.
+"""FastMCP server wiring — 38 tools across 7 categories.
 
 Each tool handler is a thin delegation to a named module (pio.py, admin.py,
 etc.). Business logic does not live here.

mcp-server/tests/README.md

@@ -49,9 +49,9 @@ pytest tests/ --force-bake --html=report.html
 
 - `MESHTASTIC_FIRMWARE_ROOT` — firmware repo path (defaults to `../` from tests/)
 - `MESHTASTIC_MCP_ENV_NRF52` — PlatformIO env for the nRF52 role (default
-  `heltec-mesh-node-t114`)
+  `rak4631`)
 - `MESHTASTIC_MCP_ENV_ESP32S3` — PlatformIO env for the ESP32-S3 role (default
-  `t-beam-1w`)
+  `heltec-v3`)
 - `MESHTASTIC_MCP_SEED` — override the session PSK seed (default:
   `pytest-<unix-ts>`). Set this to reproduce a specific failing run.
 
@@ -85,7 +85,7 @@ predicate(), timeout=60)` replaces flaky `time.sleep()` patterns.
 `pytest --junitxml=junit.xml` produces CI-integration XML.
 
 `tool_coverage.json` is emitted at session end in the tests directory — shows
-which of the 37 MCP tools the run exercised. Useful for closing test gaps.
+which of the 38 MCP tools the run exercised. Useful for closing test gaps.
 
 ## Adding a new test
 

mcp-server/tests/tool_coverage.py

@@ -1,7 +1,7 @@
 """Tool-surface coverage: track which MCP tools the test suite actually exercises.
 
 This is NOT line coverage (that's `coverage.py`). This measures which of the
-37 public MCP tools in `meshtastic_mcp.server` got invoked during a pytest
+38 public MCP tools in `meshtastic_mcp.server` got invoked during a pytest
 run — a quick signal for "where are the test-coverage gaps".
 
 Approach: introspect `meshtastic_mcp.server.app` for registered tools, find
@@ -57,6 +57,7 @@ _TOOL_MAP: dict[str, tuple[str, str]] = {
     # Serial log sessions — module-level functions on serial_session
     "serial_open": ("meshtastic_mcp.serial_session", "open_session"),
     "serial_read": ("meshtastic_mcp.serial_session", "read_session"),
+    "serial_list": ("meshtastic_mcp.registry", "all_sessions"),
     "serial_close": ("meshtastic_mcp.serial_session", "close_session"),
     # Device reads
     "device_info": ("meshtastic_mcp.info", "device_info"),
@@ -67,6 +68,7 @@ _TOOL_MAP: dict[str, tuple[str, str]] = {
     "set_config": ("meshtastic_mcp.admin", "set_config"),
     "get_channel_url": ("meshtastic_mcp.admin", "get_channel_url"),
     "set_channel_url": ("meshtastic_mcp.admin", "set_channel_url"),
+    "set_debug_log_api": ("meshtastic_mcp.admin", "set_debug_log_api"),
     "send_text": ("meshtastic_mcp.admin", "send_text"),
     "reboot": ("meshtastic_mcp.admin", "reboot"),
     "shutdown": ("meshtastic_mcp.admin", "shutdown"),

mcp-server/tests/unit/test_userprefs_parse.py

@@ -8,7 +8,6 @@ produce confusing mismatches.
 
 from __future__ import annotations
 
-import shutil
 from pathlib import Path
 
 import pytest