From 43e19d17f6c98f5902b0ae40e6484f218321967e Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:55:14 +1000 Subject: [PATCH] Devices: fixes from review round 9 - A title waiting its turn to be read stays with the headset it was dropped on, and is dropped if the app switches meanwhile. - Probes still finishing from an earlier attempt can't overwrite the rows of a newer one. - The FRAME_ALIAS the server started with stays on the list after switching away, so it can be picked again. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_link.py | 16 ++++++++++++++++ ui/frame_link.py | 31 ++++++++++++++++++++++--------- ui/index.html | 16 ++++++++++------ 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/tests/test_link.py b/tests/test_link.py index f512f24..04155a9 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -252,6 +252,22 @@ class Connecting(unittest.TestCase): self.assertEqual((s["phase"], s["retry_at"]), ("failed", None)) self.assertIn("no addresses", s["error"]["message"]) + def test_switching_back_to_the_frame_alias_the_server_started_with(self): + self.link.override = self.link.session_alias = "frame-bare" + other = self.device("localhost") + ids = [d["id"] for d in fl.devices_view(self.link)["devices"]] + self.assertEqual(ids, ["alias-frame-bare", other["id"]]) + fl.devices_action(self.link, {"action": "use", "id": other["id"]}, None) + 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", [])) + + 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") + self.assertEqual(self.link.state["probes"][0], {"host": "b", "state": "waiting"}) + 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 diff --git a/ui/frame_link.py b/ui/frame_link.py index 5dbcefc..a1b61f6 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -153,6 +153,7 @@ class Link: def __init__(self, registry, *, env_alias, mux_base, control, apply, explain): self.reg = registry self.override = env_alias # FRAME_ALIAS, if set: the headset this server starts on + self.session_alias = env_alias # ...and stays selectable after switching away self.mux_base = list(mux_base) # ["ssh", "-o", "BatchMode=yes", ControlPath...] self.control = control # ControlPath, or None where ssh can't share connections self.apply = apply # apply(alias, host_opts): point every ssh command at the headset @@ -216,8 +217,10 @@ class Link: self.version += 1 self.cond.notify_all() - def probe_update(self, index, **fields): + def probe_update(self, index, attempt=None, **fields): with self.cond: + if attempt is not None and attempt != self.state["attempt"]: + return # a probe from an earlier attempt, still finishing: not this one's row if index < len(self.state["probes"]): self.state["probes"][index].update(fields) self.version += 1 @@ -270,9 +273,13 @@ class Link: self.state["phase"] != "connecting"), wait) def use(self, device_id): - """Switch to another headset.""" - self.reg.set_active(device_id) - self.override = None + """Switch to another headset: one from the registry, or back to FRAME_ALIAS.""" + if self.session_alias and device_id == self.bare(self.session_alias)["id"] \ + and not self.reg.by_alias(self.session_alias): + self.override = self.session_alias + else: + self.reg.set_active(device_id) + self.override = None self.invalidate() def invalidate(self): @@ -535,11 +542,13 @@ class Link: results = [None] * len(ranked) done = threading.Condition() + attempt_no = self.state["attempt"] + def run_probe(i, host): - res = probe(host, port, update=lambda **f: self.probe_update(i, **f)) + res = probe(host, port, update=lambda **f: self.probe_update(i, attempt_no, **f)) res.setdefault("ip", None) res.setdefault("rtt_ms", None) - self.probe_update(i, **{k: res[k] for k in ("state", "detail", "ip", "rtt_ms")}) + self.probe_update(i, attempt_no, **{k: res[k] for k in ("state", "detail", "ip", "rtt_ms")}) with done: if results[i] is None: # not already given up on results[i] = dict(res, t=time.monotonic()) @@ -888,8 +897,11 @@ def devices_view(link): active = link.active_device() names = {nid: link.reg.network_name(dict(n, id=nid)) for nid, n in snap["networks"].items()} devices = [] - if active.get("transient") and not active.get("none"): - devices.append(dict(link.public_device(active), active=True, addresses=[], managed=False, pinned=False)) + 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 []) + \ + ([bare] if bare and bare["id"] != active["id"] else []): + devices.append(dict(link.public_device(extra), active=extra["id"] == active["id"], addresses=[], + managed=False, pinned=False)) for d in snap["devices"]: view = {k: v for k, v in d.items() if k not in ("config_host", "addresses")} view["active"] = d["id"] == active["id"] @@ -916,7 +928,8 @@ def devices_action(link, body, open_setup, busy=lambda: 0): raise frame_devices.DeviceError( f"Wait for what's running on {active['name']} to finish (see the activity bar), then try again") if action == "use": - d = reg.get(did) + sa = link.session_alias + d = link.bare(sa) if sa and did == link.bare(sa)["id"] and not reg.by_alias(sa) else reg.get(did) link.use(did) msg = f"Switched to {d['name']}" elif action == "update": diff --git a/ui/index.html b/ui/index.html index 7f77b32..b14f94f 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1642,20 +1642,24 @@ function candLabel(c) { // Inspect (upload, or the local path in the app), confirm in the dialog, then install with progress. // One at a time, so a second drop doesn't take over the dialog of the first. let sideloadQueue = Promise.resolve(); -function sideload(...args) { - const run = sideloadQueue.then(() => sideloadOne(...args)); +function sideload(file, path, isDir = false) { + const dev = window.connDevice; // for the headset it was dropped on, even if it waits its turn + const run = sideloadQueue.then(() => { + if (window.connDevice !== dev) return toast(`Switched headsets: didn't add ${file.name}`, true); + return sideloadOne(file, path, isDir, dev); + }); sideloadQueue = run.catch(() => {}); return run; } -async function sideloadOne(file, path, isDir = false) { +async function sideloadOne(file, path, isDir = false, dev = window.connDevice) { const canPush = !isDir && file.size > 0; log(`Reading ${file.name}…`); let r; - try { r = path ? await api("/api/titles", { action: "inspect", path }) : await upload(file, "title"); } + try { r = path ? await api("/api/titles", { action: "inspect", path }) : await upload(file, "title", dev); } catch (e) { log(`${file.name}: ${e.message}`, "e"); if (canPush && confirm(`${file.name}: ${e.message}\n\nCopy it to ~/Downloads instead?`)) - await act(`Copy ${file.name} to ~/Downloads`, () => upload(file, "push")); + await act(`Copy ${file.name} to ~/Downloads`, () => upload(file, "push", dev)); return; } // Its own fresh list, so the dialog can say whether this replaces an installed title. @@ -1664,7 +1668,7 @@ async function sideloadOne(file, path, isDir = false) { const choice = await confirmTitle(r.plan, canPush, installed); if (choice === "push") { api("/api/titles", { action: "discard", token: r.token }).catch(() => {}); - await act(`Copy ${file.name} to ~/Downloads`, () => upload(file, "push")); + await act(`Copy ${file.name} to ~/Downloads`, () => upload(file, "push", dev)); return; } if (!choice) { api("/api/titles", { action: "discard", token: r.token }).catch(() => {}); return; }