diff --git a/amplifier_app_cli/dedicated_tty_input.py b/amplifier_app_cli/dedicated_tty_input.py index 03a2e81..7b5856b 100644 --- a/amplifier_app_cli/dedicated_tty_input.py +++ b/amplifier_app_cli/dedicated_tty_input.py @@ -70,11 +70,39 @@ assumption ever stops holding, rather than silently reintroducing the freeze. +macOS DETAIL -- why the fd is opened on ``os.ttyname(0)`` there instead of +``/dev/tty``: on macOS, kqueue -- the backend behind asyncio's default +``KqueueSelector`` -- cannot poll the ``/dev/tty`` alias device. +``loop.add_reader()`` on such an fd raises ``OSError(EINVAL)`` at kevent +registration. What the user then sees depends on the installed +prompt_toolkit: 3.0.53+ catches that ``OSError`` in ``_attached_input()`` +(``prompt_toolkit/input/vt100.py``) and converts it to ``EOFError``, so +the REPL's first ``prompt_async()`` "sees Ctrl-D" and the CLI exits +silently right after the banner; on <=3.0.52 (the current lock), only +``PermissionError`` is caught, so the raw ``OSError`` propagates out of +``prompt_async()`` into the REPL's generic error handler, which prints +the error and retries -- failing identically every iteration. Either +way, interactive input is completely broken on macOS. The fallback below +cannot catch this: ``os.open("/dev/tty")`` itself succeeds; the failure +only surfaces later, at event-loop attach time, inside prompt_toolkit. +The underlying pty slave device (``os.ttyname(0)``, e.g. +``/dev/ttys003``) IS kqueue-pollable, so on darwin the fd is opened on +that path instead -- the same tty workaround libuv carries for macOS. +Note the darwin fd is therefore opened on *fd 0's terminal* (exactly the +device a competing reader races against) rather than the +controlling-terminal alias; for prompt input these coincide in practice, +and fd 0's terminal is the correct one to dedicate. Everything else +about the mechanism (fresh OFD, private ``O_NONBLOCK``, non-inheritable +fd, ``O_NOCTTY`` so a session leader can never accidentally acquire the +device as its controlling terminal) is identical for that path. + GRACEFUL FALLBACK: ``open_dedicated_tty_input()`` returns ``None`` -- never raises -- whenever the dedicated fd isn't available or applicable: -no controlling terminal, ``/dev/tty`` cannot be opened (CI, containers), -stdin is not a tty (piped input, non-interactive use), or Windows (no -``/dev/tty`` / POSIX ``Vt100Input`` concept there). Callers pass the +stdin is not a tty (piped input, CI, non-interactive use), the tty +device cannot be opened or resolved (containers; on non-darwin also a +missing controlling terminal -- on darwin fd 0's own terminal is opened, +which needs no controlling terminal), or Windows (no ``/dev/tty`` / +POSIX ``Vt100Input`` concept there). Callers pass the result straight through as ``PromptSession(input=...)``; ``None`` is exactly prompt_toolkit's own default, so this is a transparent, safe drop-in with zero UX change when the dedicated fd isn't available. @@ -92,10 +120,12 @@ if TYPE_CHECKING: from prompt_toolkit.input.base import Input -# Seam for tests: production code always opens the process's controlling -# terminal. Tests point this at a real pty slave's own path instead, since -# constructing a genuine controlling-terminal setup (setsid + TIOCSCTTY) -# isn't available inside a pytest worker. +# Seam for tests: in production this stays at "/dev/tty" (on darwin the +# open path is then resolved to fd 0's terminal via os.ttyname(0) -- see +# "macOS DETAIL" in the module docstring). Tests point this at a real pty +# slave's own path instead, since constructing a genuine +# controlling-terminal setup (setsid + TIOCSCTTY) isn't available inside +# a pytest worker; a repointed seam is always honored as-is. _TTY_DEVICE_PATH = "/dev/tty" __all__ = [ @@ -224,11 +254,32 @@ def open_dedicated_tty_input() -> DedicatedTtyInput | None: # Fall back rather than risk constructing something broken. return None + tty_path = _TTY_DEVICE_PATH + if sys.platform == "darwin" and tty_path == "/dev/tty": + # macOS kqueue (asyncio's default selector there) cannot poll the + # /dev/tty alias device: loop.add_reader() raises OSError(EINVAL) + # at attach time, which breaks the REPL's first prompt (silent + # instant exit or an error loop, depending on the installed + # prompt_toolkit -- see "macOS DETAIL" in the module docstring). + # The underlying slave device (os.ttyname(0)) IS kqueue-pollable, + # so open that instead. The == "/dev/tty" guard keeps the + # _TTY_DEVICE_PATH test seam authoritative when repointed. + try: + tty_path = os.ttyname(0) + except OSError: + # fd 0 is a tty (checked above) but its name can't be + # resolved -- fall back to the default rather than open an + # alias device the event loop cannot poll. + return None + try: - fd = os.open(_TTY_DEVICE_PATH, os.O_RDONLY | os.O_NONBLOCK) + # O_NOCTTY: opening a named terminal device from a session leader + # without a controlling terminal would otherwise ACQUIRE it as the + # controlling terminal (libuv opens tty fds the same way). + fd = os.open(tty_path, os.O_RDONLY | os.O_NONBLOCK | os.O_NOCTTY) except OSError: # No controlling terminal (containers, detached processes) or - # /dev/tty otherwise unopenable -- fall back to the default. + # tty device otherwise unopenable -- fall back to the default. return None try: diff --git a/tests/test_dedicated_tty_input.py b/tests/test_dedicated_tty_input.py index aa45181..f8c95ac 100644 --- a/tests/test_dedicated_tty_input.py +++ b/tests/test_dedicated_tty_input.py @@ -207,6 +207,167 @@ def test_open_dedicated_tty_input_returns_none_on_windows(monkeypatch): assert dti.open_dedicated_tty_input() is None +# --------------------------------------------------------------------------- +# macOS: kqueue cannot poll the /dev/tty alias device -- the fd must be +# opened on the underlying slave device (os.ttyname(0)) instead, or +# loop.add_reader() raises OSError(EINVAL) at attach time and interactive +# input is completely broken (silent instant exit on prompt_toolkit +# >=3.0.53, which converts the OSError to EOFError; an error loop on +# <=3.0.52, which lets it propagate). See "macOS DETAIL" in the module +# docstring. +# --------------------------------------------------------------------------- + + +@pytest.mark.skipif(sys.platform != "darwin", reason="exercises real macOS kqueue") +@pytest.mark.asyncio +async def test_darwin_dedicated_input_attaches_to_real_kqueue_loop(): + """End-to-end regression test on real darwin, real event loop, seam at + its production default: the dedicated input must register with the + actual KqueueSelector loop and deliver bytes. + + On the unfixed code this bites in both environments: with a + controlling terminal (developer machine) ``/dev/tty`` opens but + ``attach()`` raises ``OSError(EINVAL)`` from kevent registration; + without one (CI) ``/dev/tty`` cannot be opened at all so the handle + is ``None`` and the assertion below fails. The fixed code resolves + fd 0's slave device, which kqueue polls fine in both. + """ + import asyncio + + master_fd, slave_fd = pty.openpty() + saved_stdin_fd = os.dup(0) + os.dup2(slave_fd, 0) + try: + handle = dti.open_dedicated_tty_input() + assert handle is not None, ( + "with a pty on fd 0, darwin must produce a dedicated input " + "even without a controlling terminal" + ) + try: + loop = asyncio.get_running_loop() + got_keys: asyncio.Future = loop.create_future() + + def _on_ready() -> None: + keys = handle.input.read_keys() + if keys and not got_keys.done(): + got_keys.set_result(keys) + + # attach() is where the unfixed code explodes on macOS: + # loop.add_reader() -> kqueue kevent registration -> EINVAL. + # raw_mode() mirrors production (a fresh pty is canonical, so + # a lone byte would otherwise sit unreadable until newline). + with handle.input.raw_mode(), handle.input.attach(_on_ready): + os.write(master_fd, b"x") + keys = await asyncio.wait_for(got_keys, timeout=5) + + assert keys[0].data == "x" + finally: + handle.close() + finally: + os.dup2(saved_stdin_fd, 0) + os.close(saved_stdin_fd) + os.close(slave_fd) + os.close(master_fd) + + +def test_darwin_opens_stdin_slave_device_not_devtty_alias(monkeypatch): + """On darwin with the production seam ("/dev/tty"), the dedicated fd + must be opened on ``os.ttyname(0)`` -- the kqueue-pollable slave + device -- never on the ``/dev/tty`` alias itself. + + Runs on any POSIX platform: darwin is forced via the same + ``dti.sys.platform`` monkeypatch the Windows fallback test uses. + """ + master_fd, slave_fd = pty.openpty() + slave_path = os.ttyname(slave_fd) + + saved_stdin_fd = os.dup(0) + os.dup2(slave_fd, 0) + try: + monkeypatch.setattr(dti.sys, "platform", "darwin") + monkeypatch.setattr(dti, "_TTY_DEVICE_PATH", "/dev/tty") + + opened: list[tuple[str, int]] = [] + real_open = os.open + + def _spy_open(path, flags, *args, **kwargs): + opened.append((path, flags)) + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(dti.os, "open", _spy_open) + + handle = dti.open_dedicated_tty_input() + assert handle is not None + try: + assert [path for path, _ in opened] == [slave_path], ( + "darwin must open the stdin slave device (os.ttyname(0)), " + "not the /dev/tty alias kqueue cannot poll" + ) + assert opened[0][1] & os.O_NOCTTY, ( + "the device must be opened with O_NOCTTY so a session " + "leader can never accidentally acquire it as its " + "controlling terminal" + ) + assert _is_nonblocking(handle.input.fileno()) is True + finally: + handle.close() + finally: + os.dup2(saved_stdin_fd, 0) + os.close(saved_stdin_fd) + os.close(slave_fd) + os.close(master_fd) + + +def test_darwin_falls_back_to_none_when_ttyname_unresolvable(monkeypatch): + """darwin + stdin is a tty, but ``os.ttyname(0)`` fails: must fall + back to ``None`` rather than open the unpollable ``/dev/tty`` alias + (which would reintroduce the broken event-loop attach). + """ + monkeypatch.setattr(dti.sys, "platform", "darwin") + monkeypatch.setattr(dti, "_TTY_DEVICE_PATH", "/dev/tty") + monkeypatch.setattr(dti.os, "isatty", lambda fd: True) + + def _raise_ttyname(fd): + raise OSError("simulated: ttyname unresolvable") + + monkeypatch.setattr(dti.os, "ttyname", _raise_ttyname) + assert dti.open_dedicated_tty_input() is None + + +def test_darwin_respects_repointed_test_seam(monkeypatch): + """When ``_TTY_DEVICE_PATH`` is repointed away from its production + default (the documented test seam), darwin must open exactly the seam + path and must NOT override it with ``os.ttyname(0)`` -- otherwise + every seam-based test in this file would silently test the wrong + device. + """ + master_a, slave_a = pty.openpty() + master_b, slave_b = pty.openpty() + seam_path = os.ttyname(slave_b) + + saved_stdin_fd = os.dup(0) + os.dup2(slave_a, 0) # fd 0 is pty A; the seam points at pty B + try: + monkeypatch.setattr(dti.sys, "platform", "darwin") + monkeypatch.setattr(dti, "_TTY_DEVICE_PATH", seam_path) + + handle = dti.open_dedicated_tty_input() + assert handle is not None + try: + assert os.ttyname(handle.input.fileno()) == seam_path, ( + "the repointed seam must stay authoritative on darwin" + ) + finally: + handle.close() + finally: + os.dup2(saved_stdin_fd, 0) + os.close(saved_stdin_fd) + os.close(slave_a) + os.close(master_a) + os.close(slave_b) + os.close(master_b) + + # --------------------------------------------------------------------------- # Fail-loud guard -- prompt_toolkit internals this fix depends on. # ---------------------------------------------------------------------------