From 0bb2f9eb03820e489e57c1de95fe4d052361b267 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:54:16 +1000 Subject: [PATCH 1/2] Offer Use now from the finished test's own order The fix review found Use now could still point the wrong way: the button was worked out from per-address ranks in the devices list while the test was still running (a better-ranked address might yet answer), and the test's successes changed those ranks before the list was reloaded. The test now finishes by publishing the order a reconnect on this network would try, worked out after it has recorded where each address works, together with the network it ran on. The page offers Use now only once the test is done, only for that network, and only on the first address in that order that passed. The ranks in the devices list are gone again. docs/devices.md no longer promises where a reconnect lands: a slow address loses to a later one. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/devices.md | 9 +++++---- tests/test_link.py | 21 ++++++++------------- ui/frame_link.py | 20 +++++++++++--------- ui/index.html | 11 +++++------ 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/docs/devices.md b/docs/devices.md index ac8e9e5..d398f2c 100644 --- a/docs/devices.md +++ b/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 -that would be tried before the one in use passes the test, it gets **Use now**, -which reconnects; the reconnect picks the first address that answers, which is -that one. Otherwise the change takes effect the next time Frame Control connects. +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. ## Networks diff --git a/tests/test_link.py b/tests/test_link.py index 85d1802..584b43e 100644 --- a/tests/test_link.py +++ b/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)) diff --git a/ui/frame_link.py b/ui/frame_link.py index 46a01b5..09d11b9 100644 --- a/ui/frame_link.py +++ b/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 - # tell which address a reconnect would pick. - 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"]] + view["addresses"] = [dict(a, network_names=[names.get(n, "an unnamed network") for n in a["networks"]]) + 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()], diff --git a/ui/index.html b/ui/index.html index 75df008..f2d92f2 100644 --- a/ui/index.html +++ b/ui/index.html @@ -4406,12 +4406,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 - // address other than the one in use. - const used = d.addresses.find(a => a.host === inUse); - const best = used && d.addresses.filter(a => (tested.get(a.host) || {}).ssh === "ok") - .reduce((x, a) => !x || a.rank < x.rank ? a : x, null); - const switchTo = best && best.rank < used.rank ? best.host : 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 `
From 8e8c24f37054e34fb7e573ef3162f265dc8d4dfc Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:02:53 +1000 Subject: [PATCH 2/2] Don't break startup at Games links, and reload titles in the iPhone app too The fix review found the Games reload added to showPage() read `link`, which the page declares later: opening Frame Control at #games, #library, #sideloaded or #getgames threw at startup and skipped the rest of the setup. And the iPhone app never watches the connection, so `link.s` stayed null and titles still never reloaded there. The reload now runs only when navigating to Games (the hashchange), where the first load is already done, and doesn't depend on connection state; a failed load shows its error in the list as Refresh did. tests/test_page_startup.py runs the real showPage() at every page and section link with everything declared later still uninitialised, and fails on the old line. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_page_startup.py | 55 ++++++++++++++++++++++++++++++++++++++ ui/index.html | 7 ++--- 2 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 tests/test_page_startup.py diff --git a/tests/test_page_startup.py b/tests/test_page_startup.py new file mode 100644 index 0000000..69bd5c1 --- /dev/null +++ b/tests/test_page_startup.py @@ -0,0 +1,55 @@ +"""The page calls showPage() while its first script is still running, before names declared +later (in that script or the second one) exist. Touching one of them there throws, and the +rest of the page's setup never runs: opening Frame Control at #games did exactly that.""" +import json +import pathlib +import re +import shutil +import subprocess +import unittest + +ROOT = pathlib.Path(__file__).resolve().parents[1] +STUBS = ['$', 'toggleLive', 'scrollToY', 'loadMacView', 'loadPanels', 'loadPanelSwitcher', 'loadTitles'] + +RUN = r''' +const vm = require('vm'); +const {code, hashes} = JSON.parse(require('fs').readFileSync(0, 'utf8')); +const failures = []; +for (const hash of hashes) { + const context = vm.createContext({ + location: {hash}, document: {querySelectorAll: () => [], title: ''}, live: false, + }); + try { vm.runInContext(code, context); } + catch (e) { failures.push(`${hash}: ${e.message}`); } +} +console.log(JSON.stringify(failures)); +''' + + +@unittest.skipUnless(shutil.which('node'), 'Node runs the page code') +class PageStartup(unittest.TestCase): + def test_opening_any_page_or_section_at_startup_runs(self): + page = (ROOT / 'ui/index.html').read_text(encoding='utf-8') + first, second = re.findall(r'', page, re.S)[:2] + tables = first[first.index('const PAGES ='):first.index('let page =')] + start = first.index('function showPage(') + show = first[start:first.index('\n}\n', start) + 3] + # Everything declared after the startup call is still uninitialised when it runs. + call = re.search(r'^showPage\(\);', first, re.M).end() + later = re.findall(r'^(?:const|let)\s+(\w+)', first[call:] + second, re.M) + later = [n for n in dict.fromkeys(later) if n not in STUBS and n not in ('PAGES', 'SECTION_PAGE', 'page')] + self.assertIn('link', later) # the name that broke #games + stubs = ''.join(f'function {n}() {{ return {{ classList: {{ toggle() {{}} }}, scrollIntoView() {{}} }}; }}\n' + for n in STUBS if n != '$') + code = ('const $ = id => id === "nowhere" ? null : { classList: { toggle() {} }, scrollIntoView() {} };\n' + stubs + tables + 'let page = "home";\n' + show + + 'showPage();\n' + ''.join(f'let {n};\n' for n in later)) + hashes = ['', '#home', '#games', '#android', '#tools', '#settings', '#devices', '#nowhere'] + hashes += ['#' + k for k in re.findall(r'(\w+): "', tables)] + r = subprocess.run(['node', '-e', RUN], input=json.dumps({'code': code, 'hashes': hashes}), + capture_output=True, text=True) + self.assertEqual(r.returncode, 0, r.stderr) + self.assertEqual(json.loads(r.stdout), []) + + +if __name__ == '__main__': + unittest.main() diff --git a/ui/index.html b/ui/index.html index d924968..707867a 100644 --- a/ui/index.html +++ b/ui/index.html @@ -3827,7 +3827,7 @@ const SECTION_PAGE = { view: "home", device: "home", shots: "home", comfort: "ho transfer: "tools", apps: "tools", power: "tools", macview: "tools", media: "tools", panels: "tools", privacy: "settings", artworkSettings: "settings", assistant: "settings" }; let page = "home"; -function showPage() { +function showPage(e) { const id = location.hash.slice(1); const next = PAGES.includes(id) ? id : SECTION_PAGE[id] || "home"; if (next !== page && page === "home" && live) toggleLive(false); // don't stream video nobody is watching @@ -3840,8 +3840,9 @@ function showPage() { const section = !PAGES.includes(id) && id && $(id); if (section) section.scrollIntoView(); else scrollToY(0); if (page === "tools") { loadMacView(); loadPanels(); } - // Another computer may have added or removed a title: phones have no Refresh button. - if (page === "games" && prev !== "games" && link.s && link.s.phase === "connected") loadTitles(); + // Another computer may have added or removed a title: phones have no Refresh button. Only + // on navigating here (e: the hashchange); at startup the first load is already on its way. + if (e && page === "games" && prev !== "games") loadTitles(); } window.addEventListener("hashchange", showPage); // While a text field has focus, phones hide the bottom tab bar (see body.typing in the CSS).