fix(mcp): avoid replay after mid-call stdio exit

(cherry picked from commit b29e68d0b020c805d1fccfc55f3a7c6ee6589ba9)

tests/tools/test_mcp_stdio_fastfail_reconnect.py

@@ -9,15 +9,17 @@ subprocess (#95626 added the reconnect signal).
 Signalling alone still lost the call: a gateway restart kills every MCP stdio
 child, and the first call from a surviving agent session (or a cron run
 spanning the restart) failed in 0.00s while the subprocess was respawned
-seconds later. Both fast-fail sites now respawn AND retry once:
+seconds later. Both fast-fail sites request a respawn, but only a call that
+has not reached the server is safe to retry:
 
-- pre-call gate (children already dead when the call arrives);
-- mid-call watcher race (children die while the RPC is in flight).
+- pre-call gate (children already dead when the call arrives): retry once;
+- mid-call watcher race (children die while the RPC is in flight): report an
+  uncertain outcome without replaying a possibly completed side effect.
 
-Both must recover transparently, and both must stop after ONE retry so a
-server that keeps dying parks via run()'s rapid-drop budget instead of
-hot-cycling respawns forever. The error text must never claim a timeout —
-that wording is what misdirected the original investigation.
+The pre-call path must stop after ONE retry so a server that keeps dying parks
+via run()'s rapid-drop budget instead of hot-cycling respawns forever. The
+error text must never claim a timeout — that wording is what misdirected the
+original investigation.
 """
 
 import asyncio
@@ -131,19 +133,22 @@ def test_precall_dead_children_respawn_and_retry(monkeypatch, tmp_path):
         _cleanup(mcp_tool, "srv-dead")
 
 
-def test_midcall_child_exit_respawn_and_retry(monkeypatch, tmp_path):
-    """Subprocess dies while the RPC is in flight → respawn and retry once,
-    so the caller still gets its result."""
+def test_midcall_child_exit_reconnects_without_replay(monkeypatch, tmp_path):
+    """An in-flight side effect may have completed, so reconnect but do not replay it."""
     monkeypatch.setenv("HERMES_HOME", str(tmp_path))
     from tools import mcp_tool
     from tools.mcp_tool_handlers import _make_tool_handler
 
     alive = {"v": True}
+    effects = {"n": 0}
 
     async def _hanging_call(*a, **kw):
+        effects["n"] += 1
+        alive["v"] = False
         await asyncio.sleep(30)
 
     async def _good_call(*a, **kw):
+        effects["n"] += 1
         return _success_result()
 
     async def _watch_children():
@@ -167,12 +172,12 @@ def test_midcall_child_exit_respawn_and_retry(monkeypatch, tmp_path):
     _mcp_loop._ensure_mcp_loop()
     try:
         handler = _make_tool_handler("srv-midcall", "tool1", 10.0)
-        # The child dies once the RPC is in flight.
-        alive["v"] = False
         parsed = json.loads(handler({}))
-        assert "error" not in parsed, parsed
-        assert parsed["result"] == "ok", parsed
+        assert parsed["outcome_uncertain"] is True, parsed
+        assert "may have completed" in parsed["error"], parsed
+        assert "did not replay" in parsed["error"], parsed
         assert server._reconnect_event.set_calls == 1
+        assert effects["n"] == 1, "the replacement session must not repeat an uncertain side effect"
     finally:
         _cleanup(mcp_tool, "srv-midcall")
 

tools/mcp_tool_handlers.py

@@ -33,6 +33,9 @@ _STDIO_NO_RESPAWN_MSG = (
 _STDIO_DIED_AGAIN_MSG = (
     "MCP server '{s}' respawned its stdio subprocess and it exited again immediately. The server is not starting "
     "cleanly — do NOT retry this tool; ask the user to check the server's command and its stderr log.")
+_STDIO_OUTCOME_UNCERTAIN_MSG = (
+    "MCP server '{s}' lost its stdio subprocess after the tool call began. The operation may have completed, so "
+    "Hermes did not replay it. Do NOT retry automatically; inspect the external state first.")
 
 
 def _trust_gate_check(server_name: str, tool_name: str) -> Optional[str]:
@@ -187,11 +190,19 @@ def _handle_session_expired_and_retry(server_name: str, exc: BaseException, retr
 class _StdioChildExited(RuntimeError):
     """Stdio subprocess gone when (or while) a call ran. Deliberately NOT a TimeoutError."""
 
+    def __init__(self, message: str, *, in_flight: bool):
+        super().__init__(message)
+        self.in_flight = in_flight
+
 
 def _handle_stdio_child_exited_and_retry(server_name: str, exc: Exception, retry_call, op_description: str):
-    """Respawn a dead stdio child and retry once; None if not our error. Never spawns itself: it
-    sets ``_reconnect_event`` and waits, so spawn frequency stays governed by ``run()``'s
-    rapid-drop budget. Single-shot: a child that dies again reports and stops.
+    """Respawn a dead stdio child; retry once only when it was dead before dispatch.
+
+    A mid-call exit is ambiguous: the server may have applied a side effect before its
+    response pipe disappeared. Reconnect for future calls but never replay that operation.
+    None means this is not our error. This function never spawns itself: it sets
+    ``_reconnect_event`` and waits, so spawn frequency stays governed by ``run()``'s rapid-drop
+    budget. A pre-dispatch retry whose child dies again reports and stops.
 
     Why retrying here cannot hot-cycle respawns: this function never spawns anything. It sets
     ``_reconnect_event`` (one signal, same as before) and waits for the server task to publish a fresh
@@ -203,13 +214,20 @@ def _handle_stdio_child_exited_and_retry(server_name: str, exc: Exception, retry
     reconnected = False
     srv = _lookup_reconnectable_server(server_name)
     if srv is not None:
-        logger.info("MCP server '%s': %s found the stdio subprocess dead (%s); respawning and retrying once.",
-                    server_name, op_description, exc)
+        action = "reconnecting without replay" if exc.in_flight else "respawning and retrying once"
+        logger.info("MCP server '%s': %s found the stdio subprocess dead (%s); %s.",
+                    server_name, op_description, exc, action)
         if _mcp_loop_running():
             reconnected = _loop._signal_reconnect_and_wait(
                 server_name, srv, op_description=op_description, timeout=_core._STDIO_RESPAWN_WAIT_SEC)
         else:  # No MCP loop to wait on (non-async adapters, tests): still request the respawn.
             _loop._signal_reconnect(srv)
+    if exc.in_flight:
+        return _strike(
+            server_name,
+            _STDIO_OUTCOME_UNCERTAIN_MSG.format(s=server_name),
+            outcome_uncertain=True,
+        )
     if not reconnected:
         return _strike(server_name, _STDIO_NO_RESPAWN_MSG.format(s=server_name, t=_core._STDIO_RESPAWN_WAIT_SEC))
     try:
@@ -283,12 +301,16 @@ async def _call_tool_racing_stdio_death(server, server_name: str, tool_name: str
     """``session.call_tool`` that fails fast when the stdio child is/gets dead: pre-call (a dead
     child must not hold the slot for the full timeout) and mid-call (race against
     ``_watch_stdio_children``). Both raise :class:`_StdioChildExited` for the respawn path, which
-    owns the reconnect signal. callable()/``is True`` because MagicMock attributes are truthy."""
+    owns the reconnect signal; only pre-call failure is safe to replay. callable()/``is True``
+    because MagicMock attributes are truthy."""
     # Fast-fail (#81995): a stdio subprocess that is already dead must not own this call slot — fail
     # immediately instead of waiting out the full tool timeout on a transport nobody will ever answer.
     _stdio_dead = getattr(server, "_stdio_children_dead", None)
     if callable(_stdio_dead) and _stdio_dead() is True:
-        raise _StdioChildExited(f"MCP stdio subprocess for '{server_name}' had already exited when the call was dispatched")
+        raise _StdioChildExited(
+            f"MCP stdio subprocess for '{server_name}' had already exited when the call was dispatched",
+            in_flight=False,
+        )
     _call_coro = server.session.call_tool(tool_name, arguments=args)
     _watch_children = getattr(server, "_watch_stdio_children", None)
     if not (inspect.iscoroutinefunction(_watch_children) and asyncio.iscoroutine(_call_coro)):
@@ -302,7 +324,10 @@ async def _call_tool_racing_stdio_death(server, server_name: str, tool_name: str
         done, _pending = await asyncio.wait({rpc_task, watch_task}, return_when=asyncio.FIRST_COMPLETED)
         if watch_task in done and not rpc_task.done():
             rpc_task.cancel()
-            raise _StdioChildExited(f"MCP stdio subprocess for '{server_name}' exited mid-call")
+            raise _StdioChildExited(
+                f"MCP stdio subprocess for '{server_name}' exited mid-call",
+                in_flight=True,
+            )
         return await rpc_task
     finally:
         watch_task.cancel()