Devices: fixes from review round 24

- While the connector is taking a queued reconnect off its list, the old
  connection no longer counts as live, so no install starts on it.
- Importing a block without a Port takes the port ssh would really use
  (ssh -F <config> -G), e.g. one a later Host * sets.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/test_devices.py

@@ -124,6 +124,13 @@ class Migration(Base):
         self.assertEqual([d["alias"] for d in self.reg.devices()], ["frame-2", "frame"])
         self.assertEqual(self.reg.active(), self.reg.by_alias("frame")["id"])
 
+    @unittest.skipUnless(shutil.which("ssh"), "needs ssh")
+    def test_a_port_inherited_from_another_host_entry_is_kept(self):
+        (self.ssh / "config").write_text(CONFIG.replace("Host *\n  ServerAliveInterval 60", "Host *\n  Port 2222"))
+        self.reg.sync_from_config(seed=False)
+        self.assertEqual(self.reg.by_alias("frame")["port"], 2222)  # what ssh itself would use
+        self.assertEqual(self.reg.by_alias("frame-2")["port"], 2222)  # its own Port line
+
     def test_setup_finding_a_new_address_adds_it(self):
         self.reg.sync_from_config(seed=False)
         (self.ssh / "config").write_text(CONFIG.replace("HostName frame.tail1234.ts.net", "HostName 192.168.1.237"))

tests/test_link.py

@@ -328,6 +328,12 @@ class Connecting(unittest.TestCase):
         self.assertEqual(self.link.failed("login", ["steamos@frame: Permission denied (publickey)."], False, "frame"),
                          "stop")
 
+    def test_a_reconnect_being_started_isnt_a_live_connection(self):
+        self.link.state["phase"] = "connected"
+        self.assertTrue(self.link.alive())
+        self.link.busy = True  # the loop took a Retry off the queue and is about to reconnect
+        self.assertFalse(self.link.alive())  # so ensure() waits instead of starting work on it
+
     def test_probes_from_an_earlier_attempt_leave_the_new_rows_alone(self):
         self.link.state.update(attempt=2, probes=[{"host": "b", "state": "waiting"}])
         self.link.probe_update(0, 1, state="answered", ip="10.0.0.2")

ui/frame_devices.py

@@ -157,7 +157,8 @@ def parse_blocks(text):
     for line in text.splitlines():
         m = BLOCK_RE.fullmatch(line.strip())
         if m:
-            cur = {"alias": m.group(1), "hostname": None, "user": None, "port": 22, "identity_files": []}
+            cur = {"alias": m.group(1), "hostname": None, "user": None, "port": 22, "port_set": False,
+                   "identity_files": []}
             continue
         if cur is None:
             continue
@@ -174,7 +175,7 @@ def parse_blocks(text):
         elif key == "user" and cur["user"] is None:
             cur["user"] = value
         elif key == "port" and value.isdigit():
-            cur["port"] = int(value)
+            cur["port"], cur["port_set"] = int(value), True
         elif key == "identityfile":
             cur["identity_files"].append(value)
     return blocks
@@ -278,7 +279,9 @@ def rewrite_block(alias, path=None, hostname=None, user=None, port=None, expect=
     def change(lines):
         if expect:
             block = next((b for b in parse_blocks("\n".join(lines)) if b["alias"] == alias), None)
-            if not block or any(v is not None and block[k] != v for k, v in expect.items()):
+            # A port the block doesn't set is inherited from elsewhere in the file: not compared.
+            if not block or any(v is not None and block[k] != v and (k != "port" or block["port_set"])
+                                for k, v in expect.items()):
                 return None
         return _rewritten(lines, alias, hostname, user, port)
     return _edit_config(Path(path or ssh_config()), change)
@@ -321,6 +324,18 @@ def remove_block(alias, path=None):
     return _edit_config(Path(path or ssh_config()), change)
 
 
+def effective_port(alias, config):
+    """The port ssh uses for ALIAS with this config file (`ssh -F FILE -G ALIAS`), else 22."""
+    try:
+        out = subprocess.run(["ssh", "-F", str(config), "-G", alias], capture_output=True, text=True,
+                             stdin=subprocess.DEVNULL, timeout=10).stdout
+    except (OSError, subprocess.TimeoutExpired):
+        return 22
+    m = re.search(r"^port (\d+)$", out, re.M)
+    port = int(m.group(1)) if m else 22
+    return port if 1 <= port <= 65535 else 22
+
+
 # ---- pinned host keys -------------------------------------------------------------
 
 def _keygen(*args):
@@ -670,6 +685,10 @@ class Registry:
         """Import managed blocks we don't know yet, and pick up a HostName that Set Up
         Connection changed since we last looked. -> True if anything changed."""
         blocks = parse_blocks(read_config(self.config))
+        for b in blocks:
+            if not b["port_set"]:
+                # No Port in the block: another Host entry may give one (ssh uses the first).
+                b["port"] = effective_port(b["alias"], self.config or ssh_config())
         changed = False
         with self.lock:
             first = not self.data["devices"] and not self.data.get("active")

ui/frame_link.py

@@ -259,8 +259,8 @@ class Link:
         self.close_master()
 
     def alive(self):
-        if self.state["phase"] != "connected" or self.kicks:
-            return False
+        if self.state["phase"] != "connected" or self.kicks or self.busy:
+            return False  # a reconnect is queued or starting: don't begin anything on this connection
         if not self.control:
             return True
         return self.master is None or self.master.poll() is None