Skip to content

fix(server): restore Windows terminal startup after node-pty upgrade - #1

Merged
SegFaultZero merged 2 commits into
mainfrom
fix/windows-terminal-async-pid
Sep 27, 2026
Merged

SegFaultZero merged 2 commits into
mainfrom
fix/windows-terminal-async-pid

Conversation

@SegFaultZero

@SegFaultZero SegFaultZero commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

What Changed

Wait for a valid Windows terminal PID before completing the node-pty adapter's spawn operation. This restores terminal startup after the node-pty upgrade; the terminal manager keeps storing the PID once, and Unix startup is unchanged.

The adapter uses node-pty's private ready_datapipe event, validates the PID, and removes listeners on completion or interruption. Startup failures are reported as PtySpawnError. Cancellation uses the internal Windows agent because public kill() waits for first output. Comments link the upstream changes that require these compatibility hooks.

Why

Terminals on Windows are broken after the node-pty upgrade: opening a terminal displays "[terminal] The environment request failed." instead of a usable shell.

After pingdotgg#13748 upgraded node-pty from 1.1.0 to 1.2.0-beta.15, Windows spawn initially returns PID 0 while process creation is still pending. T3 immediately stored that startup value and never refreshed it when node-pty assigned the real PID. Clients rejected the resulting terminal snapshots because the contract accepts only a positive PID or null, displaying "[terminal] The environment request failed."
image

microsoft/node-pty#885 deferred Windows process creation to avoid blocking on named pipes. Waiting for its internal readiness event handles silent processes without polling or waiting for shell output. The private API dependency is confined to the adapter.

Validation

  • 97 focused tests passed: vp test run src/terminal/NodePtyAdapter.test.ts src/terminal/Manager.test.ts from apps/server.
  • Server typecheck and targeted lint passed.
  • vp run build:desktop succeeded on Windows.
  • Native Windows PTY check returned a positive PID and preserved output and exit delivery.
  • Reporter ran the compiled desktop app and confirmed that terminals work.

Implemented with GPT-6-Astra through the Codex harness.

Comment thread apps/server/src/terminal/Manager.ts Outdated

@SegFaultZero SegFaultZero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@SegFaultZero
SegFaultZero marked this pull request as ready for review September 27, 2026 06:12
@SegFaultZero SegFaultZero changed the title fix(server): handle asynchronous Windows terminal process IDs fix(server): restore Windows terminal startup after node-pty upgrade Sep 27, 2026
@SegFaultZero
SegFaultZero merged commit 2a44bba into main Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant