fix(desktop): bump @lydell/node-pty to 1.2.0-beta.12#29803
Merged
Conversation
Fixes the Windows sidecar crash flood where killing a PTY forks conpty_console_list_agent.js, which called getConsoleProcessList() without a try/catch and threw an uncaught 'AttachConsole failed' (node v24). beta.12 wraps the call in try/catch and guards innerPid<=0, eliminating the per-terminal-close crash.
beta.12 ships node-pty.d.ts, so the import now type-resolves and the suppression directive is flagged unused (TS2578).
Contributor
There was a problem hiding this comment.
Pull request overview
Bumps @lydell/node-pty from 1.2.0-beta.10 to 1.2.0-beta.12 to pick up the upstream fix that wraps getConsoleProcessList in try/catch, eliminating the repeated Windows sidecar crash on PTY kill() (issue #29599). The now-redundant @ts-expect-error directive is removed since beta.12 ships type definitions.
Changes:
- Bump catalog and all 6 platform-specific
@lydell/node-pty-*optional deps to1.2.0-beta.12(with regeneratedbun.lock). - Drop the
@ts-expect-errordirective above the@lydell/node-ptyimport in the Node PTY adapter.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| package.json | Catalog @lydell/node-pty bumped to 1.2.0-beta.12. |
| packages/desktop/package.json | All six platform-specific optional deps bumped to 1.2.0-beta.12. |
| bun.lock | Regenerated lockfile entries and checksums for beta.12. |
| packages/opencode/src/pty/pty.node.ts | Removes unused @ts-expect-error since beta.12 ships type defs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tranngoclai
pushed a commit
to tranngoclai/opencode
that referenced
this pull request
May 30, 2026
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.
Summary
Bumps
@lydell/node-pty(and its per-platform packages) from1.2.0-beta.10→1.2.0-beta.12to fix the repeated Windows desktop sidecar crash / stderr flood reported in #29599.Root cause (reproduced headlessly on Windows, node v24.14.1)
The desktop sidecar resolves
@lydell/node-ptyto@lydell/node-pty-win32-x64(viaelectron.vite.config.ts). On every PTYkill(), node-pty'sWindowsPtyAgentdoeschild_process.fork('conpty_console_list_agent'), and in beta.10 that agent callsgetConsoleProcessList(shellPid)with no error handling:kill()forks this agent and then synchronously kills the pty, so by the time the agent runsAttachConsole(shellPid)the shell is already gone (and the Electron utilityProcess has no console at all) → native throwsError: AttachConsole failed→ the forked process exits with the stack dumped to stderr, which the desktop surfaces assidecar stderr {...}. This matches the issue's "Error 1" line-for-line and fires on every terminal close (reproduced 40/40, including in a detached no-console process).Fix
@lydell/node-pty@1.2.0-beta.12carries the upstream fix that wraps the call intry/catchand guardsinnerPid <= 0:Verified locally: after the bump, spawning + killing a PTY produces no
AttachConsole failedoutput.Changes
package.json— catalog@lydell/node-pty→1.2.0-beta.12packages/desktop/package.json— the 6@lydell/node-pty-<platform>optional deps →1.2.0-beta.12bun.lock— regeneratedpackages/opencode/src/pty/pty.node.ts— drop the now-redundant@ts-expect-erroron the@lydell/node-ptyimport (beta.12 shipsnode-pty.d.ts, so the directive became unused → TS2578)Notes / not addressed here
forkhas nowindowsHide); beta.12 reduces but doesn't fully remove this.net::ERR_FAILED/ "fetch failed") is a separate follow-up inpackages/desktop/src/main/index.ts.Refs #29599