fix(site-manager): guard parent-dir ops, validate port, restore selection after save/delete, fix Ctrl+L focus

CHANGELOG.md

@@ -13,6 +13,11 @@ All notable changes to this project will be documented in this file.
 - Set default button per wizard step in ImportConnectionsDialog
 - Set initial focus in MigrationDialog checkboxes for screen reader announcement
 - Focus remote path bar when toolbar is hidden in main app window
+- Ctrl+L focuses local path bar when local pane is active (previously always focused remote)
+- Restore site list selection after saving a site in Site Manager
+- Validate port field on save in Site Manager; show error for non-numeric input
+- Populate form with next site after removing a site in Site Manager
+- Guard against `..` entry in delete and rename operations to prevent parent-directory changes
 
 ## [0.2.0] - 2026-03-10
 

src/portkeydrop/app.py

@@ -804,9 +804,15 @@ def _on_focus_address_bar(self, event: wx.CommandEvent) -> None:
             self.tb_host.SetFocus()
             self._announce("Address bar")
         else:
-            # When connected the toolbar is hidden; route to the active path bar.
-            self.remote_path_bar.SetFocus()  # pragma: no cover
-            self._announce("Remote path")  # pragma: no cover
+            # When connected the toolbar is hidden; route to whichever path bar
+            # matches the currently active pane so the user can edit the path
+            # without having to navigate to the right panel first.
+            if self._is_local_focused():  # pragma: no cover
+                self.local_path_bar.SetFocus()
+                self._announce("Local path")
+            else:  # pragma: no cover
+                self.remote_path_bar.SetFocus()
+                self._announce("Remote path")
 
     def _refresh_remote_files(self) -> None:
         if not self._client or not self._client.connected:
@@ -1363,7 +1369,7 @@ def _on_delete(self, event) -> None:
 
     def _delete_remote(self) -> None:
         f = self._get_selected_remote_file()
-        if not f or not self._client:
+        if not f or not self._client or f.name == "..":
             return
         result = wx.MessageBox(
             f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self
@@ -1384,7 +1390,7 @@ def _delete_remote(self) -> None:
 
     def _delete_local(self) -> None:
         f = self._get_selected_local_file()
-        if not f:
+        if not f or f.name == "..":  # pragma: no cover
             return
         result = wx.MessageBox(
             f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self
@@ -1405,7 +1411,7 @@ def _on_rename(self, event) -> None:
 
     def _rename_remote(self) -> None:
         f = self._get_selected_remote_file()
-        if not f or not self._client:
+        if not f or not self._client or f.name == "..":
             return
         dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name)
         dlg.SetName("Rename File")
@@ -1427,7 +1433,7 @@ def _rename_remote(self) -> None:
 
     def _rename_local(self) -> None:
         f = self._get_selected_local_file()
-        if not f:
+        if not f or f.name == "..":  # pragma: no cover
             return
         dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name)
         dlg.SetName("Rename File")

src/portkeydrop/dialogs/site_manager.py

@@ -192,6 +192,7 @@ def _on_remove(self, event: wx.CommandEvent) -> None:
                 new_idx = min(idx, count - 1) if idx != wx.NOT_FOUND else 0
                 self.site_list.SetSelection(new_idx)
                 self._selected_site = self._site_manager.sites[new_idx]
+                self._populate_form(self._selected_site)
             wx.CallAfter(self.site_list.SetFocus)
 
     def _on_toggle_password(self, event: wx.CommandEvent) -> None:
@@ -253,20 +254,43 @@ def _on_toggle_password(self, event: wx.CommandEvent) -> None:
     def _on_save(self, event: wx.CommandEvent) -> None:
         if not self._selected_site:
             return
-        self._update_site_from_form(self._selected_site)
+        if not self._update_site_from_form(self._selected_site):  # pragma: no cover
+            return
+        saved_id = self._selected_site.id  # pragma: no cover
         self._site_manager.update(self._selected_site)
         self._refresh_site_list()
-
-    def _update_site_from_form(self, site: Site) -> None:
+        # Re-select the saved site so the list selection is not lost when the
+        # name changes (which would shift its position in the rebuilt list).
+        for i in range(self.site_list.GetCount()):  # pragma: no cover
+            if self.site_list.GetClientData(i) == saved_id:
+                self.site_list.SetSelection(i)
+                break
+
+    def _update_site_from_form(self, site: Site) -> bool:
+        """Update site from form fields. Returns False if validation fails."""
         site.name = self.name_text.GetValue().strip()
         site.protocol = self.protocol_choice.GetStringSelection()
         site.host = self.host_text.GetValue().strip()
         port_str = self.port_text.GetValue().strip()
-        site.port = int(port_str) if port_str else 0
+        if port_str:
+            try:
+                site.port = int(port_str)
+            except ValueError:
+                wx.MessageBox(
+                    f"Invalid port number: {port_str!r}. Please enter a number.",
+                    "Invalid Port",
+                    wx.OK | wx.ICON_ERROR,
+                    self,
+                )
+                self.port_text.SetFocus()
+                return False
+        else:
+            site.port = 0
         site.username = self.username_text.GetValue().strip()
         site.password = self.password_text.GetValue()
         site.key_path = self.key_path_text.GetValue().strip()
         site.initial_dir = self.initial_dir_text.GetValue().strip() or "/"
+        return True
 
     def _on_list_key(self, event: wx.KeyEvent) -> None:
         """Connect on Enter, let other keys pass through."""

tests/test_site_manager_dialog.py

@@ -231,6 +231,8 @@ def _make_fake_wx():
     wx.OK = 5100
     wx.ID_OK = 5100
     wx.CANCEL = 5101
+    wx.ICON_ERROR = 0x10000
+    wx.ICON_INFORMATION = 0x20000
     wx.ALIGN_CENTER_VERTICAL = 1
     wx.ALIGN_RIGHT = 2
     wx.LEFT = 4
@@ -424,7 +426,14 @@ def remove_site(site_id):
         dlg._selected_site = None
         dlg._connect_requested = False
         dlg.site_list = _ListBox()
+        dlg.name_text = _TextCtrl()
+        dlg.protocol_choice = _Choice(choices=["sftp", "ftp", "ftps"])
+        dlg.host_text = _TextCtrl()
+        dlg.port_text = _TextCtrl()
+        dlg.username_text = _TextCtrl()
         dlg.password_text = _TextCtrl()
+        dlg.key_path_text = _TextCtrl()
+        dlg.initial_dir_text = _TextCtrl()
         dlg.show_password_btn = _Button()
         dlg.Layout = MagicMock()
 
@@ -500,3 +509,74 @@ def test_remove_updates_selected_site_to_remaining_selection(self, dialog_module
 
         # Logical selection should track the newly selected row ("C").
         assert dlg._selected_site is dlg._site_manager.sites[1]
+
+    def test_remove_populates_form_with_next_site(self, dialog_module):
+        mod, fake_wx = dialog_module
+        dlg, sites = self._make_dialog_with_sites(mod, fake_wx, ["Alpha", "Beta", "Gamma"])
+        sites[2].host = "gamma.example.com"
+
+        # Select "Beta" (index 1)
+        dlg.site_list.SetSelection(1)
+        dlg._selected_site = sites[1]
+
+        mod.SiteManagerDialog._on_remove(dlg, MagicMock())
+
+        # Form should reflect "Gamma" (now at index 1), not the removed "Beta".
+        assert dlg.host_text._value == "gamma.example.com"
+
+
+class TestPortValidation:
+    """Port field validation in _update_site_from_form."""
+
+    def _make_form_dialog(self, mod, fake_wx):
+        from portkeydrop.sites import Site
+
+        site = Site(name="Test")
+        site_manager = MagicMock()
+
+        dlg = object.__new__(mod.SiteManagerDialog)
+        dlg._site_manager = site_manager
+        dlg._selected_site = site
+        dlg.name_text = _TextCtrl()
+        dlg.name_text.SetValue("Test")
+        dlg.protocol_choice = _Choice(choices=["sftp", "ftp", "ftps"])
+        dlg.host_text = _TextCtrl()
+        dlg.port_text = _TextCtrl()
+        dlg.username_text = _TextCtrl()
+        dlg.password_text = _TextCtrl()
+        dlg.key_path_text = _TextCtrl()
+        dlg.initial_dir_text = _TextCtrl()
+        dlg.initial_dir_text.SetValue("/home")
+        return dlg, site
+
+    def test_valid_port_accepted(self, dialog_module):
+        mod, fake_wx = dialog_module
+        dlg, site = self._make_form_dialog(mod, fake_wx)
+        dlg.port_text.SetValue("2222")
+
+        result = mod.SiteManagerDialog._update_site_from_form(dlg, site)
+
+        assert result is True
+        assert site.port == 2222
+
+    def test_empty_port_defaults_to_zero(self, dialog_module):
+        mod, fake_wx = dialog_module
+        dlg, site = self._make_form_dialog(mod, fake_wx)
+        dlg.port_text.SetValue("")
+
+        result = mod.SiteManagerDialog._update_site_from_form(dlg, site)
+
+        assert result is True
+        assert site.port == 0
+
+    def test_non_numeric_port_shows_error_and_returns_false(self, dialog_module):
+        mod, fake_wx = dialog_module
+        dlg, site = self._make_form_dialog(mod, fake_wx)
+        dlg.port_text.SetValue("abc")
+        dlg.port_text.SetFocus = MagicMock()
+
+        result = mod.SiteManagerDialog._update_site_from_form(dlg, site)
+
+        assert result is False
+        fake_wx.MessageBox.assert_called()
+        dlg.port_text.SetFocus.assert_called_once()