From fa05a4b310d53014ac469ca104cdaf06e216bd9d Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Tue, 29 Sep 2026 00:00:00 +1000 Subject: [PATCH] Devices: fixes from review round 16 - Terminals, power and a reconnect's probes use the route commands have now (a pinned bare-alias destination, a login change still deferred). - Saving port 22 keeps an explicit Port line where there was one. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_devices.py | 4 ++-- tests/test_link.py | 15 +++++++++++++++ ui/frame_devices.py | 6 +++--- ui/frame_link.py | 13 ++++++++++--- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/tests/test_devices.py b/tests/test_devices.py index b276fbd..748d605 100644 --- a/tests/test_devices.py +++ b/tests/test_devices.py @@ -159,9 +159,9 @@ class ConfigRewrite(Base): self.assertTrue(fd.rewrite_block("frame", port=2200, user="deck")) block = fd.parse_blocks(cfg.read_text())[0] self.assertEqual((block["port"], block["user"], block["hostname"]), (2200, "deck", "192.168.1.237")) - self.assertTrue(fd.rewrite_block("frame-2", port=22)) # back to the default: the line goes + self.assertTrue(fd.rewrite_block("frame-2", port=22)) # back to the default: said explicitly self.assertEqual(fd.parse_blocks(cfg.read_text())[1]["port"], 22) - self.assertNotIn("Port 22\n", cfg.read_text()) + self.assertIn(" Port 22\n", cfg.read_text()) self.assertIn("HostName 192.168.1.109", cfg.read_text()) # other hosts untouched if os.name != "nt": self.assertEqual(cfg.stat().st_mode & 0o777, 0o600) diff --git a/tests/test_link.py b/tests/test_link.py index d66357a..2605de8 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -321,6 +321,21 @@ class Connecting(unittest.TestCase): self.assertEqual(self.routes[-1], ("frame-bare", ["-o", "HostName=localhost", "-o", f"Port={self.port}", "-o", "User=tester"])) + def test_a_bare_alias_keeps_its_pinned_route_for_reconnects_and_terminals(self): + self.link.override = "frame-bare" + self.hosts({"localhost": "ok"}) + with mock.patch.object(fl, "ssh_g", return_value=("localhost", self.port, "tester", False)): + self.link.connect(["start"]) + pinned = self.routes[-1][1] + # ~/.ssh/config now sends the alias elsewhere, but an install is running. + self.link.work = lambda: 1 + with mock.patch.object(fl, "ssh_g", return_value=("elsewhere.invalid", 2222, "other", False)): + self.link.close_master() + self.link.connect(["dropped"]) + self.assertEqual(self.link.snapshot()["probes"][0]["host"], "localhost") # probed where commands go + self.assertEqual(self.link.snapshot()["phase"], "connected") + self.assertEqual(self.link.named_route(), ("frame-bare", pinned)) # terminals go there too + def test_a_bare_alias_behind_a_jump_host_is_left_to_ssh(self): self.link.override = "frame-jump" self.hosts({"10.99.99.99": "ok"}) # ssh's ProxyJump would get there diff --git a/ui/frame_devices.py b/ui/frame_devices.py index 891046e..7119030 100644 --- a/ui/frame_devices.py +++ b/ui/frame_devices.py @@ -300,12 +300,12 @@ def _rewritten(lines, alias, hostname, user, port): key = f[0].lower() if f else "" if key in want and want[key] is not None and key not in seen: seen.add(key) - if key == "port" and want[key] == "22": - continue # the default; connect.sh leaves it out + # An existing Port line is kept, even for 22: dropping it could let a later + # `Host *` Port apply to Terminal but not to the app. out.append(f" {f[0]} {want[key]}") else: out.append(line) - if want["port"] and want["port"] != "22" and "port" not in seen: + if want["port"] and want["port"] != "22" and "port" not in seen: # 22 needs no line (as connect.sh writes it) at = next((n + 1 for n, line in enumerate(out) if line.split(None, 1)[:1] == ["HostName"]), 2) out.insert(at, f" Port {want['port']}") new = lines[:i] + out + lines[j:] diff --git a/ui/frame_link.py b/ui/frame_link.py index ba5161f..08994c2 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -309,6 +309,9 @@ class Link: address by name, not the IP it answered from. A zone's % can't be passed through Windows' console, and ssh resolves the name itself.""" device = self.active_device() + routed = self.routed_device + if self.routed is not None and routed and routed["alias"] == device["alias"]: + device = routed # as every command has it now (a frozen route, a login change deferred) via = self.state.get("via") if self.state.get("phase") == "connected" else None host = via["host"] if via else (device["addresses"][0]["host"] if device.get("addresses") else None) return device["alias"], self.host_opts(device, host) @@ -454,8 +457,8 @@ class Link: if device.get("transient") and not device.get("none") and "frozen" not in device: # A bare alias: pin down where ~/.ssh/config sends it now, so an edit to that # file can't move the commands of an install that's running. - h, p, u, _ = ssh_g(device["alias"]) - device = dict(device, user=device.get("user") or u, port=p, frozen=[ + h, p, u, proxied = ssh_g(device["alias"]) + device = dict(device, user=device.get("user") or u, port=p, frozen_host=h, proxied=proxied, frozen=[ "-o", f"HostName={frame_devices.ssh_host(h)}", "-o", f"Port={p}", *(["-o", f"User={u}"] if u else [])]) if self.routed and self.routed_device and device["alias"] == self.routed_device["alias"] \ @@ -558,7 +561,11 @@ class Link: self.stage("find", "active") port = device.get("port") or 22 if device.get("transient"): - host, port, user, proxied = ssh_g(device["alias"]) + # Where the route was pinned (connect), so the probe checks what commands use. + if "frozen_host" in device: + host, port, user, proxied = device["frozen_host"], device["port"], device.get("user"), device["proxied"] + else: + host, port, user, proxied = ssh_g(device["alias"]) if user and not device.get("user"): device["user"] = user a = {"host": host, "kind": frame_network.guess_kind(host), "label": "from ~/.ssh/config"}