Harden MCP transport error handling

Normalize timeout output to text, convert tool exceptions into tool errors, and keep stdio running after response serialization failures.

3 files changed, 123 insertions, 10 deletions

diff --git a/src/sailfish_devel_mcp/runner.py b/src/sailfish_devel_mcp/runner.py
index b0b88bf..74748f4 100644
--- a/src/sailfish_devel_mcp/runner.py
+++ b/src/sailfish_devel_mcp/runner.py
@@ -4,7 +4,7 @@ from dataclasses import dataclass
 from pathlib import Path
 import shlex
 import subprocess
-from typing import Iterable, Sequence
+from typing import Any, Iterable, Sequence
 
 from .config import DeviceConfig
 
@@ -39,6 +39,16 @@ def truncate(text: str, limit: int) -> tuple[str, bool]:
     return text[:limit] + f"\n[truncated after {limit} characters]", True
 
 
+def ensure_text(value: Any) -> str:
+    if value is None:
+        return ""
+    if isinstance(value, str):
+        return value
+    if isinstance(value, bytes):
+        return value.decode("utf-8", errors="replace")
+    return str(value)
+
+
 def run(
     argv: Sequence[str],
     *,
@@ -57,15 +67,15 @@ def run(
         return CommandResult(
             argv=tuple(str(arg) for arg in argv),
             returncode=completed.returncode,
-            stdout=completed.stdout,
-            stderr=completed.stderr,
+            stdout=ensure_text(completed.stdout),
+            stderr=ensure_text(completed.stderr),
         )
     except subprocess.TimeoutExpired as exc:
         return CommandResult(
             argv=tuple(str(arg) for arg in argv),
             returncode=124,
-            stdout=exc.stdout or "",
-            stderr=(exc.stderr or "") + f"\ncommand timed out after {timeout}s",
+            stdout=ensure_text(exc.stdout),
+            stderr=ensure_text(exc.stderr) + f"\ncommand timed out after {timeout}s",
         )
 
 
diff --git a/src/sailfish_devel_mcp/server.py b/src/sailfish_devel_mcp/server.py
index 4fb2f91..7982b21 100644
--- a/src/sailfish_devel_mcp/server.py
+++ b/src/sailfish_devel_mcp/server.py
@@ -119,6 +119,15 @@ class McpServer:
             return self.registry[name].handler(args)
         except ValueError as exc:
             return tool_error(str(exc))
+        except Exception as exc:
+            return tool_error(
+                f"{name} failed: {exc}",
+                {
+                    "error": str(exc),
+                    "exception": type(exc).__name__,
+                    "tool": name,
+                },
+            )
 
     def _resources_list(self) -> dict[str, Any]:
         return {
@@ -158,6 +167,7 @@ def run_stdio(server: McpServer, stdin: TextIO = sys.stdin, stdout: TextIO = sys
     for line in stdin:
         if not line.strip():
             continue
+        message: Any = None
         try:
             message = json.loads(line)
         except json.JSONDecodeError as exc:
@@ -169,8 +179,26 @@ def run_stdio(server: McpServer, stdin: TextIO = sys.stdin, stdout: TextIO = sys
         else:
             response = server.handle(message)
         if response is not None:
-            stdout.write(json.dumps(response, separators=(",", ":")) + "\n")
-            stdout.flush()
+            try:
+                response_text = json.dumps(response, separators=(",", ":"))
+            except TypeError as exc:
+                request_id = message.get("id") if isinstance(message, dict) else None
+                response_text = json.dumps(
+                    {
+                        "jsonrpc": "2.0",
+                        "id": request_id,
+                        "error": {
+                            "code": -32603,
+                            "message": f"response serialization failed: {exc}",
+                        },
+                    },
+                    separators=(",", ":"),
+                )
+            try:
+                stdout.write(response_text + "\n")
+                stdout.flush()
+            except BrokenPipeError:
+                return
 
 
 def main(argv: list[str] | None = None) -> None:
@@ -193,4 +221,3 @@ def main(argv: list[str] | None = None) -> None:
 
 if __name__ == "__main__":
     main()
-
diff --git a/tests/test_server.py b/tests/test_server.py
index 26d01b7..c7f93d5 100644
--- a/tests/test_server.py
+++ b/tests/test_server.py
@@ -1,8 +1,10 @@
 from __future__ import annotations
 
+from io import StringIO
 import json
 import os
 from pathlib import Path
+import subprocess
 import tempfile
 import unittest
 from unittest.mock import patch
@@ -14,8 +16,8 @@ from sailfish_devel_mcp.config import (
     PathConfig,
     load_config,
 )
-from sailfish_devel_mcp.runner import CommandResult
-from sailfish_devel_mcp.server import McpServer
+from sailfish_devel_mcp.runner import CommandResult, run
+from sailfish_devel_mcp.server import McpServer, run_stdio
 from sailfish_devel_mcp.tools import _device_home_path, _screenshot_prepare_command
 from sailfish_devel_mcp.vendor import build_sailfishos
 
@@ -143,6 +145,80 @@ class McpServerTests(unittest.TestCase):
             )
         json.dumps(response)
 
+    def test_timeout_output_bytes_are_normalized_to_text(self) -> None:
+        timeout = subprocess.TimeoutExpired(
+            cmd=("ssh", "root@test"),
+            timeout=1,
+            output=b"partial stdout\n",
+            stderr=b"partial stderr\xff\n",
+        )
+        with patch("sailfish_devel_mcp.runner.subprocess.run", side_effect=timeout):
+            result = run(["ssh", "root@test"], timeout=1)
+
+        self.assertEqual(result.returncode, 124)
+        self.assertIsInstance(result.stdout, str)
+        self.assertIsInstance(result.stderr, str)
+        self.assertIn("partial stdout", result.stdout)
+        self.assertIn("partial stderr", result.stderr)
+        self.assertIn("command timed out after 1s", result.stderr)
+        json.dumps(result.public_dict())
+
+    def test_tool_exception_returns_tool_error_and_server_recovers(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            server = self.make_server(Path(tmp))
+            with patch(
+                "sailfish_devel_mcp.tools.handle_devices",
+                side_effect=RuntimeError("boom"),
+            ):
+                response = server.handle(
+                    {
+                        "jsonrpc": "2.0",
+                        "id": 6,
+                        "method": "tools/call",
+                        "params": {"name": "sailfish_devices", "arguments": {}},
+                    }
+                )
+            follow_up = server.handle(
+                {"jsonrpc": "2.0", "id": 7, "method": "tools/list", "params": {}}
+            )
+
+        self.assertNotIn("error", response)
+        self.assertTrue(response["result"]["isError"])
+        self.assertEqual(
+            response["result"]["structuredContent"]["exception"],
+            "RuntimeError",
+        )
+        self.assertIn("tools", follow_up["result"])
+
+    def test_stdio_serialization_failure_does_not_stop_server(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            server = self.make_server(Path(tmp))
+            stdin = StringIO(
+                "\n".join(
+                    [
+                        '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}',
+                        '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}',
+                    ]
+                )
+                + "\n"
+            )
+            stdout = StringIO()
+            with patch.object(
+                server,
+                "handle",
+                side_effect=[
+                    {"jsonrpc": "2.0", "id": 1, "result": {"bad": {1}}},
+                    {"jsonrpc": "2.0", "id": 2, "result": {"ok": True}},
+                ],
+            ):
+                run_stdio(server, stdin, stdout)
+
+        responses = [json.loads(line) for line in stdout.getvalue().splitlines()]
+        self.assertEqual(responses[0]["id"], 1)
+        self.assertEqual(responses[0]["error"]["code"], -32603)
+        self.assertIn("response serialization failed", responses[0]["error"]["message"])
+        self.assertEqual(responses[1]["result"], {"ok": True})
+
     def test_default_config_uses_bundled_build_helper(self) -> None:
         with tempfile.TemporaryDirectory() as tmp:
             missing = Path(tmp) / "missing-config.json"