fix: spawn pi via resolved binary path and quiet missing-pi startup (#118)

* fix: spawn pi via resolved binary path and quiet missing-pi startup (#116)

* fix: pass pi system prompt as a file and resolve the binary once (#116 review)

CHANGELOG.yaml

@@ -24,6 +24,7 @@
   - "OpenRouter Client: Added a Parameters config tab with individual toggles for `temperature`, `top_p`, `top_k`, `min_p`, `frequency_penalty`, `presence_penalty`, and `repetition_penalty`. Toggling a parameter off omits it from the request entirely, for model providers that hard-error on parameters they don't support for the selected model — some, for example, only accept a `frequency_penalty` of exactly 0 for certain models. All default to on."
   - "Pi Bridge Client: The Docker image now ships with pi preinstalled, and pi's configuration directory (models.json, auth.json) is mounted from ./pi next to the compose file, so the client works in Docker out of the box."
   fixes:
+  - "Pi Bridge Client: On Windows the client could not launch pi at all — the startup model fetch logged an unhelpful '[WinError 2] The system cannot find the file specified' and generations failed the same way, because pi installed via npm is a pi.cmd shim that Windows only executes by its full path. The pi executable is now resolved to its full path before spawning, and if a launch still fails the error names the executable that could not be run. A missing pi install is also no longer reported at startup — it only surfaces on a configured Pi Bridge client."
   - "Client Status: A single failed status check (e.g. a brief connection timeout to a busy local KoboldCpp) no longer flaps a connected client to 'Could not connect' for a few seconds — the client is only marked disconnected after consecutive status failures."
   - "Agent Settings Dialog: Closing the dialog without changing anything no longer re-saves its settings — previously the untouched dialog pushed the copy it was opened with back to the backend, silently reverting any setting changed elsewhere while it was open (such as by the help agent). The help agent likewise refuses to change settings of an agent whose dialog is currently open, asking to close it first."
   - "OpenRouter Client: A failed model/provider list fetch at startup no longer sticks until the server is restarted — later config saves and client status refreshes now retry it. Setting the API key for the first time during initial setup also correctly triggers the provider fetch."

src/talemate/client/pi_bridge.py

@@ -3,6 +3,7 @@ import json
 import os
 import re
 import shutil
+import tempfile
 import time
 from typing import Literal
 
@@ -61,6 +62,17 @@ FETCH_RETRY_COOLDOWN = 30.0
 _models_last_attempt: float | None = None
 
 
+def resolve_pi_binary() -> str | None:
+    """Absolute path to the pi executable, or None when pi is not installed.
+
+    Spawns must use the resolved path: on Windows shutil.which finds npm's
+    pi.cmd shim via PATHEXT, but CreateProcess only looks for pi.exe when
+    given the bare name — spawning "pi" fails with WinError 2 even though
+    the availability check passed.
+    """
+    return shutil.which(PI_BINARY)
+
+
 def pi_subprocess_env() -> dict:
     """Environment for pi subprocesses: talemate-managed values layered over
     the process environment. The configured openrouter API key doubles as
@@ -108,13 +120,16 @@ async def fetch_available_models():
             return AVAILABLE_MODELS
         _models_last_attempt = now
 
-        if not shutil.which(PI_BINARY):
-            log.warning("pi binary not found, cannot fetch models")
+        pi_path = resolve_pi_binary()
+        if not pi_path:
+            # pi is optional — stay quiet until the user actually sets up a
+            # pi_bridge client, whose status reports the missing binary
+            log.debug("pi binary not found, skipping model fetch")
             return AVAILABLE_MODELS
 
         try:
             proc = await asyncio.create_subprocess_exec(
-                PI_BINARY,
+                pi_path,
                 "--list-models",
                 "--offline",
                 stdout=asyncio.subprocess.PIPE,
@@ -137,8 +152,15 @@ async def fetch_available_models():
                 providers=len(AVAILABLE_MODELS),
                 models=sum(len(models) for models in AVAILABLE_MODELS.values()),
             )
+        except FileNotFoundError as e:
+            # WinError 2 / ENOENT don't say which file — name the executable
+            log.error(
+                "error fetching models from pi - could not execute the pi binary",
+                path=pi_path,
+                error=str(e),
+            )
         except Exception as e:
-            log.error("error fetching models from pi", error=str(e))
+            log.error("error fetching models from pi", path=pi_path, error=str(e))
 
         return AVAILABLE_MODELS
 
@@ -313,7 +335,7 @@ class PiBridgeClient(ConcurrentInferenceMixin, ClientBase):
 
     @property
     def pi_available(self) -> bool:
-        return shutil.which(PI_BINARY) is not None
+        return resolve_pi_binary() is not None
 
     @property
     def reasoning_display(self) -> ReasoningDisplay | None:
@@ -367,11 +389,20 @@ class PiBridgeClient(ConcurrentInferenceMixin, ClientBase):
             await fetch_available_models()
         self.emit_status()
 
-    def _build_command(self, kind: str) -> list[str]:
-        """Assemble the pi RPC invocation for a single generation."""
+    def _build_command(
+        self, kind: str, pi_path: str, system_prompt_path: str
+    ) -> list[str]:
+        """Assemble the pi RPC invocation for a single generation.
+
+        The system prompt travels as a file (pi reads --system-prompt from a
+        path when one exists) rather than inline: on Windows the resolved pi
+        is npm's pi.cmd, which CreateProcess runs through cmd.exe, and cmd
+        re-parses the argv — an embedded newline would end the command and
+        the rest of the prompt would be interpreted as one.
+        """
         thinking = self.effort_level if self.reason_enabled else "off"
         return [
-            PI_BINARY,
+            pi_path,
             "--mode",
             "rpc",
             "--no-session",
@@ -387,7 +418,7 @@ class PiBridgeClient(ConcurrentInferenceMixin, ClientBase):
             "--no-context-files",
             "--no-prompt-templates",
             "--system-prompt",
-            self.get_system_message(kind),
+            system_prompt_path,
         ]
 
     async def generate(self, prompt: str, parameters: dict, kind: str):
@@ -395,7 +426,8 @@ class PiBridgeClient(ConcurrentInferenceMixin, ClientBase):
         Generates text by spawning a pi RPC subprocess for this request.
         """
 
-        if not self.pi_available:
+        pi_path = resolve_pi_binary()
+        if not pi_path:
             raise PiBridgeError("pi binary not found")
 
         self.log.debug(
@@ -405,49 +437,60 @@ class PiBridgeClient(ConcurrentInferenceMixin, ClientBase):
             provider=self.provider,
         )
 
-        proc = await asyncio.create_subprocess_exec(
-            *self._build_command(kind),
-            stdin=asyncio.subprocess.PIPE,
-            stdout=asyncio.subprocess.PIPE,
-            stderr=asyncio.subprocess.PIPE,
-            env=pi_subprocess_env(),
-            limit=PI_STDOUT_LIMIT,
-        )
+        with tempfile.TemporaryDirectory(prefix="talemate-pi-") as prompt_dir:
+            system_prompt_path = os.path.join(prompt_dir, "system-prompt.txt")
+            with open(system_prompt_path, "w", encoding="utf-8") as f:
+                f.write(self.get_system_message(kind))
 
-        # drain stderr continuously so a chatty pi process can never fill the
-        # OS pipe buffer and deadlock against our stdout readline loop; the
-        # collected output feeds the unexpected-exit error message.
-        stderr_task = asyncio.create_task(proc.stderr.read())
+            argv = self._build_command(kind, pi_path, system_prompt_path)
+            try:
+                proc = await asyncio.create_subprocess_exec(
+                    *argv,
+                    stdin=asyncio.subprocess.PIPE,
+                    stdout=asyncio.subprocess.PIPE,
+                    stderr=asyncio.subprocess.PIPE,
+                    env=pi_subprocess_env(),
+                    limit=PI_STDOUT_LIMIT,
+                )
+            except FileNotFoundError as e:
+                raise PiBridgeError(
+                    f"could not execute the pi binary: {pi_path}"
+                ) from e
 
-        try:
-            command = {"id": "generate", "type": "prompt", "message": prompt}
-            proc.stdin.write((json.dumps(command) + "\n").encode("utf-8"))
-            await proc.stdin.drain()
+            # drain stderr continuously so a chatty pi process can never fill
+            # the OS pipe buffer and deadlock against our stdout readline loop;
+            # the collected output feeds the unexpected-exit error message.
+            stderr_task = asyncio.create_task(proc.stderr.read())
 
-            response_text, reasoning_text = await self._consume_events(
-                proc, stderr_task
-            )
+            try:
+                rpc_command = {"id": "generate", "type": "prompt", "message": prompt}
+                proc.stdin.write((json.dumps(rpc_command) + "\n").encode("utf-8"))
+                await proc.stdin.drain()
 
-            self._reasoning_response = reasoning_text or None
+                response_text, reasoning_text = await self._consume_events(
+                    proc, stderr_task
+                )
 
-            self.log.debug(
-                "generated response",
-                response=response_text[:128] + " ..."
-                if len(response_text) > 128
-                else response_text,
-                reasoning_length=len(reasoning_text),
-            )
+                self._reasoning_response = reasoning_text or None
 
-            return response_text
-        finally:
-            if proc.returncode is None:
-                try:
-                    proc.kill()
-                except ProcessLookupError:
-                    pass
-            await proc.wait()
-            # the kill/exit above closes the pipe, so the drain finishes
-            await stderr_task
+                self.log.debug(
+                    "generated response",
+                    response=response_text[:128] + " ..."
+                    if len(response_text) > 128
+                    else response_text,
+                    reasoning_length=len(reasoning_text),
+                )
+
+                return response_text
+            finally:
+                if proc.returncode is None:
+                    try:
+                        proc.kill()
+                    except ProcessLookupError:
+                        pass
+                await proc.wait()
+                # the kill/exit above closes the pipe, so the drain finishes
+                await stderr_task
 
     async def _consume_events(self, proc, stderr_task) -> tuple[str, str]:
         """Read the pi RPC event stream until the agent settles.

tests/test_client_pi_bridge.py

@@ -10,9 +10,12 @@ from __future__ import annotations
 
 import asyncio
 import json
+import os
 
 import pytest
 
+from structlog.testing import capture_logs
+
 import talemate.config.state as config_state
 from talemate.client import pi_bridge
 from talemate.client.context import ClientContext, set_client_context_attribute
@@ -168,6 +171,9 @@ async def test_generate_sends_prompt_command_and_cleans_up(client, spawner):
 
     command = json.loads(proc.stdin.written.decode())
     assert command == {"id": "generate", "type": "prompt", "message": "tell a story"}
+    # the resolved binary path is spawned, not the bare name (Windows
+    # CreateProcess cannot find npm's pi.cmd shim by bare name)
+    assert spawner.calls[0]["args"][0] == "/usr/bin/pi"
     # the subprocess is terminated after the response
     assert proc.killed
     # the raised stream limit is requested from the subprocess reader
@@ -264,6 +270,19 @@ async def test_generate_error_on_provider_error(client, spawner):
     assert excinfo.value.status_code == 400
 
 
+@pytest.mark.asyncio
+async def test_generate_error_on_spawn_failure_names_binary(
+    client, spawner, monkeypatch
+):
+    async def spawn(*args, **kwargs):
+        raise FileNotFoundError(2, "The system cannot find the file specified")
+
+    monkeypatch.setattr(asyncio, "create_subprocess_exec", spawn)
+
+    with pytest.raises(pi_bridge.PiBridgeError, match="/usr/bin/pi"):
+        await client.generate("hi", {}, "conversation")
+
+
 @pytest.mark.asyncio
 async def test_generate_error_on_unexpected_exit(client, spawner):
     spawner.queue(FakeProcess([], stderr=b'Error: Unknown provider "nope".'))
@@ -338,8 +357,9 @@ async def test_concurrent_generates_use_isolated_processes(client, spawner):
 
 
 def test_build_command_pure_bridge_flags(client):
-    command = client._build_command("conversation")
+    command = client._build_command("conversation", "/usr/bin/pi", "/tmp/system.txt")
 
+    assert command[0] == "/usr/bin/pi"
     for flag in (
         "--no-tools",
         "--no-extensions",
@@ -352,6 +372,7 @@ def test_build_command_pure_bridge_flags(client):
 
     assert command[command.index("--provider") + 1] == "openrouter"
     assert command[command.index("--model") + 1] == "deepseek/deepseek-v4-flash"
+    assert command[command.index("--system-prompt") + 1] == "/tmp/system.txt"
     # reasoning disabled by default
     assert command[command.index("--thinking") + 1] == "off"
 
@@ -360,11 +381,37 @@ def test_build_command_thinking_level(client):
     client.client_config.reason_enabled = True
     client.client_config.effort_level = "high"
 
-    command = client._build_command("conversation")
+    command = client._build_command("conversation", "/usr/bin/pi", "/tmp/system.txt")
 
     assert command[command.index("--thinking") + 1] == "high"
 
 
+@pytest.mark.asyncio
+async def test_generate_passes_system_prompt_as_file(client, spawner, monkeypatch):
+    """The system prompt goes to pi as a file path, never inline on the argv:
+    on Windows the resolved pi is npm's pi.cmd and cmd.exe re-parses the
+    command line, where the prompt's newlines would truncate it."""
+    message = _assistant_message([{"type": "text", "text": "ok"}])
+    spawner.queue(FakeProcess(_generation_events(message)))
+
+    system_message = client.get_system_message("conversation")
+    seen = {}
+
+    async def spawn(*args, **kwargs):
+        path = args[args.index("--system-prompt") + 1]
+        seen["path"] = path
+        seen["contents"] = open(path, encoding="utf-8").read()
+        return spawner.processes.pop(0)
+
+    monkeypatch.setattr(asyncio, "create_subprocess_exec", spawn)
+    await client.generate("hi", {}, "conversation")
+
+    assert seen["contents"] == system_message
+    assert system_message not in seen["path"]
+    # the temporary file is cleaned up once the generation finishes
+    assert not os.path.exists(seen["path"])
+
+
 # ---------------------------------------------------------------------------
 # model catalog
 # ---------------------------------------------------------------------------
@@ -429,6 +476,7 @@ async def test_fetch_available_models_success(monkeypatch, catalog_state):
     monkeypatch.setattr(pi_bridge.shutil, "which", lambda _: "/usr/bin/pi")
 
     async def spawn(*args, **kwargs):
+        assert args[0] == "/usr/bin/pi"
         assert "--list-models" in args
         return FakeListModelsProcess(PI_LIST_MODELS_OUTPUT.encode())
 
@@ -466,12 +514,38 @@ async def test_fetch_available_models_failure_stays_unlatched(
 
 @pytest.mark.asyncio
 async def test_fetch_available_models_missing_binary(monkeypatch, catalog_state):
+    """pi not being installed is not an error - the startup fetch stays
+    silent (debug only); the error belongs to configured pi_bridge clients."""
     monkeypatch.setattr(pi_bridge.shutil, "which", lambda _: None)
 
-    models = await pi_bridge.fetch_available_models()
+    with capture_logs() as logs:
+        models = await pi_bridge.fetch_available_models()
 
     assert models == {}
     assert not pi_bridge.MODELS_FETCHED
+    assert all(entry["log_level"] == "debug" for entry in logs)
+
+
+@pytest.mark.asyncio
+async def test_fetch_available_models_spawn_failure_names_binary(
+    monkeypatch, catalog_state
+):
+    """A spawn-time FileNotFoundError (e.g. WinError 2) is logged with the
+    resolved executable path - the raw OS error doesn't name the file."""
+    monkeypatch.setattr(pi_bridge.shutil, "which", lambda _: "/usr/bin/pi")
+
+    async def spawn(*args, **kwargs):
+        raise FileNotFoundError(2, "The system cannot find the file specified")
+
+    monkeypatch.setattr(asyncio, "create_subprocess_exec", spawn)
+
+    with capture_logs() as logs:
+        models = await pi_bridge.fetch_available_models()
+
+    assert models == {}
+    assert not pi_bridge.MODELS_FETCHED
+    errors = [entry for entry in logs if entry["log_level"] == "error"]
+    assert errors and errors[0]["path"] == "/usr/bin/pi"
 
 
 @pytest.mark.asyncio