Skip to content

Commit 3896f5a

Browse files
Technologicatclaude
andcommitted
net.ptyproxy: enable Windows CI, fix \n→\r\n translation, CHANGELOG
Third and final commit for the Windows port of `unpythonic.net`. This one enables the integration test suite on Windows CI (which was already in the matrix but had been gated off by an early-return in `test_client.py:runtests`), fixes one real bug that only manifests on Windows, and updates the docs. Windows-only bug fix in `ptyproxy_windows.open_slave_streams`: `socket.makefile("w")` creates a `TextIOWrapper` with default `newline=None`. Per Python docs, this translates every `\n` the application writes into `os.linesep` — which is `\n` on POSIX (so the POSIX backend and the Linux smoke test of the Windows backend never saw the issue) but `\r\n` on Windows. The consequence on real Windows: the server's prompts, banner, and eval results would arrive at the client with `\r\n` instead of the `\n` the session ID parser and prompt detector expect. The regex `r"session (\d+) connected"` would still match (it doesn't anchor on `\n`), but the client's display would fill with stray `\r`s and the downstream text handling could break in subtle ways. Fix: pass `newline=""` on the write side, which disables the translation. On POSIX this is a no-op (since `os.linesep == "\n"`), so the Linux test suite validates the same code path that Windows runs — no divergence, no "works on POSIX but might break on Windows" risk for this specific issue. The read side stays at default `newline=None` (universal newlines), which gives `code.InteractiveConsole` the `\n`-terminated lines it expects from `sys.stdin.readline()`. Test suite: Windows early-return removed. `test_client.py:runtests` previously gated the entire integration test suite (full client ↔ server roundtrip, netcat mode, control- channel RPC, sequential reconnect, concurrent clients) behind a `platform.system() == "Windows"` early-return. With the Windows backend in place, that gate is gone — the 18 integration tests now run on every platform in the CI matrix (Linux, macOS, Windows × 3.14, pypy-3.11). The `tier 1: Windows backend via server (POSIX-only smoke)` testset, which force-runs the Windows backend on POSIX as a pre-CI validation, is wrapped in `if platform.system() != "Windows":` — redundant on Windows, where the native backend already *is* `WindowsPTYSocketProxy`. The cleanup-contract testset is unchanged: it runs on every platform and exercises both backends explicitly (POSIX backend gated to POSIX, Windows backend always, plus a dispatch-correctness check). Doc/reference hygiene: TODO_DEFERRED item codes are ephemeral — they get removed when the item is resolved, so referring to them in permanent source comments creates dangling references. Stripped four such references: * `ptyproxy_windows.py` module docstring: "D9 design discussion" → inline the rationale. * `test_client.py` module docstring: "POSIX-only (tracked as TODO_DEFERRED D9)" + "Tier 2 deferred as TODO_DEFERRED D10" → drop the item codes; keep the content. * `client.py` readline-fallback comment: "eventual Windows port tracked as TODO_DEFERRED D9" → "for Windows support". TODO_DEFERRED.md: * D9 (Windows port) removed. * D10 (tier 2 REPL tests) amended: its Windows note used to say "depends on D9 landing first" — replaced with a note that the Windows port happened via `socket.socketpair`, so any tier-2 Windows variant (if we ever build one) has to do its own ConPTY design work, independent of what D9 delivered. * D12 added: `unpythonic.net.util.ReceiveBuffer` lacks direct unit tests. It's used internally by `MessageDecoder` (exercised transitively) and externally by `raven.common.netutil. multipart_x_mixed_replace_payload_extractor`, so as a public API symbol it deserves targeted coverage. Noticed during this port. * Next unused code bumped to D13. CHANGELOG.md (2.0.1 in-progress): * New "Changed" subsection with the Windows-support entry and the `isatty()` caveat for REPL-internal user code. * Existing "Fixed" subsection: added the idempotent-`stop()` latent-bug entry. * Existing "Internal" subsection: updated the test-suite entry ("POSIX-only; Windows port deferred" → "Runs on every CI platform (Linux, macOS, Windows)"), and added an entry describing the ptyproxy refactor (ABC + platform dispatch + context manager). Total test count on Linux: 40 assertions (21 cleanup contract, 1 force-Windows-backend smoke, 18 integration), all green. Windows CI will be the real verification for the line-ending fix and the integration path through `WindowsPTYSocketProxy` — if anything else is Windows-specific and broken, the CI failure will tell us. Completes D9. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f1dfe3a commit 3896f5a

5 files changed

Lines changed: 73 additions & 73 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,22 @@
22

33
**2.0.1** (in progress):
44

5+
**Changed**:
6+
7+
- `unpythonic.net` (REPL server and client) now runs on MS Windows. Previously the whole subsystem was POSIX-only because `unpythonic.net.ptyproxy` required `termios`, `tty`, and `os.openpty`. A new `socket.socketpair()`-based backend stands in for the pty master/slave endpoints on Windows, plugged in via a platform dispatch in `PTYSocketProxy`. Known wart: `os.isatty(sys.stdin.fileno())` inside a REPL session returns `False` on Windows (no real pseudo-terminal is involved), whereas it returns `True` on POSIX — user code *inside* the REPL that checks `sys.stdin.isatty()` will see the Windows result; the framework itself doesn't care.
8+
59
**Fixed**:
610

711
- `unpythonic.net.server`: `start()` now returns the actually-bound ports, not the values the caller passed in. Matters when passing `repl_port=0` / `control_port=0` to let the kernel pick a free port — previously the caller got `(bind, 0, 0)` back. Also fixes a latent bug in `unpythonic.net.util.ReuseAddrThreadingTCPServer`: its custom `server_bind()` override dropped the `self.server_address = self.socket.getsockname()` refresh from stdlib's `TCPServer.server_bind`.
12+
- `unpythonic.net.ptyproxy`: `stop()` is now idempotent and safe to call on a proxy that was never started. Latent bug: previously `stop()` gated the entire teardown (including `os.close(master)` / `os.close(slave)`) behind `if self._thread:`, so constructing a proxy and then exiting without calling `start()` leaked both fds.
813
- `unpythonic.net.client`: tab completion now works on macOS. macOS ships `readline` backed by `libedit`, which speaks a different `parse_and_bind` dialect than GNU readline — the client now detects `platform.system() == "Darwin"` and issues the libedit form there.
914
- `unpythonic.misc.timer` and `unpythonic.timeutil.ETAEstimator`: switched from `time.monotonic()` to `time.perf_counter()`. Latent Windows-only bug: `monotonic` is backed by a ~16 ms tick counter on Windows, so microsecond-scale `with timer() as t: ...` blocks recorded `t.dt = 0.0` and downstream divisions raised `ZeroDivisionError`. `perf_counter` has the highest available resolution on every platform. POSIX unaffected.
1015
- `unpythonic.test.runner`: module discovery no longer crashes on MS Windows with `re.error: bad escape`. The runner used `re.sub(os.path.sep, ...)``os.path.sep` is a lone backslash on Windows, an invalid regex pattern. Fixed by using `str.replace`. Affects any project reusing `unpythonic.test.runner`.
1116

1217
**Internal**:
1318

14-
- `unpythonic.net` now has an automated test suite for the REPL client and server. `unpythonic/net/tests/test_client.py` exercises the full client ↔ server roundtrip in-process (eval, multi-line, syntax-error recovery, clean disconnect), netcat-mode raw-socket access, control-channel RPC, and stretch cases (sequential reconnect, two concurrent clients). Tier 1 only — no subprocess / pty driver. POSIX-only; Windows port deferred.
19+
- `unpythonic.net` now has an automated test suite for the REPL client and server. `unpythonic/net/tests/test_client.py` exercises the full client ↔ server roundtrip in-process (eval, multi-line, syntax-error recovery, clean disconnect), netcat-mode raw-socket access, control-channel RPC, and stretch cases (sequential reconnect, two concurrent clients). Tier 1 only — no subprocess / pty driver. Runs on every CI platform (Linux, macOS, Windows).
20+
- `unpythonic.net.ptyproxy`: refactored into an abstract base class with platform-specific backends (`PosixPTYSocketProxy` via `os.openpty`, `WindowsPTYSocketProxy` via `socket.socketpair`). Dispatch happens inside `PTYSocketProxy.__new__`, so callers instantiate the base class and get the right backend for free. `PTYSocketProxy` is now a context manager (`with PTYSocketProxy(...) as proxy:`) for guaranteed cleanup. Public interface otherwise unchanged.
1521
- `unpythonic.net.client`: `connect()` is now a thin public shim over a private `_connect(..., _input=None)`. The `_input` seam lets tests drive the REPL loop without monkey-patching `builtins.input` globally. Public API unchanged.
1622
- `unpythonic.net.client`: `import readline` moved from module top into `connect()`, with a three-tier fallback (`readline``pyreadline3` → graceful degradation). POSIX behaviour unchanged; the module is now importable on Windows.
1723
- `unpythonic.net.tests.fixtures.nettest`: binds on port 0 (kernel-assigned), removed a `sleep(0.05)` race-condition bandage, and re-raises worker-thread exceptions instead of swallowing them.

TODO_DEFERRED.md

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,13 @@
11
# Deferred Issues
22

3-
Next unused item code: D12
3+
Next unused item code: D13
44

55
- **D5**: `dispatch.py` — moved to GitHub issue #99. Dispatch-layer improvements for parametric ABCs (warn/error on indistinguishable multimethods). Typecheck-layer part resolved.
66

77

88
- **D8: Audit typing: abstract parameter types, concrete return types**: Parameters should use abstract types from `collections.abc` (`Mapping`, `Sequence`, `Iterable`) for widest-possible-accepted semantics. Return types should use concrete lowercase builtins (`tuple[int, int]`, `list[int]`, `dict[str, int]`) — PEP 585, Python 3.9+. The capitalized `typing` forms (`Dict`, `List`, `Tuple`) are deprecated aliases for the builtins and offer no extra width — avoid them. Audit existing type hints across the codebase for consistency. (Discovered during raven-cherrypick compare mode planning, 2026-03-30.)
99

1010

11-
- **D9: Port `unpythonic.net` (REPL server/client) to MS Windows**: The remote-REPL subsystem (`unpythonic.net.server`, `unpythonic.net.client`, `unpythonic.net.ptyproxy`) is currently documented as POSIX-only (see 2.0.0 CHANGELOG). The blockers are in `ptyproxy.py`, which uses `termios`, `fcntl`, `pty`, and `select` to create a pseudoterminal pair for the server-side `code.InteractiveConsole` to read/write through. None of those modules exist on Windows.
12-
13-
**Rough list of Windows equivalents**, in order of plausibility:
14-
15-
- **`pywinpty`** (third-party, ~active maintenance) — the preferred option. Wraps the Windows **Pseudo Console API** (ConPTY, introduced in Windows 10 1809, October 2018). Used by JupyterLab's terminal and by `xterm.js`-based backends. Provides a pty-like interface that `ptyproxy.py` could consume with a thin adapter. Would need to become a Windows-only optional dependency of unpythonic.
16-
- **`msvcrt`** (stdlib) — low-level Windows console I/O. NOT a pty equivalent: it's just console-focused character access (`kbhit`, `getch`, `getwch`, etc.). Probably not useful on its own for this purpose — it doesn't give you the "line discipline + bidirectional pipe" semantics that `pty` provides on POSIX.
17-
- **`winpty`** (pre-ConPTY, C-level library, third-party) — the older Cygwin-era solution. ConPTY supersedes it and `pywinpty` can use ConPTY directly on recent Windows. Only relevant if we need to support Windows versions before 10-1809, which we don't.
18-
- **`ptyprocess` / `pexpect`** — pexpect has a Windows backend, but it uses `wexpect` under the hood and has historically been finicky. Not currently recommended as the primary approach, but could serve as a higher-level wrapper *on top of* pywinpty.
19-
- **Raw Windows API via `ctypes`**`CreatePseudoConsole()`, `ResizePseudoConsole()`, `ClosePseudoConsole()` can be called directly through `ctypes` if we want to avoid the `pywinpty` dependency. More work; smaller dep footprint. Decision to make at design time.
20-
21-
**Likely decomposition** of the work:
22-
23-
1. Split `ptyproxy.py` into a platform-dispatch wrapper that imports either a `ptyproxy_posix` submodule (current code) or a new `ptyproxy_windows` submodule. Both expose the same `PTYSocketProxy` class.
24-
2. Implement `ptyproxy_windows` using `pywinpty` — or, if that turns out to be heavy for an optional dep, using direct `ctypes` calls to ConPTY. ConPTY's semantics differ from Unix pty in subtle ways (output buffering, line-discipline equivalent, terminal-resize signalling), so this will need careful testing against the existing unit/integration tests once those exist (see the interactive-REPL testing strategy designed in this same session).
25-
3. Make `pywinpty` (or whatever is chosen) a Windows-only optional dep via `[project.optional-dependencies]` — e.g. `windows = ["pywinpty>=2.0"]` — with a helpful ImportError message if someone tries to use `unpythonic.net.{server,client}` on Windows without it installed.
26-
4. Add Windows cells to `unpythonic.net`'s test matrix (which depends on tests existing in the first place — currently `unpythonic.net` has no tests, also covered by the testing-strategy discussion this session).
27-
5. Update the 2.0.0 CHANGELOG entry that currently documents `unpythonic.net` as POSIX-only once this lands.
28-
29-
**Why deferred**: this is a non-trivial port that probably deserves its own design session — at minimum a careful read of pywinpty's API surface, a mapping of pty primitives to their ConPTY equivalents, and a plan for how to test the result without having a Windows dev machine. User currently has only Linux dev boxes; any debugging would be entirely via CI, which is feasible but slow-iterating.
30-
31-
**Related**: the `parse_and_bind` Darwin-branch fix in `net/client.py` (2026-04-15) was a prerequisite refactor — `net/client.py` is now in the right shape for the Windows port to plug into. Also, the three-tier hybrid readline fallback pattern documented in `raven.librarian.minichat` and `mcpyrate.repl.macropython` (same session) is directly reusable for `net/client.py` once `net/client.py`'s top-level `import readline` is moved inside the client function and guarded.
32-
33-
(Added 2026-04-15, based on audit + discussion during the Windows-CI expansion session.)
34-
35-
3611
- **D10: Tier 2 REPL tests (subprocess + pty) for `unpythonic.net` client/server**: Tier 1 coverage for `unpythonic.net.client` and `unpythonic.net.server` uses a server-in-thread + in-process client pattern (see `unpythonic/net/tests/`) with scripted input via `builtins.input` monkey-patch and captured stdout/stderr via `io.StringIO`. Fast, single-process, no subprocess boundary needed — the server speaks TCP to `127.0.0.1` and the client loop runs in the same pytest process. **We might never need tier 2.**
3712

3813
A second tier would spawn the server and client as real subprocesses, with each end driven through a pseudo-terminal (`pexpect` / `ptyprocess`), to catch things tier 1 cannot reach:
@@ -44,7 +19,7 @@ Next unused item code: D12
4419

4520
Cost:
4621
- ~0.5–1 s startup per test × two processes per test (client + server) = ~1–2 s per test. Matters for suite size.
47-
- POSIX-only naturally. Windows support depends on D9 (port `unpythonic.net` to MS Windows) landing first — no point designing tier 2 for a subsystem that doesn't run on Windows yet. If/when D9 lands, Windows tier 2 can use the same ConPTY backend that D9 introduces.
22+
- POSIX-only naturally. Since D9 landed (2026-04-16), `unpythonic.net` runs on Windows too via `socket.socketpair`, but tier 2 still needs real pseudo-terminals — on Windows that means ConPTY, which D9 deliberately avoided (see the D9 discussion). If tier 2 ever materializes, its Windows variant is an independent design problem.
4823
- `pexpect` would become a new dev dep. Small but non-zero.
4924

5025
**Rough shape if we ever do it:**
@@ -64,3 +39,6 @@ Next unused item code: D12
6439
**When to actually do it**: only if tier 1 coverage turns out to miss something important (a regression hits prod that tier 1 would not have caught). The in-thread server + scripted client approach already exercises most of the protocol surface; tier 2 is primarily a safety net for terminal-semantics and signal-path bugs. Until one of those bites, tier 1 is the main win. (Added 2026-04-15, alongside the tier 1 bring-up.)
6540

6641

42+
- **D12: Unit tests for `unpythonic.net.util.ReceiveBuffer`**: The `ReceiveBuffer` class in `unpythonic/net/util.py` is used internally by `unpythonic.net.msg.MessageDecoder` (exercised transitively by the REPL test suite), but it also has a real external consumer in production — `raven.common.netutil.multipart_x_mixed_replace_payload_extractor`, which reuses it as a general-purpose append/set/getvalue buffer for message-boundary-aware reads. As part of the public `unpythonic.net.util` API (`__all__`), it deserves its own targeted unit tests rather than only being covered through `MessageDecoder`'s indirect use. Cheap to add — a new test module in `unpythonic/net/tests/` exercising `append`, `set`, `getvalue`, and the type-check error paths. (Noticed 2026-04-16 during the D9 Windows port.)
43+
44+

unpythonic/net/client.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@
4747
# this whole module unimportable on Windows — even though `connect()`
4848
# is the only thing that needs it. Keeping the import inside the
4949
# function lets callers on non-POSIX platforms at least import the
50-
# module (useful for test collection, docs, and for the eventual
51-
# Windows port tracked as TODO_DEFERRED D9).
50+
# module (useful for test collection, docs, and for Windows support).
5251
#
5352
# 2. A three-tier fallback is applied at the import site: stdlib
5453
# `readline` first, then third-party `pyreadline3` (a Windows drop-in

unpythonic/net/ptyproxy_windows.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,12 @@
2121
primitive, and spawning a subprocess per REPL session would defeat the
2222
whole point of `unpythonic.net.server`, which is to let a remote client
2323
inspect and hot-patch state in the *host* Python process — that requires
24-
the REPL to run in the same process as the server. Rationale recorded in
25-
the D9 design discussion (2026-04-16).
24+
the REPL to run in the same process as the server.
25+
26+
The right question isn't "how do we get ConPTY", it's "what do we
27+
actually need". The answer: two connected bidirectional byte streams.
28+
`socket.socketpair()` provides exactly that, stdlib-only, with lines
29+
of code that mirror the POSIX backend almost 1:1.
2630
2731
**Why the Windows backend also works on POSIX**: `socket.socketpair()` is
2832
available on every platform Python supports. The Windows-specific
@@ -104,8 +108,24 @@ def open_slave_streams(self, encoding="utf-8"):
104108
# also explicitly calls `sys.stdout.flush()` before reading, so
105109
# bare prompts (no trailing newline) also reach the client
106110
# promptly.
111+
#
112+
# `newline=""` on the writer disables `\n` → `os.linesep`
113+
# translation — a CRITICAL Windows fix, because `os.linesep` is
114+
# `\r\n` there, and the default `newline=None` would translate
115+
# every `\n` the application writes into `\r\n` on the wire.
116+
# That would pollute the client's display with stray `\r`s and
117+
# potentially break the prompt-detection / session-ID-parsing
118+
# regex on `net.client`. On POSIX the setting is a no-op (since
119+
# `os.linesep == "\n"`), so it's also safe to run on Linux —
120+
# and crucially it means the Linux test suite validates exactly
121+
# the same code path that Windows will execute.
122+
#
123+
# The reader uses default `newline=None` (universal newlines),
124+
# which returns `\n`-terminated lines regardless of the actual
125+
# on-wire ending — exactly what `code.InteractiveConsole`
126+
# expects from `sys.stdin.readline()`.
107127
with contextlib.ExitStack() as stack:
108-
wfile = stack.enter_context(self.slave.makefile("w", buffering=1, encoding=encoding))
128+
wfile = stack.enter_context(self.slave.makefile("w", buffering=1, encoding=encoding, newline=""))
109129
rfile = stack.enter_context(self.slave.makefile("r", encoding=encoding))
110130
yield rfile, wfile
111131

0 commit comments

Comments
 (0)