Spawn server processes from argv directly, no shell in between

Every FineCode-owned server (LSP servers, the ER) was started via
create_subprocess_shell with a single joined command string. A shell
in the middle re-parses that string, so a path containing a space or
shell metacharacter breaks, and on Windows cmd.exe becomes the actual
parent -- which is exactly what left an ER alive with nothing on its
pipes: the recorded pid was the shell wrapper's, not the server's, so
the process-group kill and the pid FineCode tracks disagreed.

- New finecode_jsonrpc._spawn.spawn_process() is the one place that
  calls create_subprocess_exec, shared by JsonRpcClient.start_server
  and StdioTransport.start. It rejects a str (which satisfies
  Sequence[str] so the type checker can't catch it, but would exec
  character-by-character) and an empty argv. Windows now sets only
  CREATE_NO_WINDOW: Microsoft documents it as ignored when combined
  with DETACHED_PROCESS, which the old code also passed.
- cmd/server_cmd/full_cmd move from str to Sequence[str] across
  IJsonRpcClient, ILspClient, LspService, JsonRpcClientImpl, and the
  runner_manager ER launch, so no caller can rejoin argv into a string
  first. LspService and spawn_process both raise TypeError at
  construction/spawn time rather than miscounting argv later.
- Add finecode_jsonrpc._spawn_selfcheck, run from
  setup-dev-workspace.sh right before prepare-envs: it spawns a fake
  TCP and a fake STDIO server through the real client/transport code
  paths and checks the recorded pid matches the server's own, so a
  spawn regression shows up as a two-line diagnostic instead of a
  prepare-envs timeout.

Author: Aksem

extensions/fine_python_pyrefly/fine_python_pyrefly/pyrefly_lsp_service.py

@@ -84,7 +84,7 @@ def __init__(
             lsp_client=lsp_client,
             file_editor=file_editor,
             logger=logger,
-            cmd=f"{pyrefly_bin} lsp",
+            cmd=[str(pyrefly_bin), "lsp"],
             language_id="python",
             readable_id="pyrefly-lsp",
             client_capabilities=_PYREFLY_CLIENT_CAPABILITIES,

extensions/fine_python_ruff/fine_python_ruff/ruff_lsp_service.py

@@ -165,7 +165,7 @@ def __init__(
             lsp_client=lsp_client,
             file_editor=file_editor,
             logger=logger,
-            cmd=f"{ruff_bin} server",
+            cmd=[str(ruff_bin), "server"],
             language_id="python",
             readable_id="ruff-lsp",
             client_capabilities=_RUFF_CLIENT_CAPABILITIES,

extensions/fine_python_ruff/tests/test_ruff_lsp_integration.py

@@ -20,11 +20,10 @@
 import asyncio
 import contextlib
 import json
-import shlex
 import sys
 import time
 import typing
-from collections.abc import AsyncIterator
+from collections.abc import AsyncIterator, Sequence
 from pathlib import Path
 from typing import Any, Self
 
@@ -64,7 +63,7 @@
 class _StdioLspSession:
     """Speaks LSP to a subprocess: enough of ILspSession for LspService."""
 
-    def __init__(self, cmd: str, root_uri: str, **kwargs: Any) -> None:
+    def __init__(self, cmd: Sequence[str], root_uri: str, **kwargs: Any) -> None:
         self._cmd = cmd
         self._root_uri = root_uri
         self._client_capabilities = kwargs.get("client_capabilities") or {}
@@ -80,7 +79,7 @@ def __init__(self, cmd: str, root_uri: str, **kwargs: Any) -> None:
 
     async def __aenter__(self) -> Self:
         self._process = await asyncio.create_subprocess_exec(
-            *shlex.split(self._cmd),
+            *self._cmd,
             stdin=asyncio.subprocess.PIPE,
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.DEVNULL,

extensions/fine_toml_tombi/fine_toml_tombi/tombi_lsp_service.py

@@ -65,7 +65,7 @@ def __init__(
             # against PyPI) while analyzing a pyproject.toml, on top of its local
             # schema cache. No action here consumes live dependency data, so the
             # lookups are latency for results nothing reads.
-            cmd=f"{tombi_bin} lsp --offline",
+            cmd=[str(tombi_bin), "lsp", "--offline"],
             language_id="toml",
             readable_id="tombi-lsp",
             client_capabilities=_TOMBI_CLIENT_CAPABILITIES,

finecode_extension_api/src/finecode_extension_api/contrib/lsp_service.py

@@ -86,17 +86,24 @@ def __init__(
         file_editor: ifileeditor.IFileEditor,
         logger: ilogger.ILogger,
         *,
-        cmd: str,
+        cmd: collections.abc.Sequence[str],
         language_id: str,
         readable_id: str = "",
         client_capabilities: dict[str, Any] | None = None,
         max_concurrent_requests: int | None = None,
         empty_diagnostics_settle_sec: float = 1.0,
     ) -> None:
+        # A str satisfies Sequence[str], so the type checker cannot reject it —
+        # and tuple(cmd) would silently explode "ruff server" into characters.
+        # Fail here, at service construction, rather than at the first spawn.
+        if isinstance(cmd, str):
+            raise TypeError("cmd must be an argv sequence, not a str")
         self._lsp_client = lsp_client
         self._file_editor = file_editor
         self._logger = logger
-        self._cmd = cmd
+        # tuple() so a caller mutating its list afterwards cannot change a later
+        # restart's command.
+        self._cmd = tuple(cmd)
         self._language_id = language_id
         self._readable_id = readable_id
         self._client_capabilities = client_capabilities

finecode_extension_api/src/finecode_extension_api/interfaces/ijsonrpcclient.py

@@ -108,7 +108,7 @@ class IJsonRpcClient(Protocol):
 
     def session(
         self,
-        cmd: str,
+        cmd: collections.abc.Sequence[str],
         cwd: Path | None = None,
         env: dict[str, str] | None = None,
         readable_id: str = "",
@@ -117,11 +117,12 @@ def session(
 
         Usage::
 
-            async with json_rpc_client.session("some-server --stdio") as session:
+            async with json_rpc_client.session(["some-server", "--stdio"]) as session:
                 result = await session.send_request("method", {"key": "value"})
 
         Args:
-            cmd: Shell command to start the JSON-RPC server process.
+            cmd: Program and arguments of the server process, executed directly
+                (no shell); a ``str`` is rejected.
             cwd: Working directory for the subprocess.
             env: Environment variables for the subprocess.
             readable_id: Human-readable identifier for logging.

finecode_extension_api/src/finecode_extension_api/interfaces/ilspclient.py

@@ -110,7 +110,7 @@ class ILspClient(Protocol):
 
     def session(
         self,
-        cmd: str,
+        cmd: collections.abc.Sequence[str],
         root_uri: str,
         workspace_folders: list[dict[str, str]] | None = None,
         initialization_options: dict[str, Any] | None = None,
@@ -126,7 +126,7 @@ def session(
         Usage::
 
             async with lsp_client.session(
-                cmd="pyright-langserver --stdio",
+                cmd=["pyright-langserver", "--stdio"],
                 root_uri="file:///path/to/project",
             ) as session:
                 result = await session.send_request(
@@ -135,7 +135,8 @@ def session(
                 )
 
         Args:
-            cmd: Shell command to start the language server.
+            cmd: Program and arguments of the server process, executed directly
+                (no shell); a ``str`` is rejected.
             root_uri: The root URI of the workspace.
             workspace_folders: Optional workspace folders (each with 'uri' and 'name' keys).
             initialization_options: Optional server-specific initialization options.

finecode_extension_api/tests/test_lsp_service.py

@@ -235,7 +235,7 @@ async def _running_service(
         lsp_client=_FakeLspClient(session),
         file_editor=file_editor,  # type: ignore[arg-type]
         logger=_NullLogger(),  # type: ignore[arg-type]
-        cmd="fake-lsp-server",
+        cmd=["fake-lsp-server"],
         language_id="python",
         max_concurrent_requests=max_concurrent_requests,
     )
@@ -767,12 +767,26 @@ def test_concurrency_limit_below_one_is_rejected() -> None:
             lsp_client=_FakeLspClient(_FakeLspSession()),
             file_editor=file_editor,  # type: ignore[arg-type]
             logger=_NullLogger(),  # type: ignore[arg-type]
-            cmd="fake-lsp-server",
+            cmd=["fake-lsp-server"],
             language_id="python",
             max_concurrent_requests=0,
         )
 
 
+def test_cmd_str_is_rejected() -> None:
+    """A str command would be exec'd character-by-character at spawn, long after
+    construction; reject it when the service is built."""
+    file_editor = _FakeFileEditor(Path("/nonexistent"), "")
+    with pytest.raises(TypeError, match="argv sequence"):
+        LspService(
+            lsp_client=_FakeLspClient(_FakeLspSession()),
+            file_editor=file_editor,  # type: ignore[arg-type]
+            logger=_NullLogger(),  # type: ignore[arg-type]
+            cmd="fake-lsp-server",
+            language_id="python",
+        )
+
+
 async def test_two_waiters_for_one_file_are_both_woken_by_its_diagnostics(
     tmp_path: Path,
 ) -> None:

finecode_extension_runner/src/finecode_extension_runner/impls/lsp_client.py

@@ -161,7 +161,7 @@ def __init__(self, json_rpc_client: ijsonrpcclient.IJsonRpcClient) -> None:
 
     def session(
         self,
-        cmd: str,
+        cmd: collections.abc.Sequence[str],
         root_uri: str,
         workspace_folders: list[dict[str, str]] | None = None,
         initialization_options: dict[str, Any] | None = None,

finecode_jsonrpc/src/finecode_jsonrpc/_spawn.py

@@ -0,0 +1,69 @@
+"""Spawn FineCode-owned server processes from an argv sequence.
+
+A server is exec'd directly from argv with no shell between the caller and the
+process, so a path containing spaces or shell metacharacters cannot be re-parsed
+and no ``cmd.exe`` sits in the middle on Windows. On Windows the only creation
+flag is ``CREATE_NO_WINDOW``: Microsoft documents it as ignored when combined
+with ``DETACHED_PROCESS``, so adding the latter silently disables the hidden
+console this intends.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import collections.abc
+import sys
+import typing
+from pathlib import Path
+
+__all__ = ["SpawnCommand", "spawn_process"]
+
+SpawnCommand = collections.abc.Sequence[str]
+
+# Win32 CREATE_NO_WINDOW (processthreadsapi). Spelled as a literal because
+# `subprocess.CREATE_NO_WINDOW` only exists on Windows, and the flag choice is
+# unit-tested on every platform.
+_CREATE_NO_WINDOW = 0x08000000
+
+
+def _platform_spawn_kwargs(platform: str) -> dict[str, typing.Any]:
+    if platform == "win32":
+        return {"creationflags": _CREATE_NO_WINDOW}
+    return {"start_new_session": True}
+
+
+async def spawn_process(
+    cmd: SpawnCommand,
+    *,
+    stdin_pipe: bool,
+    cwd: Path | None,
+    env: dict[str, str] | None,
+    limit: int | None = None,
+) -> asyncio.subprocess.Process:
+    """Spawn *cmd* as a child process, executing it directly.
+
+    Raises:
+        TypeError: *cmd* is a ``str``. A ``str`` satisfies ``Sequence[str]``, so
+            the type checker cannot reject one, and exec'ing ``"ruff server"``
+            would run argv ``['r', 'u', 'f', ...]`` instead of the server.
+        ValueError: *cmd* is empty.
+    """
+    if isinstance(cmd, str):
+        raise TypeError("cmd must be an argv sequence, not a str")
+    argv = list(cmd)
+    if not argv:
+        raise ValueError("empty command")
+
+    kwargs = _platform_spawn_kwargs(sys.platform)
+    if limit is not None:
+        kwargs["limit"] = limit
+
+    return await asyncio.create_subprocess_exec(
+        *argv,
+        stdin=asyncio.subprocess.PIPE if stdin_pipe else None,
+        stdout=asyncio.subprocess.PIPE,
+        stderr=asyncio.subprocess.PIPE,
+        cwd=cwd,
+        env=env,
+        **kwargs,
+    )