Fix broker auto-spawn when stale non-broker PID file blocks spawn

src/mcpbridge_wrapper/broker/proxy.py

@@ -225,7 +225,7 @@ def _check_version_mismatch(self) -> bool:
         return True
 
     def _pid_belongs_to_broker(self, pid: int) -> bool:
-        """Return True when PID command line matches broker daemon shape."""
+        """Return True when PID command line matches broker daemon invocation forms."""
         try:
             cmdline = subprocess.check_output(
                 ["ps", "-p", str(pid), "-o", "command="],
@@ -234,7 +234,16 @@ def _pid_belongs_to_broker(self, pid: int) -> bool:
             ).strip()
         except (OSError, subprocess.CalledProcessError):
             return False
-        return "mcpbridge_wrapper" in cmdline and "--broker-daemon" in cmdline
+
+        if "--broker-daemon" not in cmdline:
+            return False
+
+        broker_tokens = (
+            "mcpbridge_wrapper",
+            "mcpbridge-wrapper",
+            "xcodemcpwrapper",
+        )
+        return any(token in cmdline for token in broker_tokens)
 
     def _stop_stale_daemon(self) -> None:
         """Stop a running broker daemon via SIGTERM + wait + file cleanup."""
@@ -318,14 +327,22 @@ async def _spawn_broker_if_needed(self) -> None:
                 try:
                     pid = int(pid_file.read_text().strip())
                     os.kill(pid, 0)
-                    # Daemon is alive — check for version mismatch.
-                    if self._check_version_mismatch():
-                        logger.info("Stopping stale broker (version mismatch)…")
-                        await loop.run_in_executor(None, self._stop_stale_daemon)
-                        # Fall through to spawn a new daemon.
+                    if not self._pid_belongs_to_broker(pid):
+                        logger.warning(
+                            "Live PID %d from %s is not a broker daemon; cleaning stale files.",
+                            pid,
+                            pid_file,
+                        )
+                        self._cleanup_broker_files()
                     else:
-                        logger.debug("Broker already running (PID %d); skipping spawn.", pid)
-                        return
+                        # Daemon is alive — check for version mismatch.
+                        if self._check_version_mismatch():
+                            logger.info("Stopping stale broker (version mismatch)…")
+                            await loop.run_in_executor(None, self._stop_stale_daemon)
+                            # Fall through to spawn a new daemon.
+                        else:
+                            logger.debug("Broker already running (PID %d); skipping spawn.", pid)
+                            return
                 except (ValueError, ProcessLookupError, PermissionError):
                     logger.debug("Stale PID file; will spawn broker.")
 

tests/unit/test_broker_proxy.py

@@ -256,12 +256,40 @@ async def test_spawn_noop_when_pid_file_live(self, tmp_path: Path) -> None:
 
         proxy = BrokerProxy(cfg, auto_spawn=True, connect_timeout=0.1)
 
-        with patch("subprocess.Popen") as mock_popen:
+        with patch.object(proxy, "_pid_belongs_to_broker", return_value=True), patch(
+            "subprocess.Popen"
+        ) as mock_popen:
             # _spawn_broker_if_needed should return without calling Popen
             await proxy._spawn_broker_if_needed()
 
         mock_popen.assert_not_called()
 
+    @pytest.mark.asyncio
+    async def test_live_non_broker_pid_is_treated_as_stale(self, tmp_path: Path) -> None:
+        """Live unrelated PID in pid file should be cleaned and replaced by a new broker."""
+        cfg = _make_config(tmp_path)
+        cfg.pid_file.write_text(str(os.getpid()))
+        cfg.socket_path.write_text("stale")
+        cfg.version_file.write_text("old")
+
+        proxy = BrokerProxy(cfg, auto_spawn=True, connect_timeout=1.0)
+
+        real_exists = Path.exists
+        socket_checks = {"count": 0}
+
+        def _fake_exists(path_obj: Path) -> bool:
+            if path_obj == cfg.socket_path:
+                socket_checks["count"] += 1
+                return socket_checks["count"] >= 2
+            return real_exists(path_obj)
+
+        with patch.object(proxy, "_pid_belongs_to_broker", return_value=False), patch.object(
+            Path, "exists", _fake_exists
+        ), patch("subprocess.Popen") as mock_popen:
+            await proxy._spawn_broker_if_needed()
+
+        mock_popen.assert_called_once()
+
     @pytest.mark.asyncio
     async def test_spawn_noop_when_socket_exists(self, tmp_path: Path) -> None:
         """_spawn_broker_if_needed does nothing when socket file exists and broker is alive."""
@@ -809,6 +837,24 @@ def test_pid_belongs_to_broker_true_for_expected_command(self, tmp_path: Path) -
         ):
             assert proxy._pid_belongs_to_broker(123) is True
 
+    def test_pid_belongs_to_broker_true_for_console_script_command(self, tmp_path: Path) -> None:
+        cfg = _make_config(tmp_path)
+        proxy = BrokerProxy(cfg)
+        with patch(
+            "mcpbridge_wrapper.broker.proxy.subprocess.check_output",
+            return_value="/Users/me/.local/bin/mcpbridge-wrapper --broker-daemon --web-ui",
+        ):
+            assert proxy._pid_belongs_to_broker(123) is True
+
+    def test_pid_belongs_to_broker_true_for_legacy_wrapper_command(self, tmp_path: Path) -> None:
+        cfg = _make_config(tmp_path)
+        proxy = BrokerProxy(cfg)
+        with patch(
+            "mcpbridge_wrapper.broker.proxy.subprocess.check_output",
+            return_value="/Users/me/bin/xcodemcpwrapper --broker-daemon",
+        ):
+            assert proxy._pid_belongs_to_broker(123) is True
+
     def test_pid_belongs_to_broker_false_on_ps_failure(self, tmp_path: Path) -> None:
         cfg = _make_config(tmp_path)
         proxy = BrokerProxy(cfg)
@@ -970,9 +1016,9 @@ def fake_stop() -> None:
             cfg.socket_path.unlink(missing_ok=True)
             cfg.version_file.unlink(missing_ok=True)
 
-        with patch.object(proxy, "_stop_stale_daemon", fake_stop), patch(
-            "subprocess.Popen"
-        ), pytest.raises(TimeoutError):
+        with patch.object(proxy, "_pid_belongs_to_broker", return_value=True), patch.object(
+            proxy, "_stop_stale_daemon", fake_stop
+        ), patch("subprocess.Popen"), pytest.raises(TimeoutError):
             await proxy._spawn_broker_if_needed()
 
         assert stop_called == [True]
@@ -988,7 +1034,9 @@ async def test_version_match_reuses_daemon(self, tmp_path: Path) -> None:
 
         proxy = BrokerProxy(cfg, auto_spawn=True, connect_timeout=0.3)
 
-        with patch("subprocess.Popen") as mock_popen:
+        with patch.object(proxy, "_pid_belongs_to_broker", return_value=True), patch(
+            "subprocess.Popen"
+        ) as mock_popen:
             await proxy._spawn_broker_if_needed()
 
         mock_popen.assert_not_called()