fix(render-mcp): close LOVR launch context on respawn (fixes #196)

agent-mcp-servers/render-mcp/render_mcp/__main__.py

@@ -177,6 +177,15 @@ def __init__(self, cfg: Config, stack: contextlib.AsyncExitStack) -> None:
         self._render_drops: int = 0
         self._spawn_error: str | None = None
         self._watch_task: asyncio.Task | None = None
+        # Per-launch context for the current LOVR child. Each launch gets its
+        # own AsyncExitStack so its ManagedProcess teardown (pipe-task cancel +
+        # log-sink close) can run on respawn instead of piling up in the
+        # app-lifetime stack until whole-process shutdown (issue #196).
+        self._launch_stack: contextlib.AsyncExitStack | None = None
+        # Safety net: close whatever launch context is live when the server
+        # shuts down (LOVR still running). Registered once on the app-lifetime
+        # stack so it never accumulates per launch.
+        self._stack.push_async_callback(self._aclose_live_launch)
 
         # Scene state: { id → { type, position, color, scale } }
         self._objects: dict[str, dict] = {}
@@ -216,16 +225,26 @@ async def start_lovr_once(self) -> dict:
             logger.info(
                 "render-mcp: starting LOVR  bin={}  app={}", cfg.lovr_bin, cfg.xr_app_dir,
             )
-            lovr_proc = await self._stack.enter_async_context(
+            launch_stack = contextlib.AsyncExitStack()
+            lovr_proc = await launch_stack.enter_async_context(
                 ManagedProcess("lovr", lovr_cmd, cwd=cfg.xr_app_dir)
             )
+            self._launch_stack = launch_stack
 
             async def _watch() -> None:
                 rc = await lovr_proc.wait()
                 logger.warning(
                     "render-mcp: LOVR child exited (rc={}) — "
                     "resetting lovr_started so next start_xr respawns it", rc,
                 )
+                # Tear down this launch's context (cancel the two pipe-forward
+                # tasks, close the log sink; terminate is a no-op since the
+                # child already exited) BEFORE allowing a respawn, so dead
+                # contexts don't accumulate in the app-lifetime stack (#196).
+                with contextlib.suppress(Exception):
+                    await launch_stack.aclose()
+                if self._launch_stack is launch_stack:
+                    self._launch_stack = None
                 self._lovr_started = False
 
             self._watch_task = asyncio.create_task(_watch(), name="lovr-watch")
@@ -326,6 +345,18 @@ async def forward(self, op: str, value: Any) -> dict:
         except zmq.Again:
             return {"ok": False, "reason": "backpressure"}
 
+    async def _aclose_live_launch(self) -> None:
+        """Close the current launch context, if any.
+
+        Registered once on the app-lifetime stack so a LOVR child still running
+        at server shutdown is torn down. On a normal LOVR exit ``_watch`` has
+        already closed and cleared the launch stack, so this is a no-op.
+        """
+        launch_stack, self._launch_stack = self._launch_stack, None
+        if launch_stack is not None:
+            with contextlib.suppress(Exception):
+                await launch_stack.aclose()
+
     def close(self) -> None:
         if self._watch_task is not None and not self._watch_task.done():
             self._watch_task.cancel()

docs/changelog.md

@@ -9,6 +9,17 @@ Significant decisions, in reverse-chronological order. Update this whenever a
 non-trivial architectural or design decision is made so the rationale is
 preserved and not re-litigated.
 
+### 2026-06-09 — render-mcp: per-launch context for LOVR respawn (no leaked pipe tasks / log handles)
+
+`render-mcp` parked every LOVR launch's `ManagedProcess` in the
+process-lifetime `AsyncExitStack`, so on each respawn the previous context's
+teardown (its two `_forward` pipe tasks + open log-file handle) never ran until
+whole-process shutdown — N restarts leaked N-1 dead contexts. Each launch now
+gets its own `AsyncExitStack`, closed inside `_watch` as soon as the child
+exits (before a respawn is allowed); a single `_aclose_live_launch` callback on
+the app-lifetime stack covers the shutdown-while-LOVR-running case, so it never
+accumulates per launch. Fixes #196.
+
 ### 2026-06-09 — Ctrl-C during startup tears down everything, incl. persist + docker containers
 
 Pressing Ctrl-C while `model-servers` was launching (slow image pull / weight

tests/test_local_render_mcp.py

@@ -312,23 +312,31 @@ async def test_get_health_and_scene_state(self, dispatcher):
 
 
 class _FakeLovrProc:
-    """Stand-in for ManagedProcess that never exits until cancelled."""
+    """Stand-in for ManagedProcess that exits only when told (or never)."""
     def __init__(self) -> None:
         self._done = asyncio.Event()
 
     async def wait(self) -> int:
         await self._done.wait()
         return 0
 
+    def trigger_exit(self) -> None:
+        """Simulate the LOVR child exiting, unblocking ``wait()``."""
+        self._done.set()
+
 
 class _FakeManagedProcessCtx:
     def __init__(self) -> None:
         self.proc = _FakeLovrProc()
+        # Records whether this launch's context was torn down — the leak in
+        # issue #196 is exactly that this never ran until process shutdown.
+        self.exited = False
 
     async def __aenter__(self) -> _FakeLovrProc:
         return self.proc
 
     async def __aexit__(self, *exc) -> None:
+        self.exited = True
         return None
 
 
@@ -372,3 +380,77 @@ async def test_close_cancels_lovr_watch_task(tmp_path: Path, monkeypatch):
         assert disp._watch_task.done()
     finally:
         await stack.__aexit__(None, None, None)
+
+
+@_asyncio
+async def test_lovr_respawn_closes_previous_launch_context(tmp_path: Path, monkeypatch):
+    """Issue #196: each LOVR launch's ManagedProcess context must be torn down
+    on respawn instead of accumulating in the app-lifetime stack.
+
+    Without the fix the previous context's ``__aexit__`` (pipe-task cancel +
+    log-sink close) only runs at whole-process shutdown, so N restarts leak
+    N-1 contexts. With the fix, ``_watch`` closes the per-launch stack as soon
+    as the child exits, before the next ``start_lovr_once``.
+    """
+    sock_path = _unique_ipc(tmp_path)
+    lovr_bin  = tmp_path / "lovr.sh"
+    lovr_bin.write_text("#!/bin/sh\nsleep 999\n")
+    lovr_bin.chmod(0o755)
+    xr_app_dir = tmp_path / "xr_app"
+    xr_app_dir.mkdir()
+
+    cfg = Config(
+        lovr_bin         = lovr_bin,
+        xr_app_dir       = xr_app_dir,
+        scene_socket     = sock_path,
+        cloudxr_env_file = None,
+        host             = "127.0.0.1",
+        port             = 0,
+    )
+
+    created: list[_FakeManagedProcessCtx] = []
+
+    def _make_ctx(*_a, **_kw) -> _FakeManagedProcessCtx:
+        ctx = _FakeManagedProcessCtx()
+        created.append(ctx)
+        return ctx
+
+    monkeypatch.setattr(render_main, "ManagedProcess", _make_ctx)
+
+    stack = contextlib.AsyncExitStack()
+    await stack.__aenter__()
+    try:
+        disp = SceneDispatcher(cfg, stack)
+
+        # First launch.
+        assert await disp.start_lovr_once() == {"status": "started"}
+        assert len(created) == 1
+        assert disp._launch_stack is not None
+        assert created[0].exited is False  # live
+
+        # Simulate the LOVR child exiting and let _watch run to completion.
+        created[0].proc.trigger_exit()
+        await disp._watch_task
+
+        # The previous launch context is torn down on respawn — not leaked.
+        assert created[0].exited is True
+        assert disp._launch_stack is None
+        assert disp._lovr_started is False
+
+        # Second launch reuses a fresh context; the first stays closed (no
+        # accumulation), the second is live.
+        assert await disp.start_lovr_once() == {"status": "started"}
+        assert len(created) == 2
+        assert created[0].exited is True
+        assert created[1].exited is False
+        assert sum(c.exited for c in created) == 1  # only the dead one closed
+
+        disp.close()
+        with contextlib.suppress(asyncio.CancelledError):
+            await disp._watch_task
+    finally:
+        await stack.__aexit__(None, None, None)
+
+    # Safety net: the still-live second context is closed when the
+    # app-lifetime stack unwinds at shutdown.
+    assert created[1].exited is True