Server: refine shutdown logic per bot feedback\n- Parameterize _force_exit(code) and use timers with args\n- Consistent behavior on BrokenPipeError (no immediate exit)\n- Exit code 1 on unexpected exceptions\n\nTests: restore telemetry module after disabling to avoid bleed-over

Author: dsarno

Server/server.py

@@ -4,6 +4,9 @@
 from logging.handlers import RotatingFileHandler
 import os
 from contextlib import asynccontextmanager
+import sys
+import signal
+import threading
 from typing import AsyncIterator, Dict, Any
 from config import config
 from tools import register_all_tools
@@ -64,6 +67,9 @@
 # Global connection state
 _unity_connection: UnityConnection = None
 
+# Global shutdown coordination
+_shutdown_flag = threading.Event()
+
 
 @asynccontextmanager
 async def server_lifespan(server: FastMCP) -> AsyncIterator[Dict[str, Any]]:
@@ -186,9 +192,96 @@ def _emit_startup():
 register_all_resources(mcp)
 
 
+def _force_exit(code: int = 0):
+    """Force process exit, bypassing any background threads that might linger."""
+    try:
+        sys.exit(code)
+    except SystemExit:
+        os._exit(code)
+
+
+def _signal_handler(signum, frame):
+    logger.info(f"Received signal {signum}, initiating shutdown...")
+    _shutdown_flag.set()
+    threading.Timer(1.0, _force_exit, args=(0,)).start()
+
+
+def _monitor_stdin():
+    """Background thread to detect stdio detach (stdin EOF) or parent exit."""
+    try:
+        parent_pid = os.getppid() if hasattr(os, "getppid") else None
+        while not _shutdown_flag.is_set():
+            if _shutdown_flag.wait(0.5):
+                break
+
+            if parent_pid is not None:
+                try:
+                    os.kill(parent_pid, 0)
+                except (ProcessLookupError, OSError):
+                    logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
+                    break
+
+            try:
+                if sys.stdin.closed:
+                    logger.info("stdin.closed is True; client disconnected")
+                    break
+                fd = sys.stdin.fileno()
+                if fd < 0:
+                    logger.info("stdin fd invalid; client disconnected")
+                    break
+            except (ValueError, OSError, AttributeError):
+                # Closed pipe or unavailable stdin
+                break
+            except Exception:
+                # Ignore transient errors
+                pass
+
+        if not _shutdown_flag.is_set():
+            logger.info("Client disconnected (stdin or parent), initiating shutdown...")
+            _shutdown_flag.set()
+            if not _shutdown_flag.is_set():
+                threading.Timer(0.5, _force_exit, args=(0,)).start()
+            else:
+                threading.Timer(0.5, _force_exit, args=(0,)).start()
+    except Exception:
+        # Never let monitor thread crash the process
+        pass
+
+
 def main():
     """Entry point for uvx and console scripts."""
-    mcp.run(transport='stdio')
+    try:
+        signal.signal(signal.SIGTERM, _signal_handler)
+        signal.signal(signal.SIGINT, _signal_handler)
+        if hasattr(signal, "SIGPIPE"):
+            signal.signal(signal.SIGPIPE, signal.SIG_IGN)
+        if hasattr(signal, "SIGBREAK"):
+            signal.signal(signal.SIGBREAK, _signal_handler)
+    except Exception:
+        # Signals can fail in some environments
+        pass
+
+    t = threading.Thread(target=_monitor_stdin, daemon=True)
+    t.start()
+
+    try:
+        mcp.run(transport='stdio')
+        logger.info("FastMCP run() returned (stdin EOF or disconnect)")
+    except (KeyboardInterrupt, SystemExit):
+        logger.info("Server interrupted; shutting down")
+        _shutdown_flag.set()
+    except BrokenPipeError:
+        logger.info("Broken pipe; shutting down")
+        _shutdown_flag.set()
+        # rely on finally to schedule exit for consistency
+    except Exception as e:
+        logger.error(f"Server error: {e}", exc_info=True)
+        _shutdown_flag.set()
+        _force_exit(1)
+    finally:
+        _shutdown_flag.set()
+        logger.info("Server main loop exited")
+        threading.Timer(0.5, _force_exit, args=(0,)).start()
 
 
 # Run the server

Server/test_telemetry.py

@@ -23,8 +23,8 @@ def test_telemetry_basic():
         )
         pass
     except ImportError as e:
-        # Silent failure path for tests
-        return False
+        # Fail explicitly when imports are missing
+        assert False, f"telemetry import failed: {e}"
 
     # Test telemetry enabled status
     _ = is_telemetry_enabled()
@@ -37,8 +37,7 @@ def test_telemetry_basic():
         })
         pass
     except Exception as e:
-        # Silent failure path for tests
-        return False
+        assert False, f"record_telemetry failed: {e}"
 
     # Test milestone recording
     try:
@@ -47,26 +46,23 @@ def test_telemetry_basic():
         })
         _ = is_first
     except Exception as e:
-        # Silent failure path for tests
-        return False
+        assert False, f"record_milestone failed: {e}"
 
     # Test telemetry collector
     try:
         collector = get_telemetry()
         _ = collector
     except Exception as e:
-        # Silent failure path for tests
-        return False
+        assert False, f"get_telemetry failed: {e}"
+    assert True
 
-    return True
 
-
-def test_telemetry_disabled():
+def test_telemetry_disabled(monkeypatch):
     """Test telemetry with disabled state"""
     # Silent for tests
 
     # Set environment variable to disable telemetry
-    os.environ["DISABLE_TELEMETRY"] = "true"
+    monkeypatch.setenv("DISABLE_TELEMETRY", "true")
 
     # Re-import to get fresh config
     import importlib
@@ -77,17 +73,12 @@ def test_telemetry_disabled():
 
     _ = is_telemetry_enabled()
 
-    if not is_telemetry_enabled():
-        pass
-
-        # Test that records are ignored when disabled
-        record_telemetry(RecordType.USAGE, {"test": "should_be_ignored"})
-        pass
-
-        return True
-    else:
-        pass
-        return False
+    assert is_telemetry_enabled() is False
+    # Test that records are ignored when disabled (should not raise)
+    record_telemetry(RecordType.USAGE, {"test": "should_be_ignored"})
+    # Restore module state for subsequent tests
+    monkeypatch.delenv("DISABLE_TELEMETRY", raising=False)
+    importlib.reload(telemetry)
 
 
 def test_data_storage():
@@ -114,11 +105,10 @@ def test_data_storage():
         else:
             pass
 
-        return True
+        assert True
 
     except Exception as e:
-        # Silent failure path for tests
-        return False
+        assert False, f"data storage test failed: {e}"
 
 
 def main():