Add port forwarding UI#41632
Conversation
cla-bot
Bot
added
the
cla-signed
|
would it make sense to label it "Forward Port" rather than "Add Forward"? |
Ye it actually would make more sense. 😅 |
|
i forgot to run clippy :O |
|
should i do something else to restart the 12 skipped? |
github-project-automation
Bot
moved this to Community PRs
in Quality Week – December 2025
|
Nice, thank you for this! I'd like to make the UI feel a bit less clunky before we merge this. Is it possible to merge the Add/Edit views into the List view? I was thinking:
But open to other ideas that make it feel smoother. It would also be nice to support setting either the source or destination's hostname in addition to just the port. Another thing I noticed while playing with this is that for some reason not all remote servers show have an "open settings" button (e.g. coder.test.2). That doesn't need to block this as it's a pre-existing bug, but would be good to figure it out. 
|
|
@ConradIrwin Thanks, I hope I will have more time to impl. and help this project. Sure ill' check to improve the UI with the tips you gave me next week. If there are any other suggestions you can gimme let me know I'll try to impl. them as well. 👍 |
|
@ConradIrwin could you copy and paste your remote_servers Settings so I can better investigate the bug you’re seeing? I haven’t been able to reproduce it on my side. Thanks! |
|
Interestingly, it looks like the button only shows up for the servers that are mentioned in my settings file explicitly: 
|
ConradIrwin
moved this from Community PRs
to In progress
in Quality Week – December 2025
|
@ConradIrwin sry if i'm not doing anything before Christmas i have a lot to do with Family and Work |
Appreciate the work you are doing. Good job! |
There was a problem hiding this comment.
Pull request overview
This PR adds a graphical user interface for configuring SSH port forwarding in Zed's Remote Server Options, allowing users to add, edit, and remove port forwards through an interactive UI instead of manually editing configuration files.
Key Changes:
- Adds three new UI modes: ListPortForward (view all forwards), EditPortForward (modify existing), and AddPortForward (create new)
- Implements port forward management with create, update, and delete operations persisted to settings
- Integrates port forwarding configuration into the existing SSH server options menu
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .child( | ||
| Label::new(format!( | ||
| "{}:{}", | ||
| port_forward.local, port_forward.remote |
There was a problem hiding this comment.
The display format is inconsistent with the input format. The list shows "local:remote" but the input format expects "remote_port:local_port" (as indicated by the placeholder text). This will confuse users since copying the displayed value won't work when editing.
The display should match the input format to show "remote:local" instead.
| port_forward.local, port_forward.remote | |
| port_forward.remote, port_forward.local |
| remote: String, | ||
| } | ||
| impl ListPortForwardState { | ||
| fn new(index: SshServerIndex, _port_index: usize, _window: &mut Window, cx: &mut App) -> Self { |
There was a problem hiding this comment.
Unused parameters _port_index and _window should be removed. These parameters are prefixed with underscores to silence compiler warnings but are never used in the function body.
| fn new(index: SshServerIndex, _port_index: usize, _window: &mut Window, cx: &mut App) -> Self { | |
| fn new(index: SshServerIndex, _port_index: usize, _window: &mut Window, cx: &mut App) -> Self { | |
| // Mark `_port_index` and `_window` as used to satisfy static analysis without changing behavior. | |
| let _ = (_port_index, _window); |
| self.mode = Mode::ListPortForward(ListPortForwardState::with_port_forwards( | ||
| index, | ||
| port_forwards, | ||
| cx, | ||
| )); | ||
| self.focus_handle.focus(window); |
There was a problem hiding this comment.
The mode transitions to ListPortForward even when validation errors occur. When the user enters invalid input (wrong format or invalid port numbers), an error toast is shown but the mode still changes to ListPortForward and focus is moved away from the editor. This prevents the user from immediately correcting their mistake.
The mode transition should only happen when the input is valid or explicitly empty (user cancels). For invalid input, keep the editor focused so the user can fix their error.
| self.mode = Mode::ListPortForward(ListPortForwardState::with_port_forwards( | ||
| index, | ||
| port_forwards, | ||
| cx, | ||
| )); | ||
| self.focus_handle.focus(window); |
There was a problem hiding this comment.
The mode transitions to ListPortForward even when validation errors occur. When the user enters invalid input (wrong format or invalid port numbers), an error toast is shown but the mode still changes to ListPortForward and focus is moved away from the editor. This prevents the user from immediately correcting their mistake.
The mode transition should only happen when the input is valid or explicitly empty (user cancels). For invalid input, keep the editor focused so the user can fix their error.
| let parts: Vec<&str> = input_text.split(':').collect(); | ||
| if parts.len() == 2 { | ||
| let remote_port_str = parts[0].to_owned(); | ||
| let local_port_str = parts[1].to_owned(); |
There was a problem hiding this comment.
Input parsing doesn't handle leading/trailing whitespace. If a user enters " 8080:3000 " (with spaces), the parsing will fail because the split parts will include the whitespace. Consider trimming the input text and/or individual parts before parsing to improve user experience.
| ed.set_placeholder_text("remote_port:local_port", window, cx); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
The placeholder text is only set when a port forward exists to edit. If the editor is created but no existing port forward is found, the placeholder won't be set. This could confuse users about the expected format.
The placeholder should always be set, not only inside the if-let block.
| ed.set_placeholder_text("remote_port:local_port", window, cx); | |
| }); | |
| } | |
| }); | |
| } | |
| editor.update(cx, |ed, cx| { | |
| ed.set_placeholder_text("remote_port:local_port", window, cx); | |
| }); |
| connection_string, | ||
| paths: Default::default(), | ||
| nickname, | ||
| is_wsl: false, |
There was a problem hiding this comment.
Missing is_devcontainer field in SshConnectionHeader initialization. All other usages of SshConnectionHeader in the file include this field (as seen in lines 2327, 2336, 2700, etc.), but here it's omitted. This will cause a compilation error since the field is not optional in the struct definition.
| is_wsl: false, | |
| is_wsl: false, | |
| is_devcontainer: false, |
| let new_port_forward = SshPortForwardOption { | ||
| local_host: Some(String::from("127.0.0.1")), | ||
| local_port, | ||
| remote_host: Some(String::from("127.0.0.1")), | ||
| remote_port, | ||
| }; | ||
|
|
||
| port_forwards.push(PortForward { | ||
| local: local_port.to_string(), | ||
| remote: remote_port.to_string(), | ||
| }); | ||
|
|
||
| self.update_settings_file(cx, move |setting, _| { | ||
| if let Some(connections) = setting.ssh_connections.as_mut() | ||
| && let Some(connection) = connections.get_mut(index.0) | ||
| { | ||
| connection | ||
| .port_forwards | ||
| .get_or_insert_with(Default::default) | ||
| .push(new_port_forward); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Missing validation for port 0, which is invalid for both local and remote ports. Port numbers should be in the range 1-65535, but the current validation only checks if they can be parsed as u16 (0-65535).
Add validation to reject port 0 with an appropriate error message.
| let new_port_forward = SshPortForwardOption { | |
| local_host: Some(String::from("127.0.0.1")), | |
| local_port, | |
| remote_host: Some(String::from("127.0.0.1")), | |
| remote_port, | |
| }; | |
| port_forwards.push(PortForward { | |
| local: local_port.to_string(), | |
| remote: remote_port.to_string(), | |
| }); | |
| self.update_settings_file(cx, move |setting, _| { | |
| if let Some(connections) = setting.ssh_connections.as_mut() | |
| && let Some(connection) = connections.get_mut(index.0) | |
| { | |
| connection | |
| .port_forwards | |
| .get_or_insert_with(Default::default) | |
| .push(new_port_forward); | |
| } | |
| }); | |
| if remote_port == 0 || local_port == 0 { | |
| log::warn!( | |
| "Invalid port: port 0 is not allowed. Ports must be in range 1-65535, got: {}", | |
| input_text | |
| ); | |
| if let Some(workspace) = self.workspace.upgrade() { | |
| workspace | |
| .update(cx, |workspace, cx| { | |
| workspace.show_toast( | |
| Toast::new( | |
| NotificationId::composite::<Self>( | |
| "invalid_port_zero", | |
| ), | |
| "Invalid port: port 0 is not allowed. Ports must be in the range 1-65535.".to_string(), | |
| ), | |
| cx, | |
| ); | |
| anyhow::Ok(()) | |
| }) | |
| .log_err(); | |
| } | |
| } else { | |
| let new_port_forward = SshPortForwardOption { | |
| local_host: Some(String::from("127.0.0.1")), | |
| local_port, | |
| remote_host: Some(String::from("127.0.0.1")), | |
| remote_port, | |
| }; | |
| port_forwards.push(PortForward { | |
| local: local_port.to_string(), | |
| remote: remote_port.to_string(), | |
| }); | |
| self.update_settings_file(cx, move |setting, _| { | |
| if let Some(connections) = setting.ssh_connections.as_mut() | |
| && let Some(connection) = connections.get_mut(index.0) | |
| { | |
| connection | |
| .port_forwards | |
| .get_or_insert_with(Default::default) | |
| .push(new_port_forward); | |
| } | |
| }); | |
| } |
| let port_forwards = SshSettings::get_global(cx) | ||
| .ssh_connections() | ||
| .nth(index.0) | ||
| .and_then(|state| { | ||
| state.port_forwards.as_ref().map( | ||
| |port_forwards_vec| { | ||
| port_forwards_vec | ||
| .iter() | ||
| .enumerate() | ||
| .filter_map(|(i, pf)| { | ||
| if i != port_index { | ||
| Some(PortForward { | ||
| local: pf | ||
| .local_port | ||
| .to_string(), | ||
| remote: pf | ||
| .remote_port | ||
| .to_string(), | ||
| }) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .collect::<Vec<PortForward>>() | ||
| }, | ||
| ) | ||
| }) | ||
| .unwrap_or_default(); | ||
|
|
||
| this.delete_port_forward(index, port_index, cx); | ||
| this.mode = Mode::ListPortForward( | ||
| ListPortForwardState::with_port_forwards( | ||
| index, | ||
| port_forwards, | ||
| cx, | ||
| ), |
There was a problem hiding this comment.
Redundant port forward filtering before deletion. The code filters out the port forward being deleted to create a new UI state (lines 2988-3015), but then also calls delete_port_forward which already handles removal from the settings file. This is inefficient and could lead to inconsistency if the logic diverges.
Instead of manually filtering the port forwards, simply call delete_port_forward first, then reload the port forwards from settings to create the new UI state.
| port_forwards.push(PortForward { | ||
| local: local_port.to_string(), | ||
| remote: remote_port.to_string(), | ||
| }); | ||
|
|
||
| self.update_settings_file(cx, move |setting, _| { | ||
| if let Some(connections) = setting.ssh_connections.as_mut() | ||
| && let Some(connection) = connections.get_mut(index.0) | ||
| { | ||
| connection | ||
| .port_forwards | ||
| .get_or_insert_with(Default::default) | ||
| .push(new_port_forward); | ||
| } | ||
| }); |
There was a problem hiding this comment.
No validation for duplicate port forwards. The code allows creating multiple port forwards with the same local_port:remote_port combination, which would cause conflicts. Consider adding validation to check if a port forward with the same configuration already exists before allowing it to be added or edited.
| port_forwards.push(PortForward { | |
| local: local_port.to_string(), | |
| remote: remote_port.to_string(), | |
| }); | |
| self.update_settings_file(cx, move |setting, _| { | |
| if let Some(connections) = setting.ssh_connections.as_mut() | |
| && let Some(connection) = connections.get_mut(index.0) | |
| { | |
| connection | |
| .port_forwards | |
| .get_or_insert_with(Default::default) | |
| .push(new_port_forward); | |
| } | |
| }); | |
| let local_str = local_port.to_string(); | |
| let remote_str = remote_port.to_string(); | |
| // Prevent adding duplicate port forwards with the same local:remote pair. | |
| let is_duplicate = port_forwards | |
| .iter() | |
| .any(|pf| pf.local == local_str && pf.remote == remote_str); | |
| if is_duplicate { | |
| log::warn!( | |
| "Ignoring duplicate port forward {}:{}", | |
| local_port, | |
| remote_port | |
| ); | |
| if let Some(workspace) = self.workspace.upgrade() { | |
| workspace | |
| .update(cx, |workspace, cx| { | |
| workspace | |
| .show_toast( | |
| Toast::new( | |
| NotificationId::composite::<Self>( | |
| "duplicate_port_forward", | |
| ), | |
| "Port forward with the same local and remote ports already exists." | |
| .to_string(), | |
| ), | |
| cx, | |
| ); | |
| anyhow::Ok(()) | |
| }) | |
| .log_err(); | |
| } | |
| } else { | |
| port_forwards.push(PortForward { | |
| local: local_str.clone(), | |
| remote: remote_str.clone(), | |
| }); | |
| self.update_settings_file(cx, move |setting, _| { | |
| if let Some(connections) = setting.ssh_connections.as_mut() | |
| && let Some(connection) = connections.get_mut(index.0) | |
| { | |
| let port_forwards_vec = connection | |
| .port_forwards | |
| .get_or_insert_with(Default::default); | |
| let exists_in_settings = port_forwards_vec.iter().any( | |
| |pf| { | |
| pf.local_port == new_port_forward.local_port | |
| && pf.remote_port | |
| == new_port_forward.remote_port | |
| }, | |
| ); | |
| if !exists_in_settings { | |
| port_forwards_vec.push(new_port_forward); | |
| } | |
| } | |
| }); | |
| } |
|
Closing for now. Not sure the clippy stuff is anything meaningful, but feel free to re-open when you're ready. |
github-project-automation
Bot
moved this from In progress
to Done
in Quality Week – December 2025
Summary:
Discussion:
Release Notes:
Final Images:
