-
Notifications
You must be signed in to change notification settings - Fork 14
🤖 fix: support SSH workspaces in Open Terminal button #557
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
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
7bee2d3 to
0360d52
Compare
- Change IPC handler to accept workspaceId instead of workspacePath - Look up workspace metadata to determine if SSH or local - For SSH: spawn local terminal that runs 'ssh -t host "cd path && exec $SHELL"' - For local: spawn terminal with cwd set (existing behavior) - Support SSH options: port (-p), identity file (-i) - Handle all platforms: macOS (Ghostty, Terminal.app), Windows (cmd), Linux (9+ terminal emulators) - Update all callers to pass workspaceId instead of path Generated with `cmux`
Fixes P1 issue where nested single quotes in the remote command would break Terminal.app's AppleScript parsing. Now uses proper shell escaping: 'foo'\''bar' Generated with `cmux`
Ghostty on macOS requires: open -n -a Ghostty --args --command="ssh ..." instead of passing SSH args directly. Generated with `cmux`
Use multi-line AppleScript with activate and end tell: tell application "Terminal" activate do script "ssh ..." end tell Generated with `cmux`
The isSSH check already validates runtimeConfig.type === "ssh", so the redundant check in the if condition is unnecessary. Generated with `cmux`
Added isSSHRuntime() type guard to RuntimeConfig that provides proper type narrowing, replacing manual type === 'ssh' checks. Generated with `cmux`
The previous commit referenced isSSHRuntime but the function wasn't actually added to the file. Generated with `cmux`
Generated with `cmux`
Applied isSSHRuntime() to RuntimeBadge, GitStatusStore, and ipcMain to replace manual type === 'ssh' checks with type-safe guards. Generated with `cmux`
Store config.type === 'ssh' check in isSSH constant at the start of openTerminal() to avoid multiple type checks throughout. Generated with `cmux`
Use the type guard function instead of manual type === 'ssh' check for consistency with the rest of the codebase. Generated with `cmux`
The config parameter in openTerminal has a different shape than RuntimeConfig (workspacePath vs srcBaseDir), so we can't use the isSSHRuntime type guard here. Generated with `cmux`
219fb7b to
3c6830c
Compare
After rebase, getAllWorkspaceMetadata() is now async. Generated with `cmux`
Fixes the "Open Terminal" button for SSH workspaces, which was failing with ENOENT errors because it tried to open a local path that doesn't exist locally.
Changes
The IPC handler now accepts
workspaceIdinstead ofworkspacePath, looks up the workspace metadata, and spawns the appropriate terminal:ssh -t <host> 'cd <path> && exec $SHELL'-p), identity file (-i)All callers updated to pass
workspaceIdinstead ofnamedWorkspacePath.Testing
make typecheckpasses ✅Generated with
cmux