fix: enable Windows PTY compatibility for local terminals#286
fix: enable Windows PTY compatibility for local terminals#286
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab87cf611d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rendererType = settings?.rendererType ?? "auto"; | ||
| const bridge = netcattyBridge.get(); | ||
| const windowsPty = | ||
| platform === "win32" && ctx.host.protocol === "local" |
There was a problem hiding this comment.
The new windowsPty option is only enabled when ctx.host.protocol === "local", but local sessions are also started for hosts with hostname === "localhost" (components/Terminal.tsx, local-session branch). On Windows, those legacy/default localhost entries will still run through the local node-pty path without windowsPty metadata, so the redraw/wrap heuristics this commit intends to enable remain disabled and the rendering issue can persist for that class of local sessions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Local sessions are started for both protocol === "local" and legacy/default hostname === "localhost" entries, so I aligned the windowsPty detection with that same predicate in 4c32ea2.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 991beb6ff4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const settings = ctx.terminalSettingsRef.current; | ||
| const rendererType = settings?.rendererType ?? "auto"; | ||
| const bridge = netcattyBridge.get(); | ||
| const isLocalTerminalHost = ctx.host.protocol === "local"; |
There was a problem hiding this comment.
windowsPty is only enabled when ctx.host.protocol === "local", but local sessions are still launched for legacy/default hosts where hostname === "localhost" (see components/Terminal.tsx, local-session branch at line 515), so those Windows local terminals miss the xterm Windows PTY heuristics this fix is trying to enable. Fresh evidence: this commit’s gate at createXTermRuntime keeps the narrower predicate while the launcher path still accepts both local forms, so localhost-backed local sessions remain affected.
Useful? React with 👍 / 👎.
1 participant
Summary
windowsPtycompatibility metadata for Windows local terminal sessionsWhy
Issue #280 shows Codex CLI's animated status text being repeatedly painted down the screen when run inside Netcatty's local PowerShell terminal on Windows. Netcatty starts local shells via node-pty, but the frontend terminal was not told it was rendering a Windows PTY stream, so xterm.js could not enable its Windows-specific redraw/wrap heuristics.
Validation
Notes
Refs #280