-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add terminal support #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Downgrade electron ^38.2.1 → ^31.7.7 (Node 22→20, C++20→C++17) - Add node-pty ^1.0.0 to dependencies - node-pty@1.0.0 + nan@2.17.0 requires C++17, incompatible with Electron 38's C++20 - Electron 31 is last version before C++20 requirement - Verified node-pty builds successfully and spawns PTY processes _Generated with `cmux`_
- Add node-pty dependency for local PTY sessions - Link ghostty-wasm library for terminal rendering - Implement PTYService for managing terminal sessions (local and SSH) - Implement TerminalServer for WebSocket-based terminal I/O - Add IPC handlers for terminal create/close/resize operations - Support SSH terminals via runtime.exec() with PTY allocation (-t flag) - Add terminal types and IPC constants - Start terminal server on IpcMain initialization Phase 1 of 2 for embedded terminal feature. Backend is complete and type-checks.
- Add terminal methods to IPCApi interface - Implement terminal IPC in preload.ts - Add terminal mock implementations for browser and storybook - Import terminal types in ipc.ts Prepares frontend for terminal component implementation.
- Implement useTerminalSession hook for WebSocket connection management - Create TerminalView component with ghostty-wasm integration - Integrate terminal toggle into AIView (Ctrl+` keybind) - Add WASM asset support to vite.config - Terminal appears at bottom of chat area (VS Code style) - Per-workspace terminal visibility persistence Frontend complete - embedded terminal now functional.
- Fix Infinity timeout issue causing SSH terminals to fail immediately - Use 24-hour timeout instead of Infinity for SSH terminals - Add CMUX_ALLOW_MULTIPLE_INSTANCES env var for development - Fix WASM path for dev mode (/src/assets instead of /assets) - Switch to node-pty-prebuilt-multiarch (local terminals need ABI 139) SSH terminals now work. Local terminals require node-pty ABI 139 prebuilt.
- Fixed useTerminalSession dependency array to exclude sessionId - sessionId in deps caused infinite recreation loop - Now uses closure variable for cleanup instead - Added comprehensive debug logging for terminal lifecycle - Tracks session creation, WebSocket events, and cleanup - Logs SSH command execution and stream state - Helps diagnose rapid exit issues - Removed unused @ts-expect-error for ghostty-wasm import - Added node-pty-prebuilt-multiarch (still lacks ABI 139 for Electron 38) This should resolve the SSH terminal reconnection loop. Local terminals still blocked on node-pty ABI 139 availability for Electron 38. Generated with `cmux`
Moved node-pty require() inside the LocalRuntime branch so it only loads when actually creating a local terminal. This prevents the app from crashing on startup when node-pty binaries are missing. Error message now clearly explains the ABI mismatch and suggests using SSH workspaces as a workaround. Generated with `cmux`
The log service is a Node.js backend service and cannot be imported in renderer/frontend code. Changed useTerminalSession to use console methods instead, which work in the browser context. This fixes the blank screen issue caused by trying to import backend services in the frontend. Generated with `cmux`
Added debug logs to track terminal initialization steps: - Component lifecycle (mount/unmount) - Terminal instance creation - FitAddon loading - DOM mounting and fitting This will help diagnose why the terminal window opens but shows no content. Generated with `cmux`
Added logging to track WebSocket → Terminal data flow: - WebSocket effect lifecycle (setup/teardown) - Message receipt (type and data length) - Terminal write operations This will show if PTY output is reaching the terminal and being written. Generated with `cmux`
The useEffect for WebSocket messages had wsRef in the dependency array, but refs don't trigger effects. Changed to only depend on 'connected' and capture ws/term refs at the start of the effect. This should fix the issue where terminal window opens but shows no output. Generated with `cmux`
Added terminalReady state to track when terminal is fully initialized. This prevents the WebSocket message handler from being set up before the terminal ref is available, which was causing the timing issue where messages arrived but had nowhere to go. Generated with `cmux`
Added automatic newline send after WebSocket handler setup to trigger the shell to output its prompt. This helps diagnose if the issue is: - No shell output (should see prompt after newline) - WebSocket not receiving data (will see in logs) - Terminal not rendering (will appear in DOM) Generated with `cmux`
Changed log.debug to log.info for PTY SSH logs so they always appear in the console. This will help us see what's happening with the SSH connection and why no data is flowing. Generated with `cmux`
Added error handling around runtime.exec() call to see if it's failing silently. Also added logs before and after the exec call to track if it's hanging or throwing an error. Generated with `cmux`
The terminal server only knew which WebSocket belonged to which session after receiving the first input message. This meant PTY output that arrived before any input was sent had nowhere to go. Added 'attach' message type that frontend sends immediately after WebSocket connection to register the connection with the session. This should finally make terminal output appear! Generated with `cmux`
The command 'exec $SHELL' was exiting with code 127 (command not found) because $SHELL variable wasn't being expanded properly in the SSH context. Switched to 'bash -l' which starts a login bash shell, which is more reliable and available on most SSH servers. Generated with `cmux`
- Implement PTYService for managing terminal sessions (SSH + local) - Add TerminalServer WebSocket bridge for terminal I/O - Integrate ghostty-wasm for terminal rendering - Add terminal IPC handlers and types - Use 'script' command to create proper PTY for SSH sessions - Handle both stdout and stderr (zsh sends prompt to stderr) - Implement persistent stdin writer to avoid lock contention - Add keyboard shortcuts (Cmd+T) and prevent focus stealing - Fixed-size terminals (100x20) for SSH due to PTY limitations Known limitations: - SSH terminals cannot dynamically resize (fixed at creation) - Local terminals blocked by node-pty ABI mismatch (Electron 38 needs ABI 139) - Some input lag due to SSH round-trip Generated with `cmux`
- vite-plugin-top-level-await uses Node.js Module API incompatible with Bun - Vite 7+ handles top-level await natively with target: esnext - Build now succeeds and produces dist/ _Generated with `cmux`_
This reverts commit 9459a5f.
- Bun's Module class doesn't implement Node.js Module API - Plugin uses Module._resolveFilename which doesn't exist in Bun - Patch to use direct require() first, fallback to Module API - Run scripts/patch-vite-plugin.sh after bun install _Generated with `cmux`_
- Main process is built as CommonJS (tsconfig.main.json) - chalk v5+ is ESM-only, incompatible with require() - Downgrade to chalk@4.1.2 which supports CommonJS _Generated with `cmux`_
- Add '-l' flag to spawn login shell (loads .zshrc, .bash_profile, etc.) - Explicitly set TERM=xterm-256color in env - Ensures proper shell initialization for local terminals _Generated with `cmux`_
- Set TERM_PROGRAM=cmux to identify embedded terminal
- Set DISABLE_AUTO_TITLE=true to prevent title escape sequences
- Set ZSH_AUTOSUGGEST_STRATEGY=history for simpler completions
- Reduces rendering issues with complex zsh plugins/prompts
User can detect cmux terminal in .zshrc with:
if [[ $TERM_PROGRAM == 'cmux' ]]; then
# Use simpler prompt/plugins
fi
_Generated with `cmux`_
This reverts commit bab1a6a.
- Wait for terminal to fit before creating PTY session - Use actual terminal dimensions instead of hardcoded 100x20 - Fixes tab completion rendering corruption (shell width matches display width) - Stabilize terminalSize object references to prevent unnecessary re-renders - Add sessionId to message handler deps to reconnect on toggle - Fixes blank terminal when toggling visibility off and back on Resolves terminal rendering issues identified in ghostty-wasm integration.
- Electron 31.7.7 → 38.6.0 (Node 20 → 22, C++17 → C++20) - node-pty 1.0.0 → 1.1.0-beta39 - node-pty beta uses node-addon-api (N-API) instead of nan - N-API is ABI-stable and C++20 compatible - Successfully rebuilt node-pty native module (84KB) - Binary builds for Electron 38's Node 22 ABI Testing in progress to verify terminal functionality works correctly.
- Update import from ghostty-wasm to ghostty-web package - Add ghostty-web as git dependency from github.com/coder/ghostty-web - Add workspace path validation before spawning PTY - Enhance error logging with shell, cwd, and environment details - Ensure PATH is properly set in PTY environment _Generated with `cmux`_
Terminals now open in new browser windows/tabs when running in server mode: 1. Added stub handlers for TERMINAL_WINDOW_OPEN and TERMINAL_WINDOW_CLOSE in main-server.ts to prevent 404 errors 2. Updated browser API to call window.open() when opening a terminal, which creates a new browser window/tab pointing to /terminal.html 3. Removed debug logging from App.tsx Browser mode terminals work the same as desktop: - Click terminal button or use Ctrl+T (not Cmd+T, that opens browser tab) - Opens /terminal.html?workspaceId=<id> in new window - Terminal connects via WebSocket and works like desktop mode Generated with `mux`
Terminal windows need the browser API to communicate with the backend in server mode. Added dynamic import of browser/api.ts when window.api is not already set up (i.e., when not running in Electron with preload). Generated with `mux`
In browser mode, BrowserWindow is undefined and event.sender is null. Updated TERMINAL_CREATE handler to use the mainWindow (mockWindow) when event.sender is null, which broadcasts terminal events to all WebSocket clients. Generated with `mux`
In server mode, the entire event object is null (not just event.sender). Use optional chaining to check event?.sender before accessing it. Generated with `mux`
In server mode, HttpIpcMainAdapter only creates endpoints for handle() methods, not on() methods. Changed TERMINAL_INPUT to use handle() so it works in both Electron and browser modes. Generated with `mux`
Added timestamp to window.open() name parameter so each Ctrl+T creates a new terminal window instead of reusing the existing one. Generated with `mux`
Removed console.log statements added during debugging. Generated with `mux`
Added platform: 'browser' to browser API so components can detect when running in browser vs Electron. Updated TitleBar to skip update checks when platform === 'browser', eliminating the need for stub handlers in main-server.ts. Generated with `mux`
Changed nodemon to only watch main-server.js and run it directly, not via main.js. This prevents the 'too many arguments' error since main.js is the Electron entry point and doesn't accept server arguments. Also increased delay from 500ms to 1000ms for consistency. Generated with `mux`
Updated terminal-window.tsx to fetch workspace metadata and set document.title to 'Terminal — project/workspace' instead of showing the raw URL in browser tabs. Falls back to workspace ID if metadata can't be fetched. Generated with `mux`
Changed TERMINAL_WINDOW_OPEN handler to silently return when terminalWindowManager is not available (server mode) instead of throwing an error. In server mode, window.open() handles opening the terminal. Generated with `mux`
Changed from non-existent workspace.getMetadata() to workspace.list() and find the matching workspace by ID. Generated with `mux`
This reverts commit c673be7.
Added 'electron-builder install-app-deps' as postinstall script to ensure node-pty and other native dependencies are rebuilt for the correct Electron version during CI builds on Linux and macOS. Fixes 'bun rebuild' not found error in electron-builder. Generated with `mux`
kylecarbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved from me but you should prob get ammar to review before merge
| // Remove 'server' from args since main-server doesn't expect it as a positional argument. | ||
| process.argv.splice(2, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm confused by this i don't think this is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something about my changes upgraded commander to v14 which enforces stricter args parsing. Because main-server.ts isn't expecting args but we're passing it server it crashes out on 14
No description provided.