ci: add cross-platform validation to ci.yaml

.github/workflows/ci.yaml

@@ -14,11 +14,23 @@ env:
 
 jobs:
   test:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     timeout-minutes: 30
     strategy:
+      fail-fast: false
       matrix:
+        os: [ubuntu-latest]
         test-type: [lint, core, interactive, integration, tools]
+        include:
+          - os: ubuntu-22.04
+            test-type: core
+          - os: ubuntu-24.04
+            test-type: core
+          - os: macos-14
+            test-type: core
+          - os: macos-14
+            test-type: host-pty
+            pytest-args: -x
 
     steps:
     - uses: actions/checkout@v4
@@ -45,6 +57,10 @@ jobs:
       if: matrix.test-type == 'tools'
       run: make test-tools
 
+    - name: Run host-PTY integration tests
+      if: matrix.test-type == 'host-pty'
+      run: uv run pytest tests/integration ${{ matrix.pytest-args || '' }}
+
   nightly:
     runs-on: ubuntu-latest
     timeout-minutes: 45

tests/integration/test_pty_integration.py

@@ -2,6 +2,8 @@
 
 import asyncio
 import os
+import select
+import sys
 from unittest.mock import patch
 
 import pytest
@@ -11,6 +13,98 @@
 from openroad_mcp.interactive.pty_handler import PTYHandler
 
 
+class _MacOSPTYHandler(PTYHandler):
+    """PTYHandler subclass for macOS.
+
+    Two macOS-specific problems combine to lose output from fast-exiting processes:
+
+    1. When all slave-fd holders close, the kernel discards the PTY master's read
+       buffer.  Fix: re-open the slave device as a "keepalive" fd immediately after
+       create_session so the buffer survives the child's exit.
+
+    2. process.wait() returns the instant the kernel notes the child's exit.  The
+       PTY line discipline may not have committed the child's final write to the
+       master's read buffer at that exact moment, so a bare non-blocking os.read()
+       races past the data.  Fix: drain via select() in a thread executor, which
+       blocks until the master fd is actually readable.
+    """
+
+    def __init__(self) -> None:
+        super().__init__()
+        self._cached_output = b""
+        self._slave_keepalive_fd: int | None = None
+
+    async def create_session(self, *args, **kwargs) -> None:
+        self._cached_output = b""
+        if self._slave_keepalive_fd is not None:
+            try:
+                os.close(self._slave_keepalive_fd)
+            except OSError:
+                pass
+            self._slave_keepalive_fd = None
+        await super().create_session(*args, **kwargs)
+        # Hold the slave device open so macOS doesn't discard the PTY master
+        # read buffer when the child closes its copy of the slave fd on exit.
+        if self.master_fd is not None:
+            try:
+                self._slave_keepalive_fd = os.open(
+                    os.ptsname(self.master_fd), os.O_RDWR | os.O_NOCTTY
+                )
+            except OSError:
+                pass
+
+    async def wait_for_exit(self, timeout: float | None = None) -> int | None:
+        result = await super().wait_for_exit(timeout=timeout)
+        # Only drain when the process actually exited (result is None on timeout).
+        if result is not None and self.master_fd is not None:
+            loop = asyncio.get_running_loop()
+            master_fd = self.master_fd
+            data = await loop.run_in_executor(None, _select_drain, master_fd)
+            if data:
+                self._cached_output += data
+        return result
+
+    async def read_output(self, size: int | None = None) -> bytes | None:
+        if self._cached_output:
+            data, self._cached_output = self._cached_output, b""
+            return data
+        return await super().read_output(size)
+
+    async def cleanup(self) -> None:
+        if self._slave_keepalive_fd is not None:
+            try:
+                os.close(self._slave_keepalive_fd)
+            except OSError:
+                pass
+            self._slave_keepalive_fd = None
+        await super().cleanup()
+
+
+def _select_drain(master_fd: int) -> bytes:
+    """Block via select() until *master_fd* is readable, then drain all data.
+
+    Runs in a thread executor so it does not stall the asyncio event loop.
+    The keepalive fd held by _MacOSPTYHandler prevents a HANGUP from firing
+    prematurely, so select() will only return once actual data is available
+    (or the 0.5 s safety timeout expires).
+    """
+    collected = b""
+    try:
+        r, _, _ = select.select([master_fd], [], [], 0.5)
+        while r:
+            try:
+                chunk = os.read(master_fd, 65536)
+            except (BlockingIOError, OSError):
+                break
+            if not chunk:
+                break
+            collected += chunk
+            r, _, _ = select.select([master_fd], [], [], 0.05)
+    except (OSError, ValueError):
+        pass
+    return collected
+
+
 def can_create_pty() -> bool:
     """Check if PTY creation is supported in current environment."""
     try:
@@ -40,7 +134,7 @@ def disable_command_validation(self):
     @pytest.fixture
     async def pty_handler(self):
         """Create and cleanup PTY handler."""
-        handler = PTYHandler()
+        handler = _MacOSPTYHandler() if sys.platform == "darwin" else PTYHandler()
         try:
             yield handler
         finally: