From 93ebd34f2cb2864024d6a3e1420600aaa0e696b5 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:51:12 +1000 Subject: [PATCH] Devices: fixes from review round 15 - A bare alias's route is pinned to where ~/.ssh/config sent it when it was routed (HostName, Port, User), so editing that file can't move an install. - Renaming during an install is allowed: only a real user or port change waits. - The Devices tab follows a network change even while the headset is offline. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_link.py | 17 +++++++++++++---- ui/frame_link.py | 27 ++++++++++++++++++++++++--- ui/index.html | 6 +++++- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/tests/test_link.py b/tests/test_link.py index 2b77eef..d66357a 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -264,7 +264,7 @@ class Connecting(unittest.TestCase): self.assertIn("alias-frame-bare", [d["id"] for d in fl.devices_view(self.link)["devices"]]) # still there fl.devices_action(self.link, {"action": "use", "id": "alias-frame-bare"}, None) self.assertEqual(self.link.active_device()["alias"], "frame-bare") - self.assertEqual(self.routes[-1], ("frame-bare", [])) + self.assertEqual(self.routes[-1][0], "frame-bare") def test_removing_the_headset_frame_alias_named_doesnt_bring_it_back_bare(self): d = self.device("localhost") @@ -311,17 +311,19 @@ class Connecting(unittest.TestCase): def test_a_bare_alias_lets_ssh_config_decide(self): self.link.override = "frame-bare" - self.hosts({"frame-bare": "ok"}) # the stand-in ssh has no config: the alias is the host + self.hosts({"localhost": "ok"}) with mock.patch.object(fl, "ssh_g", return_value=("localhost", self.port, "tester", False)): self.link.connect(["start"]) s = self.link.snapshot() self.assertEqual(s["phase"], "connected", s["error"]) self.assertTrue(s["device"]["transient"]) - self.assertEqual(self.routes[-1], ("frame-bare", [])) # no HostName override, ssh's own known_hosts + # Where ~/.ssh/config sends it, pinned down for the connection (ssh's own known_hosts). + self.assertEqual(self.routes[-1], ("frame-bare", ["-o", "HostName=localhost", "-o", f"Port={self.port}", + "-o", "User=tester"])) def test_a_bare_alias_behind_a_jump_host_is_left_to_ssh(self): self.link.override = "frame-jump" - self.hosts({"frame-jump": "ok"}) + self.hosts({"10.99.99.99": "ok"}) # ssh's ProxyJump would get there with mock.patch.object(fl, "ssh_g", return_value=("10.99.99.99", 22, "tester", True)): self.link.connect(["start"]) s = self.link.snapshot() @@ -418,6 +420,13 @@ class Connecting(unittest.TestCase): self.assertIn("HostName=localhost", opts) self.assertIn(f"HostKeyAlias=frame-control-{d['id']}", opts) + def test_renaming_during_an_install_is_fine(self): + d = self.device("localhost") + # The page sends the user and port along with the name, unchanged. + fl.devices_action(self.link, {"action": "update", "id": d["id"], "name": "Desk", "user": "steamos", + "port": str(self.port)}, None, busy=lambda: 1) + self.assertEqual(self.reg.get(d["id"])["name"], "Desk") + def test_a_rename_shows_at_once(self): d = self.device("localhost") self.hosts({"localhost": "ok"}) diff --git a/ui/frame_link.py b/ui/frame_link.py index 2427ca9..ba5161f 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -320,7 +320,7 @@ class Link: @staticmethod def route_key(device): - return device["id"], device.get("user"), device.get("port") + return device["id"], device.get("user"), device.get("port"), tuple(device.get("frozen") or ()) def lost(self, message): """A command couldn't reach the headset (Windows has no master to watch).""" @@ -359,7 +359,7 @@ class Link: if device.get("none"): return ["-o", "HostName=no-headset.invalid"] # fails at once, with ssh's own "can't resolve" if device.get("transient"): - return [] + return list(device.get("frozen") or []) # what ~/.ssh/config said when it was routed if not host: # a headset with no addresses: reach nothing, not whatever ~/.ssh/config says return ["-o", "HostName=no-address.invalid"] # StrictHostKeyChecking=yes: whatever ~/.ssh/config says for Host *, every command @@ -451,6 +451,13 @@ class Link: with self.work_lock, self.route_lock: gen = self.attempt_gen = self.gen device = self.active_device() + 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=[ + "-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"] \ and self.route_key(device) != self.routed and self.work(): # Set Up Connection changed this headset in ~/.ssh/config (its login, or a bare @@ -947,6 +954,20 @@ def devices_view(link): "kinds": frame_devices.KIND_LABEL} +def login_change(reg, did, body): + """Whether an update asks for another user or port (the page sends both every time).""" + try: + d = reg.get(did) + except frame_devices.DeviceError: + return False + user, port = body.get("user"), body.get("port") + try: + port = int(port) if port is not None else None + except (TypeError, ValueError): + return True # it will be refused anyway + return (user is not None and user != d["user"]) or (port is not None and port != d["port"]) + + def devices_action(link, body, open_setup, busy=lambda: 0): """POST /api/devices {"action": ..., "id": device id, ...}. -> {"message", ...devices_view}. busy() counts installs in progress: nothing may move them to another headset.""" @@ -958,7 +979,7 @@ def devices_action(link, body, open_setup, busy=lambda: 0): moves = action == "use" or (action == "retry" and link.alive()) or (is_active and ( # (a retry while connected would cut the install's connection) action in ("remove", "address-remove", "forget-identity") - or (action == "update" and (body.get("user") is not None or body.get("port") is not None)) + or (action == "update" and login_change(reg, did, body)) or (action == "address-update" and body.get("newHost") not in (None, body.get("host"))))) if moves and busy(): raise frame_devices.DeviceError( diff --git a/ui/index.html b/ui/index.html index 0cebcb0..23dace1 100644 --- a/ui/index.html +++ b/ui/index.html @@ -2590,7 +2590,11 @@ function onConnection(s) { } } renderBanner(); - if (s.devices_rev !== link.rev || devId !== link.device) { link.rev = s.devices_rev; loadDevices(); } + const netId = s.network ? s.network.id || s.network.gateway || "" : null; + if (s.devices_rev !== link.rev || devId !== link.device || (netId !== null && netId !== link.net)) { + link.rev = s.devices_rev; link.net = netId; // also when only the network changed: the Devices tab shows it + loadDevices(); + } link.phase = s.phase; link.device = devId; if (page === "devices") renderTests(); }