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

docs/devices.md

@@ -78,11 +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 the test
-finishes and the first address that passed would be tried before the one in use,
-that address gets **Use now**, which reconnects. A reconnect tries the addresses
-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.
+name still leads. A new or edited address is tested straight away. While
+connected, **Reconnect** tries the addresses again in their current order, so an
+address just added or moved up is used now rather than at the next connection.
+It doesn't pick a particular address: the first to answer in order wins, and a
+slow one loses to a later one.
 
 ## Networks
 

tests/test_link.py

@@ -449,22 +449,38 @@ 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))
 
+    def test_a_test_started_earlier_cant_overwrite_a_newer_one(self):
+        d = self.device("nothing.invalid")
+        release, calls = threading.Event(), []
+
+        def probe(host, port, update=None):
+            calls.append(host)
+            if len(calls) == 1:
+                release.wait(10)  # the first test is still probing when the second finishes
+                return {"state": "refused", "detail": "first test", "ip": None, "rtt_ms": None}
+            return {"state": "refused", "detail": "second test", "ip": None, "rtt_ms": None}
+
+        with mock.patch.object(fl, "probe", probe):
+            first = threading.Thread(target=self.link.test, args=(d["id"],))
+            first.start()
+            while not calls:
+                time.sleep(0.01)
+            self.link.test(d["id"])
+            self.assertEqual(self.link.snapshot()["tests"][d["id"]]["rows"][0]["detail"], "second test")
+            release.set()
+            first.join(10)
+        result = self.link.snapshot()["tests"][d["id"]]
+        self.assertTrue(result["done"])
+        self.assertEqual(result["rows"][0]["detail"], "second test")
+
     def test_switching_to_a_headset_that_never_answers_stops_using_the_last_one(self):
         self.device("localhost")
         self.hosts({"localhost": "ok"})

ui/frame_link.py

@@ -169,6 +169,7 @@ class Link:
         self.version = 0
         self.stopped = False
         self.kicks = []                 # reasons someone asked for a (re)connect
+        self.test_gen = {}              # device id -> its newest test of the addresses (see test())
         self.busy = False               # the loop is handling kicks
         self.state = {"phase": "idle", "reason": None, "device": None, "network": None, "stages": [],
                       "probes": [], "via": None, "error": None, "retry_at": None, "attempt": 0,
@@ -955,9 +956,13 @@ class Link:
         started = now()
         rows = [{"host": a["host"], "kind": a["kind"], "state": "waiting", "detail": "Waiting", "ip": None,
                  "rtt_ms": None, "ssh": None} for a in device["addresses"]]
+        with self.cond:
+            gen = self.test_gen[device_id] = self.test_gen.get(device_id, 0) + 1
 
         def put(**fields):
             with self.cond:
+                if gen != self.test_gen[device_id]:
+                    return  # a newer test has started: its results are the ones to show
                 self.state["tests"][device_id] = dict({"started": started, "done": False, "rows": rows}, **fields)
                 self.version += 1
                 self.cond.notify_all()
@@ -1006,15 +1011,7 @@ class Link:
             t.start()
         for t in threads:
             t.join(40)
-        # 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)
+        put(done=True, finished=now())
         self.devices_changed()
 
 

ui/index.html

@@ -4422,8 +4422,12 @@ function renderConnDlg() {
     : `${s.reason || "Connecting"}…`;
   // Folded while connected; opened when something's wrong. Left as the user set it otherwise.
   if (s.phase !== cd.phase) { $("connMore").open = !connected; cd.phase = s.phase; }
-  $("connRetryBtn").hidden = !failed;
+  // Connected, it's Reconnect: the addresses are tried again in their current order, so an
+  // address just added or moved up gets its chance now rather than at the next connection.
+  $("connRetryBtn").hidden = !failed && !connected;
   $("connRetryBtn").classList.toggle("action", failed);
+  $("connRetryBtn").textContent = failed ? "Retry now" : "Reconnect";
+  $("connRetryBtn").title = failed ? "" : "Try the addresses again, in order";
   $("connSetup").hidden = connected;
   const n = s.network || {}, ts = n.tailscale || {};
   const facts = [["Network", n.ssid || n.name || "—"],
@@ -4489,13 +4493,8 @@ function renderConnAddrs() {
   // same address (after an edit, on that row's Edit button).
   const held = list.contains(document.activeElement) && document.activeElement.closest("[data-host]");
   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;
+    : held ? [held.dataset.host, ["[data-mv='-1']", "[data-mv='1']", "[data-ed]", "[data-rm]"].find(q => document.activeElement.matches(q))] : null;
   cd.refocus = null;
-  // A reconnect tries the addresses in the order the finished test reports for this network
-  // and picks the first that answers: offer Use now on that one, if it isn't the one in use.
-  const test = (s.tests || {})[d.id], order = test && test.done && test.network === (s.network || {}).id && test.order;
-  const pick = order && order.find(h => (tested.get(h) || {}).ssh === "ok");
-  const switchTo = pick && inUse && pick !== inUse && order.indexOf(inUse) > order.indexOf(pick) ? pick : 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">
@@ -4509,7 +4508,6 @@ function renderConnAddrs() {
     return `<div class="item addr" data-host="${esc(a.host)}">
       <div class="grow"><div class="t">${esc(a.host)} <span class="tag">${esc(KIND_TAG[a.kind] || a.kind)}</span>${a.label ? ` <span class="sub">${esc(a.label)}</span>` : ""}</div>
         <div class="s">${result}</div></div>
-      ${a.host === switchTo ? `<button class="small" data-use title="Reconnect through this address">Use now</button>` : ""}
       <button class="small" data-mv="-1" title="Try earlier" aria-label="Move ${esc(a.host)} up"${i ? "" : " disabled"}>↑</button>
       <button class="small" data-mv="1" title="Try later" aria-label="Move ${esc(a.host)} down"${i < d.addresses.length - 1 ? "" : " disabled"}>↓</button>
       <button class="small" data-ed>Edit</button><button class="small danger" data-rm>Remove</button></div>`;
@@ -4526,8 +4524,6 @@ function renderConnAddrs() {
     row.querySelector("[data-ed]").onclick = () => {
       cd.editing = host; cd.editDev = d.id; cd.session++; renderConnAddrs(); $("ceHost").focus();
     };
-    const use = row.querySelector("[data-use]");
-    if (use) use.onclick = () => devAction(`Switch to ${host}`, { action: "retry", id: d.id }, use);
     row.querySelector("[data-rm]").onclick = e => {
       if (host === inUse && !confirm(`${host} is how Frame Control is reaching the headset now. Remove it?`)) return;
       devAction(`Remove ${host}`, { action: "address-remove", id: d.id, host }, e.currentTarget);