Merge branch 'ui-2-connection' into ui-3-home

docs/devices.md

@@ -78,10 +78,11 @@ can add, edit, reorder and remove them without leaving the page you're on. When
 the headset reports a LAN IP on the same network as this computer and that IP
 isn't saved, it offers to add it. The offer puts the address first in the list,
 so on that network it wins over the Tailscale name; away from home the Tailscale
-name still leads. A new or edited address is tested straight away. When an address
+name still leads. A new or edited address is tested straight away. When the test
-that would be tried before the one in use passes the test, it gets **Use now**,
+finishes and the first address that passed would be tried before the one in use,
-which reconnects; the reconnect picks the first address that answers, which is
+that address gets **Use now**, which reconnects. A reconnect tries the addresses
-that one. Otherwise the change takes effect the next time Frame Control connects.
+in order and normally lands on it, but takes a later one if it's slow to answer
+then. Otherwise the change takes effect the next time Frame Control connects.
 
 ## Networks
 

tests/test_link.py

@@ -304,19 +304,6 @@ class Connecting(unittest.TestCase):
         self.assertEqual(self.link.active_device()["alias"], "frame-bare")
         self.assertEqual(self.routes[-1][0], "frame-bare")
 
-    def test_devices_view_ranks_addresses_for_the_current_network(self):
-        # The page offers Use now only on the address a reconnect would pick: rank says which.
-        d = self.device("frame.tail1234.ts.net")
-        self.reg.add_address(d["id"], "192.168.1.40", kind="lan", first=True)
-        self.reg.record_success(d["id"], "frame.tail1234.ts.net", "n-home", 6.0)
-        self.link.state["network"] = {"id": "n-home", "name": "Home", "tailscale": {"up": True}}
-        view = next(x for x in fl.devices_view(self.link)["devices"] if x["id"] == d["id"])
-        self.assertEqual({a["host"]: a["rank"] for a in view["addresses"]},
-                         {"192.168.1.40": 1, "frame.tail1234.ts.net": 0})  # only Tailscale worked here so far
-        self.reg.record_success(d["id"], "192.168.1.40", "n-home", 1.0)
-        view = next(x for x in fl.devices_view(self.link)["devices"] if x["id"] == d["id"])
-        self.assertEqual([a["rank"] for a in view["addresses"]], [0, 1])  # now the user's order decides
-
     def test_removing_the_headset_frame_alias_named_doesnt_bring_it_back_bare(self):
         d = self.device("localhost")
         self.link.override = self.link.session_alias = "frame-t"
@@ -462,11 +449,19 @@ class Connecting(unittest.TestCase):
         d = self.device("::1", "127.0.0.1", "nothing.invalid")
         self.pin(d["id"])
         self.hosts({"::1": "wrong", "127.0.0.1": "ok"})
+        self.link.state["network"] = {"id": "n-home", "name": "Home", "tailscale": {"up": False}}
         self.link.test(d["id"])
         rows = {r["host"]: r for r in self.link.snapshot()["tests"][d["id"]]["rows"]}
         self.assertEqual(rows["127.0.0.1"]["ssh"], "ok")
         self.assertEqual(rows["::1"]["ssh"], "wrong")
         self.assertEqual(rows["nothing.invalid"]["state"], "unresolved")
+        done = self.link.snapshot()["tests"][d["id"]]
+        self.assertTrue(done["done"])
+        self.assertEqual(done["network"], "n-home")
+        # The address that passed has now worked on this network, so a reconnect tries it first:
+        # the page offers Use now from this order, which comes with the finished result.
+        self.assertEqual(done["order"][0], "127.0.0.1")
+        self.assertEqual(sorted(done["order"]), sorted(["::1", "127.0.0.1", "nothing.invalid"]))
         self.assertEqual(self.routes, [])
         self.assertTrue(all("ControlPath=none" in c for c in self.calls() if "-G" not in c))
 

ui/frame_link.py

@@ -1006,7 +1006,15 @@ class Link:
             t.start()
         for t in threads:
             t.join(40)
-        put(done=True, finished=now())
+        # The order a reconnect on this network would try them in, now that this test has
+        # recorded where they work: the page offers Use now only on the one it would pick.
+        try:
+            fresh = self.reg.get(device_id)["addresses"]
+        except frame_devices.DeviceError:
+            fresh = []
+        order = [a["host"] for a, _ in frame_devices.order_addresses(
+            fresh, net.get("id"), bool((net.get("tailscale") or {}).get("up")))]
+        put(done=True, finished=now(), network=net.get("id"), order=order)
         self.devices_changed()
 
 
@@ -1017,8 +1025,6 @@ def devices_view(link):
     snap = link.reg.snapshot()
     active = link.active_device()
     names = {nid: link.reg.network_name(dict(n, id=nid)) for nid, n in snap["networks"].items()}
-    net = link.state["network"] or {}
-    tailscale_up = bool((net.get("tailscale") or {}).get("up"))
     devices = []
     bare = link.bare(link.session_alias) if link.session_alias and not link.reg.by_alias(link.session_alias) else None
     for extra in ([active] if active.get("transient") and not active.get("none") else []) + \
@@ -1029,12 +1035,8 @@ def devices_view(link):
         view = {k: v for k, v in d.items() if k not in ("config_host", "addresses")}
         view["active"] = d["id"] == active["id"]
         view["pinned"] = frame_devices.pinned(d["id"])
-        # rank: where the next connection on this network tries it (0 first), so the page can
+        view["addresses"] = [dict(a, network_names=[names.get(n, "an unnamed network") for n in a["networks"]])
-        # tell which address a reconnect would pick.
+                             for a in d["addresses"]]
-        ranks = {a["host"]: i for i, (a, _) in
-                 enumerate(frame_devices.order_addresses(d["addresses"], net.get("id"), tailscale_up))}
-        view["addresses"] = [dict(a, network_names=[names.get(n, "an unnamed network") for n in a["networks"]],
-                                  rank=ranks[a["host"]]) for a in d["addresses"]]
         devices.append(view)
     return {"devices": devices, "active": active["id"], "network": link.state["network"],
             "networks": [dict(n, id=nid, display=names[nid]) for nid, n in snap["networks"].items()],

ui/index.html

@@ -4490,12 +4490,11 @@ function renderConnAddrs() {
   const refocus = cd.refocus ? [cd.refocus, "[data-ed]"]
     : held ? [held.dataset.host, ["[data-use]", "[data-mv='-1']", "[data-mv='1']", "[data-ed]", "[data-rm]"].find(q => document.activeElement.matches(q))] : null;
   cd.refocus = null;
-  // A reconnect picks the first-ranked address that answers: offer it when that's a tested
+  // A reconnect tries the addresses in the order the finished test reports for this network
-  // address other than the one in use.
+  // and picks the first that answers: offer Use now on that one, if it isn't the one in use.
-  const used = d.addresses.find(a => a.host === inUse);
+  const test = (s.tests || {})[d.id], order = test && test.done && test.network === (s.network || {}).id && test.order;
-  const best = used && d.addresses.filter(a => (tested.get(a.host) || {}).ssh === "ok")
+  const pick = order && order.find(h => (tested.get(h) || {}).ssh === "ok");
-    .reduce((x, a) => !x || a.rank < x.rank ? a : x, null);
+  const switchTo = pick && inUse && pick !== inUse && order.indexOf(inUse) > order.indexOf(pick) ? pick : null;
-  const switchTo = best && best.rank < used.rank ? best.host : null;
   list.innerHTML = d.addresses.map((a, i) => {
     if (cd.editing === a.host && cd.editDev === d.id) return `<div class="item addr"><div class="addr-edit conn-edit">
       <input type="text" id="ceHost" value="${esc(a.host)}" maxlength="253" aria-label="Address">