Skip to content

build(pty): restore bundled ConPTY after rebuild#2752

Open
janburzinski wants to merge 4 commits into
mainfrom
emdash/windows-cli-fix-0hz3f
Open

build(pty): restore bundled ConPTY after rebuild#2752
janburzinski wants to merge 4 commits into
mainfrom
emdash/windows-cli-fix-0hz3f

Conversation

@janburzinski

Copy link
Copy Markdown
Collaborator

Description

  • fullscreen tuis (openode,cc,amp,etc.) in terminals on win 10 used to get no mouse input, wheel scrolling or usable text selection
  • conpty swallos mouse-mode/alt-screen requests and drops mouse reports
  • local pty now spawn with node-pty useConptyDll:true, loading the bundled upt odate conpty
  • use bundled conpty for windows ptys
  • restore conpty files after native rebuilds

Screenshot/Recording (if applicable)

on windows:
https://cap.link/p11pxnh1aefg92x

Checklist
  • I kept this PR small and focused
  • I ran a self-review before opening this PR
  • I ran the relevant local checks or explained why not
  • I updated docs when behavior or setup changed
  • I added or updated tests when behavior changed, or explained why not
  • I only added comments where the logic is not obvious
  • I used Conventional Commits for commit
    messages and, when possible, the PR title

@janburzinski

Copy link
Copy Markdown
Collaborator Author

@greptileai

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a long-standing Windows 10 regression where fullscreen TUIs (opencode, claude, amp, etc.) received no mouse input or scroll events because the OS in-box ConPTY silently swallowed mouse-mode DECSETs and SGR mouse reports. The fix opts node-pty into its bundled, up-to-date ConPTY dll (useConptyDll: true) and adds the plumbing to restore that dll after every electron-rebuild invocation, which wipes build/Release.

  • conpty-dll.ts resolves and caches dll presence by mirroring node-pty's native-module search order, with an env-var escape hatch and a markConptyDllUnavailable seam for AV-quarantine fallback.
  • local-pty.ts forwards the resolved flag to nodePty.spawn and retries with the in-box ConPTY if the bundled dll fails to load, permanently disabling it only when the fallback succeeds.
  • copy-conpty-dll.ts (and its callers in postinstall.ts, rebuild-native.ts, package.json, and CI) restores the dll files after every node-gyp rebuild wipes them, with version-folder picking that validates all required files before selecting.

Confidence Score: 5/5

Safe to merge; the fallback to in-box ConPTY is well-guarded and the dll restore runs at all the right callsites.

The retry logic in spawnLocalPty is sound — markConptyDllUnavailable is only called when the in-box fallback succeeds, so command-level failures (bad cwd, missing binary) don't permanently disable the bundled dll. The copy script validates all required files before picking a version folder, handles the no-rebuilt-output case cleanly, and is tested against the key edge cases. The change is Windows-only at runtime and no-ops silently on all other platforms.

No files require special attention; the most scrutiny-worthy path is the spawnLocalPty retry loop in local-pty.ts, which is covered by the new unit tests.

Important Files Changed

Filename Overview
.github/workflows/windows-beta-build.yml Fixes sqlite3→better-sqlite3 module name in electron-rebuild args and adds copy-conpty-dll step after native rebuild; straightforward CI plumbing.
apps/emdash-desktop/scripts/copy-conpty-dll.ts New utility that restores the bundled ConPTY dll after electron-rebuild wipes build/Release; validates arch, checks all required files exist before picking a version folder, and handles platform/arch guards cleanly.
apps/emdash-desktop/scripts/copy-conpty-dll.test.ts Comprehensive tests covering no-op on non-Windows, correct arch selection, missing output/source, incomplete version folders, and unsupported archs.
apps/emdash-desktop/scripts/postinstall.ts Correctly gates the copyConptyDll call on !disablePty, mirroring the existing PTY rebuild guard; the call is a no-op off Windows.
apps/emdash-desktop/scripts/release/rebuild-native.ts Correctly chains copyConptyDll after rebuild completes for the target arch; arch is already validated earlier in the script.
apps/emdash-desktop/src/main/core/pty/conpty-dll.ts Well-designed module with proper memoization, escape hatch env var, test seams, and correct mirroring of node-pty's native module search order.
apps/emdash-desktop/src/main/core/pty/conpty-dll.test.ts Tests cover platform guard, env-var override, rebuilt-vs-prebuilt dll path resolution, and the electron-rebuild wipe scenario.
apps/emdash-desktop/src/main/core/pty/local-pty.ts Spawn logic cleanly delegates ConPTY selection and adds a resilient fallback; markConptyDllUnavailable is only called when the fallback succeeds.
apps/emdash-desktop/src/main/core/pty/local-pty.test.ts New unit tests verify ConPTY flag forwarding, dll-failure retry, dual-failure error propagation, and no-retry when dll was not requested.
apps/emdash-desktop/src/main/core/pty/local-pty.integration.test.ts New Windows-only integration test verifies mouse-mode and alt-screen DECSETs pass through the bundled ConPTY; correctly uses it.runIf to skip on non-Windows.
apps/emdash-desktop/package.json Appends the copy-conpty-dll step to the rebuild script so the dll is restored after every manual pnpm run rebuild invocation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[spawnLocalPty called] --> B[resolveUseConptyDll]
    B --> C{win32?}
    C -- No --> D[return false]
    C -- Yes --> E{EMDASH_DISABLE_CONPTY_DLL=1?}
    E -- Yes --> D
    E -- No --> F[conptyDllIsPresent - cached]
    F --> G{cachedPresence}
    G -- false --> D
    G -- true --> H[return true]
    D --> I[spawn with useConptyDll=false]
    H --> J[spawn with useConptyDll=true]
    J --> K{spawn throws?}
    K -- No --> L[return LocalPtySession]
    K -- Yes --> M[log warn]
    M --> N[retry spawn useConptyDll=false]
    N --> O{retry throws?}
    O -- No --> P[markConptyDllUnavailable]
    P --> Q[return LocalPtySession]
    O -- Yes --> R[throw spawnError]
    I --> S{spawn throws?}
    S -- No --> L
    S -- Yes --> T[throw spawnError]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[spawnLocalPty called] --> B[resolveUseConptyDll]
    B --> C{win32?}
    C -- No --> D[return false]
    C -- Yes --> E{EMDASH_DISABLE_CONPTY_DLL=1?}
    E -- Yes --> D
    E -- No --> F[conptyDllIsPresent - cached]
    F --> G{cachedPresence}
    G -- false --> D
    G -- true --> H[return true]
    D --> I[spawn with useConptyDll=false]
    H --> J[spawn with useConptyDll=true]
    J --> K{spawn throws?}
    K -- No --> L[return LocalPtySession]
    K -- Yes --> M[log warn]
    M --> N[retry spawn useConptyDll=false]
    N --> O{retry throws?}
    O -- No --> P[markConptyDllUnavailable]
    P --> Q[return LocalPtySession]
    O -- Yes --> R[throw spawnError]
    I --> S{spawn throws?}
    S -- No --> L
    S -- Yes --> T[throw spawnError]
Loading

Reviews (3): Last reviewed commit: "fix(pty): select complete bundled ConPTY..." | Re-trigger Greptile

Comment thread apps/emdash-desktop/scripts/copy-conpty-dll.ts Outdated
Comment thread apps/emdash-desktop/scripts/copy-conpty-dll.ts
@janburzinski janburzinski marked this pull request as ready for review July 2, 2026 18:58
@janburzinski

Copy link
Copy Markdown
Collaborator Author

@greptileai

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