fix: probe tty fd pollability instead of platform-checking; add CI (Linux + macOS) - #251
Merged
Conversation
Background: PR #250 patched the macOS /dev/tty freeze by special-casing sys.platform == "darwin" to open os.ttyname(0) instead of /dev/tty. That fixes macOS but leaves the same defect for FreeBSD/OpenBSD/NetBSD, which also run on kqueue and don't match a hardcoded "darwin" string. The real defect: the function validated that the fd could be OPENED but never that it could be POLLED by the event loop's selector. Fix: add _fd_is_pollable(fd), which performs the exact selector.register() call loop.add_reader() will make later, using a throwaway selectors.DefaultSelector() (no running loop needed). Replace the darwin special-case with an ordered, platform-agnostic candidate list -- /dev/tty first (validated on Linux in production), then os.ttyname(0) as the kqueue-platform fallback -- and open the first candidate that BOTH opens successfully AND passes the pollability probe. No platform string is consulted anywhere in the decision. When the test seam (_TTY_DEVICE_PATH) is repointed, it stays the sole candidate (os.ttyname(0) is never appended), preserving existing seam-based test behavior. When every candidate is exhausted, the function logs a warning naming the candidates tried before returning None, so a degraded fallback announces itself rather than failing silently. No behavior change on the /dev/tty path when it's pollable (Linux): same device, same open() flags, same order. Tests (tests/test_dedicated_tty_input.py): added _fd_is_pollable unit tests (real pty slave -> True; a selector whose register() raises OSError -> False, with selector.close() still verified), a deterministic candidate-fallthrough test simulating the macOS scenario on any platform (monkeypatched os.open + _fd_is_pollable, verifies the winning fd is on os.ttyname(0) and the rejected /dev/tty candidate's fd was explicitly closed -- verified via a close() spy rather than fstat, since a just-closed fd number can be reused by the very next open()), an all-candidates-unpollable -> None + warning-log test (caplog), and a renamed/generalized repointed-seam test. Removed three darwin-specific tests whose assertions encoded the now-removed platform-check mechanism directly (they would fail unconditionally against the new candidate+probe design); their coverage is superseded by the new platform-agnostic tests. The darwin-gated real-kqueue integration test is unchanged. Full suite: 1261 passed, 15 pre-existing failures (confirmed identical with and without this change via git stash A/B comparison), 1 skipped (darwin-gated integration test, not applicable on Linux), 0 regressions. Also adds .github/workflows/ci.yml (the repo had no CI at all): matrix over ubuntu-latest/macos-latest x Python 3.11/3.12, uv sync + pytest. The macOS job now actually exercises the darwin-gated kqueue test instead of it being permanently unrunnable. The 15 pre-existing failures are excluded by explicit --deselect node IDs (not by file, and not via `|| true`) with a comment naming each file and its root cause, so a new regression in any of those files still fails the build. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Collaborator
Author
|
CI green on all four jobs — first CI run this repo has ever had.
Note the delta: macOS runs one more test than ubuntu and skips one fewer. That is |
This was referenced Aug 3, 2026
Merged
bkrabach
added a commit
that referenced
this pull request
Aug 3, 2026
…bound all waits (#254) The `pytest -m integration` job added in #251 and wired up in #253 hangs forever on the macOS runner. Root-caused on real macOS hardware to a test-harness bug (not a product bug): an un-drained pty master wedges the child on macOS. Both symptoms are harness bugs. The product is correct on macOS. Root cause: macOS wedges an exiting pty child whose output queue is never drained — not slowly, unreapably. A wedged child lands in ps state `?Es` (Exiting, session leader, controlling terminal already revoked). The fix introduces a shared pty harness (`tests/pty_harness.py`) that: - Drains the pty master via a dedicated thread on a dup() so the caller's master_fd keeps blocking-write semantics - Replaces parent-side sleeps with `wait_for_marker()` readiness handshake so sends land inside the child's live window - Bounds all `waitpid` calls — no blocking waits that can hang forever - Removes the un-drained-pty + SIGKILL condition that was a landmine in test_stdout_offload_freeze_integration.py:198 CI hardening: `timeout-minutes: 10` on both jobs. Typical runtime is 20-30s. A hung job now fails loudly in 10 minutes rather than burning runner hours. Verification: 5 consecutive integration runs on macOS, all green, no stray `?Es` children left behind. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Generalizes the macOS fix from #250 and closes the class of bug that let #247 ship broken.
open_dedicated_tty_input()validated that the tty fd could be opened, but never that it could be polled. On macOS the open succeeds andloop.add_reader()then fails withOSError(EINVAL)— deep inside prompt_toolkit, where it becomesEOFError(ptk >= 3.0.53, silent exit right after the banner) or propagates (<= 3.0.52, REPL error loop). Neither is a graceful fallback; both are a dead CLI.#250 fixed the macOS instance with a
sys.platform == "darwin"branch. That is a hardcoded list of known-bad combinations: FreeBSD/OpenBSD/NetBSD use the same kqueue backend and do not match"darwin".This replaces the platform check with a probe of the actual property that matters:
_fd_is_pollable(fd)registers/unregisters the fd on a throwawayselectors.DefaultSelector()— the same objectasyncio.SelectorEventLoopbuilds, and the sameregister(fd, EVENT_READ)calladd_reader()bottoms out in. Faithful proxy, no running loop required, two syscalls./dev/ttyfirst (the controlling-terminal alias, the device the Linux path is production-validated on), thenos.ttyname(0). First candidate that opens and probes pollable wins. Rejected candidates are closed immediately.None— prompt_toolkit's own default, which is the pre-fix: dedicate a non-blocking fd for terminal input to stop TTY-reader event-loop freezes #247 behavior and works. Degraded, visible, and alive rather than silently dead.sys.platformcheck remains except thewin32early-out (Windows has no/dev/ttyand no POSIXVt100Inputat all).Net effect: macOS and the BSDs self-heal via the probe, Linux keeps the exact device and flags it has today, and any future environment where the fd is openable-but-unpollable falls back to a working prompt instead of a dead one.
Verification — real hardware, both platforms
Driven through the real code path (
PromptSession(input=get_dedicated_tty_input()), mirroringmain.py:2772-2791) underpty.fork()so the pty is a genuine controlling terminal. macOS 26.6 (Darwin 25.6.0, arm64), Python 3.12.12, prompt_toolkit 3.0.52; Linux aarch64, same ptk./dev/tty'hello world'/dev/tty'hello world'/dev/tty'hello world'/dev/tty/dev/ttys008'hello world'/dev/ttys008'hello world'Candidate-loop behavior, instrumented on real hardware:
/dev/tty/dev/tty/dev/tty,/dev/ttys008/dev/tty/dev/ttys008/dev/tty,/dev/pts/215None→ ptk default/dev/tty,/dev/ttys008None→ ptk defaultThe last two rows are the point of the change: with every candidate rejected — simulating any future unknown breakage — the CLI still gets a working prompt. Under #247 that same condition was a dead REPL.
CI
This repo had no
.github/workflowsat all. That is the actual reason #247 reached users: 875 lines through the interactive input path, merged 39 minutes after opening, with## Known gap: not exercised in a live interactive sessionin its own body, and no automated gate on any platform.Adds
.github/workflows/ci.yml— matrix overubuntu-latest+macos-latest× Python 3.11/3.12,uv syncthenuv run pytest. The macOS job actually exercises the darwin-gated real-kqueue integration test that has never run anywhere.15 pre-existing failures (identical on
main@ 7063fde, unrelated to tty input: render/streaming, cleanup-event ordering, handler methods, provider commands, session lifecycle, subprocess config) are excluded by explicit per-test--deselectwith a named TODO — not--ignoreon whole files, not|| true. Every other test in those files still runs and a new regression there still fails the build.Tests
tests/test_dedicated_tty_input.py: probe true/false, candidate fallthrough (the macOS scenario reproduced deterministically on any platform), all-unpollable →None+ warning log, repointed test seam stays authoritative, and no-fd-leak assertions on every path.Same 15 node IDs before and after. 0 regressions.
Two darwin-specific unit tests from #250 were removed — they asserted the platform-gated mechanism this PR deletes (e.g. "
/dev/ttyis never attempted"). Their scenarios are covered by the new candidate-fallthrough and all-unpollable tests.Follow-up to #247 and #250. Thanks to @Joi for the correct macOS diagnosis — this generalizes it.