Merge pull request #43 from jacob-bd/release-hardening

fix: harden Windows daemon and setup failure paths

Author: jacob-bd

README.md

@@ -278,6 +278,7 @@ pwm serve-mcp --transport streamable-http  # Start HTTP MCP on 127.0.0.1:8000/mc
 pwm serve-mcp --status                # Check daemon status
 pwm serve-mcp --stop                  # Stop the daemon
 pwm setup add codex --http            # Configure Codex for Streamable HTTP
+pwm serve-mcp --transport sse         # Start the legacy SSE transport
 ```
 
 The HTTP transports bind to loopback by default. Keep the daemon on a loopback

skills/perplexity-web-mcp/SKILL.md

@@ -283,6 +283,7 @@ pwm serve-mcp --transport streamable-http  # HTTP MCP on 127.0.0.1:8000/mcp
 pwm serve-mcp --status                # Check the daemon
 pwm serve-mcp --stop                  # Stop the daemon
 pwm setup add codex --http            # Configure Codex for Streamable HTTP
+pwm serve-mcp --transport sse         # Start the legacy SSE transport
 ```
 
 The HTTP transports bind to loopback by default. Keep the daemon on a loopback

src/perplexity_web_mcp/cli/setup.py

@@ -258,6 +258,9 @@ def _setup_codex(streamable_http: bool = False, port: int = 8000) -> bool:
                 console.print(f"[yellow]Warning:[/yellow] Could not read Codex config: {exc}")
                 return False
             mcp_servers = config.get("mcp_servers", {})
+            if not isinstance(mcp_servers, dict):
+                console.print("[yellow]Warning:[/yellow] Codex mcp_servers configuration is not a table.")
+                return False
             existing_key = next(
                 (key for key in (MCP_SERVER_KEY, "perplexity-web-mcp") if key in mcp_servers),
                 None,
@@ -891,6 +894,9 @@ def setup_add(client, streamable_http, port):
     else:
         success = _setup_json_client(client)
 
+    if client == "codex" and not success:
+        raise click.ClickException("Could not configure Codex MCP server.")
+
     if success:
         console.print(f"\n[dim]Restart {info['name']} to activate the MCP server.[/dim]")
 

src/perplexity_web_mcp/data/SKILL.md

@@ -283,6 +283,7 @@ pwm serve-mcp --transport streamable-http  # HTTP MCP on 127.0.0.1:8000/mcp
 pwm serve-mcp --status                # Check the daemon
 pwm serve-mcp --stop                  # Stop the daemon
 pwm setup add codex --http            # Configure Codex for Streamable HTTP
+pwm serve-mcp --transport sse         # Start the legacy SSE transport
 ```
 
 The HTTP transports bind to loopback by default. Keep the daemon on a loopback

src/perplexity_web_mcp/mcp/server.py

@@ -784,40 +784,38 @@ def get_daemon_pid_path(port: int) -> Path:
 
 
 def _is_windows_pid_running(pid: int) -> bool:
-    """Check process state without sending a signal on Windows.
-
-    Indeterminate results are treated as running so a valid daemon lock is not
-    discarded merely because the process cannot be inspected.
-    """
-    import ctypes
-    from ctypes import wintypes
-
-    synchronize = 0x00100000
-    error_invalid_parameter = 87
-    wait_object_0 = 0x00000000
-    wait_timeout = 0x00000102
-
-    kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
-    kernel32.OpenProcess.argtypes = (wintypes.DWORD, wintypes.BOOL, wintypes.DWORD)
-    kernel32.OpenProcess.restype = wintypes.HANDLE
-    kernel32.WaitForSingleObject.argtypes = (wintypes.HANDLE, wintypes.DWORD)
-    kernel32.WaitForSingleObject.restype = wintypes.DWORD
-    kernel32.CloseHandle.argtypes = (wintypes.HANDLE,)
-    kernel32.CloseHandle.restype = wintypes.BOOL
-
-    handle = kernel32.OpenProcess(synchronize, False, pid)
-    if not handle:
-        return ctypes.get_last_error() != error_invalid_parameter
-
     try:
-        wait_result = kernel32.WaitForSingleObject(handle, 0)
-        if wait_result == wait_object_0:
-            return False
-        if wait_result == wait_timeout:
+        import ctypes
+        from ctypes import wintypes
+
+        synchronize = 0x00100000
+        error_invalid_parameter = 87
+        wait_object_0 = 0x00000000
+        wait_timeout = 0x00000102
+
+        kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
+        kernel32.OpenProcess.argtypes = (wintypes.DWORD, wintypes.BOOL, wintypes.DWORD)
+        kernel32.OpenProcess.restype = wintypes.HANDLE
+        kernel32.WaitForSingleObject.argtypes = (wintypes.HANDLE, wintypes.DWORD)
+        kernel32.WaitForSingleObject.restype = wintypes.DWORD
+        kernel32.CloseHandle.argtypes = (wintypes.HANDLE,)
+        kernel32.CloseHandle.restype = wintypes.BOOL
+
+        handle = kernel32.OpenProcess(synchronize, False, pid)
+        if not handle:
+            return ctypes.get_last_error() != error_invalid_parameter
+
+        try:
+            wait_result = kernel32.WaitForSingleObject(handle, 0)
+            if wait_result == wait_object_0:
+                return False
+            if wait_result == wait_timeout:
+                return True
             return True
+        finally:
+            kernel32.CloseHandle(handle)
+    except Exception:
         return True
-    finally:
-        kernel32.CloseHandle(handle)
 
 
 def is_pid_running(pid: int) -> bool:
@@ -852,11 +850,23 @@ def get_running_daemon_pid(port: int) -> int | None:
         return None
     try:
         pid = int(pid_path.read_text().strip())
+    except (OSError, ValueError):
+        try:
+            pid_path.unlink(missing_ok=True)
+        except OSError:
+            pass
+        return None
+
+    try:
         if is_pid_running(pid):
             return pid
-        pid_path.unlink(missing_ok=True)
     except Exception:
+        return pid
+
+    try:
         pid_path.unlink(missing_ok=True)
+    except OSError:
+        pass
     return None
 
 

tests/test_mcp_server.py

@@ -178,6 +178,25 @@ def test_windows_live_pid_file_is_retained_and_blocks_duplicate(tmp_path: Path,
     probe.assert_called_once_with(4242)
 
 
+def test_windows_pid_probe_treats_unexpected_api_error_as_running() -> None:
+    with (
+        patch.object(server.sys, "platform", "win32"),
+        patch.object(ctypes, "WinDLL", side_effect=OSError("probe unavailable"), create=True),
+    ):
+        assert server.is_pid_running(4242) is True
+
+
+def test_windows_pid_probe_error_preserves_pid_file(tmp_path: Path, monkeypatch) -> None:
+    monkeypatch.setattr(server, "CONFIG_DIR", tmp_path)
+    pid_path = server.get_daemon_pid_path(8994)
+    pid_path.write_text("4242", encoding="utf-8")
+
+    with patch.object(server, "is_pid_running", side_effect=RuntimeError("probe unavailable")):
+        assert server.get_running_daemon_pid(8994) == 4242
+
+    assert pid_path.exists()
+
+
 @pytest.mark.skipif(sys.platform != "win32", reason="requires Windows process APIs")
 def test_windows_pid_probe_preserves_live_child() -> None:
     child = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"])

tests/test_setup.py

@@ -258,6 +258,12 @@ def test_add_codex_sse_remains_a_legacy_alias(self) -> None:
             assert result.exit_code == 0
             mock_setup.assert_called_once_with(streamable_http=True, port=8000)
 
+    def test_add_codex_http_propagates_setup_failure(self) -> None:
+        with patch("perplexity_web_mcp.cli.setup._setup_codex", return_value=False):
+            result = self._run("add", "codex", "--http")
+
+        assert result.exit_code == 1
+
     def test_remove_codex_removes_mcp(self) -> None:
         with patch("perplexity_web_mcp.cli.setup._remove_codex", return_value=True) as mock_remove:
             result = self._run("remove", "codex")
@@ -514,6 +520,22 @@ def test_setup_codex_http_refuses_disabled_matching_config(tmp_path: Path) -> No
     assert config_path.read_text() == original
 
 
+def test_setup_codex_http_refuses_non_table_mcp_servers(tmp_path: Path) -> None:
+    from perplexity_web_mcp.cli.setup import _setup_codex
+
+    config_path = tmp_path / "config.toml"
+    original = "mcp_servers = []\n"
+    config_path.write_text(original)
+
+    with (
+        patch("perplexity_web_mcp.cli.setup._codex_config_path", return_value=tmp_path),
+        patch("perplexity_web_mcp.cli.setup.shutil.which", return_value=None),
+    ):
+        assert _setup_codex(streamable_http=True) is False
+
+    assert config_path.read_text() == original
+
+
 def test_setup_codex_http_writes_mcp_endpoint_without_codex_cli(tmp_path: Path) -> None:
     from perplexity_web_mcp.cli.setup import _setup_codex
 

tests/test_skill.py

@@ -289,6 +289,15 @@ def test_uninstall_removes_directory(self, skill_source: Path, dest_dir: Path) -
         assert result is True
         assert not installed.exists()
 
+    def test_uninstall_routes_through_readonly_safe_removal(self, skill_source: Path, dest_dir: Path) -> None:
+        _install_skill(skill_source, dest_dir)
+        installed = dest_dir / SKILL_DIR_NAME
+
+        with patch("perplexity_web_mcp.cli.skill._remove_tree") as remove_tree:
+            assert _uninstall_skill(dest_dir) is True
+
+        remove_tree.assert_called_once_with(installed)
+
     def test_uninstall_removes_readonly_tree(self, skill_source: Path, dest_dir: Path) -> None:
         _install_skill(skill_source, dest_dir)
         installed = dest_dir / SKILL_DIR_NAME