Skip to content

Conversation

@sreya
Copy link

@sreya sreya commented Nov 12, 2025

No description provided.

sreya added 30 commits November 13, 2025 07:30
- 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`_
- 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`_
- 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`_
…mpty

When ChatInput is used with variant='workspace', it passes empty projectPath
as a dummy value. Skip the API call in this case to avoid errors.
Restore static import of topLevelAwait plugin. The dynamic import
provided no benefit since the dependency is already in package.json.
The original code already only used it in development mode.
@sreya sreya marked this pull request as ready for review November 13, 2025 22:37
@sreya sreya requested a review from kylecarbs November 13, 2025 22:37
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

Comment on lines 82 to 83
this.partialService,
this.initStateManager

Choose a reason for hiding this comment

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

P1 Badge Server mode now crashes due to hard Electron dependency

The commit makes IpcMain unconditionally construct a TerminalWindowManager (lines 30‑83 of src/services/ipcMain.ts), and that class imports BrowserWindow from electron at runtime. The HTTP/server build (src/main-server.ts) reuses IpcMain, but it runs under plain Node on headless machines that typically install only production dependencies (Electron is listed under devDependencies). After this change any make dev-server/node dist/main-server.js run without Electron present fails immediately with Cannot find module 'electron' before the server even starts, breaking remote/server deployments. The terminal window support needs to be guarded so that the server build doesn’t import or instantiate BrowserWindow-only code when Electron isn’t available.

Useful? React with 👍 / 👎.

…erver terminals

Replace dedicated WebSocket server (TerminalServer) with IPC-based terminal
communication. This simplifies architecture and fixes server mode by removing
Electron dependencies from shared code.

Changes:
- PTYService: Use callbacks (onData/onExit) instead of TerminalServer
- IpcMain: Send terminal events via IPC channels (terminal:output:*, terminal:exit:*)
- Preload: Add sendInput, onOutput, onExit methods; remove getPort
- useTerminalSession: Subscribe to IPC events instead of WebSocket
- TerminalView: Pass callbacks to useTerminalSession
- main-desktop: Set TerminalWindowManager (desktop-only feature)
- Delete TerminalServer (215 lines)
- Remove unused WebSocket message types

Benefits:
- Desktop mode: Uses native Electron IPC (more efficient than WebSocket)
- Server mode: Uses existing /ws endpoint (consolidates from 2 WebSocket servers to 1)
- Simpler: No port management, no separate WebSocket server
- Fixes server mode: No BrowserWindow imports in shared IpcMain code

Net: -241 lines

Generated with `mux`
- Remove unused eslint-disable directives in TerminalView
- Remove unused connected/sessionId variables
- Fix BrowserWindow import to use type import
- Remove unused no-unsafe-return eslint disable
- Add stub implementations for terminal methods in browser API
- Change TerminalWindowManager import to 'import type' (only used as type)
- Remove dynamic BrowserWindow import (violates no-restricted-syntax)
- Pass mainWindow to registerTerminalHandlers instead
- All terminals use main window for IPC events

Verified with 'make lint' before commit.

Generated with `mux`
Terminal events (output/exit) now subscribe via WebSocket in browser mode,
matching the pattern used for chat events. Terminal input is sent via HTTP IPC.

This enables terminals to work when accessing mux server from a browser.

Generated with `mux`
The main.js wrapper doesn't properly pass arguments through to main-server.js.
Run main-server.js directly with host/port args instead.

Also remove dist/main.js from nodemon watch since we only care about main-server.js changes.

Generated with `mux`
tsc-alias needs time to replace path aliases after tsgo compiles.
Increase delay from 500ms to 1000ms to prevent race condition where
nodemon restarts before aliases are resolved.

Generated with `mux`
Comment on lines +256 to +259
sendInput: (sessionId: string, data: string) => {
// Send via IPC - in browser mode this becomes an HTTP POST
void invokeIPC(IPC_CHANNELS.TERMINAL_INPUT, sessionId, data);
},
Copy link
Member

Choose a reason for hiding this comment

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

This might be slow over the network. Fine for now, but we should probably allow the wsManager to send data for the IPC at some point.

Comment on lines 240 to 249
// Don't trigger focus keybinds when in terminal
const target = event.target as HTMLElement;
const terminalHasFocus =
target?.closest?.(".terminal-view") !== null ||
target?.classList?.contains("terminal-view") ||
target?.classList?.contains("terminal-container") ||
target?.tagName === "CANVAS"; // ghostty-wasm uses canvas
if (terminalHasFocus) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

We should just stop propagation of keyboard events if the terminal is focused.

I think you can just do event.stopPropagation() in the input handler.

Comment on lines 374 to 380
// Don't auto-focus if the terminal has focus (user is typing in terminal)
const timer = setTimeout(() => {
focusMessageInput();
const activeElement = document.activeElement;
const terminalHasFocus = activeElement?.closest(".terminal-view") !== null;
if (!terminalHasFocus) {
focusMessageInput();
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this would trigger unless I'm confused. This would only happen on the initial render I believe unless something else is wack.

- Remove unnecessary eslint-disable comments from TerminalView
- Move TerminalView out of single-file Terminal folder
- Fix ptyService.ts types and remove eslint-disables
  - Add IPty type import from node-pty
  - Remove runtime.exec 'as any' cast
  - Use ?? instead of || for nullish coalescing
  - Fix template literal type errors
  - Make resize() synchronous (no await needed)
- Remove devServerPort legacy code from TerminalWindowManager
  - Hardcode dev server port to 5173
  - Remove unused CMUX_DEVSERVER_PORT env var
- Add terminal window auto-open on create
  - TERMINAL_CREATE handler auto-opens window in desktop mode
  - Pass sessionId in URL query params for reload support
  - Update TerminalView to accept optional sessionId prop
  - Update useTerminalSession to use existing sessionId if provided

Generated with `mux`
The auto-open feature broke terminal I/O because:
1. Main window calls terminal.create() → backend sends events to main window
2. Backend auto-opens terminal window with sessionId
3. Terminal window never receives events (sent to wrong window)

Reverted to original flow:
1. User clicks "Open Terminal" → openWindow(workspaceId)
2. Window opens and calls terminal.create()
3. Backend sends events to the correct window (sender)

The sessionId parameter is kept in the API for future session reload support,
but currently always creates a new session.

Generated with `mux`
The TERMINAL_CREATE handler was hardcoded to send events to mainWindow,
but terminal windows are separate BrowserWindows that call terminal.create().
This caused terminal output to be sent to the main app window instead of
the terminal window, resulting in a blank/blinking cursor with no prompt.

Fix: Use BrowserWindow.fromWebContents(event.sender) to get the actual
window that made the IPC call, so events are routed correctly.

Generated with `mux`
…checks

Added onKeyDown handler to TerminalView that calls stopPropagation() to prevent
keyboard events from bubbling up to global keybind handlers. This is cleaner than
checking if the terminal has focus in the keybind handler.

Removed terminalHasFocus check from useAIViewKeybinds since terminal now properly
stops event propagation.

Note: ChatInput's terminalHasFocus check serves a different purpose (preventing
auto-focus when switching workspaces while user is in terminal) so it remains.

Generated with `mux`
Removed the last eslint-disable comment by replacing synchronous fs.existsSync()
with async fs.access() using fs/promises. This follows the project convention
of avoiding synchronous fs methods.

Generated with `mux`
…gation

Replaced terminal focus checks in ChatInput with a proper native event listener
in TerminalView that stops propagation in the capture phase. This prevents
keyboard events from reaching window listeners that handle global keybinds.

The native listener is necessary because React's stopPropagation() only works
on synthetic events within React's event system, not on window event listeners.

Removed terminalHasFocus checks from ChatInput's global keydown handler.

Generated with `mux`
Changed keydown event listener from capture phase (true) to bubble phase (false).
In capture phase, we were stopping propagation before the event reached the
terminal's canvas element, preventing any input from working.

Now events reach the terminal canvas first, get processed, then we stop
propagation in the bubble phase to prevent global keybinds from triggering.

Generated with `mux`
Since terminals now run in separate BrowserWindows, the main app window
can never have a terminal with focus. Removed all terminal focus detection:

1. ChatInput auto-focus check - terminals are in separate windows
2. TerminalView stopPropagation - terminal window has no global keybinds
3. TerminalView onKeyDown handler - not needed in separate window

This simplifies the code and removes ~25 lines of now-unnecessary logic.

Generated with `mux`
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.

2 participants