From ba33d2ff40cde893a784ebcac2bdf3116723fc51 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:24:46 +1000 Subject: [PATCH] Devices: fixes from review round 6 - The headset a change is meant for is checked and the work counted in one step, so a switch can't slip in between (uploads too). - A sideloaded title read on one headset can't be installed on another; open confirmations close on a switch. - The only headset can't be removed while its ssh alias stays behind. - Answers about the previous headset are dropped without touching panels; the catalogue's Installed tags are rebuilt for the new headset. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_link.py | 5 +++++ ui/frame_link.py | 5 +++++ ui/index.html | 15 ++++++++++++--- ui/server.py | 22 +++++++++++++--------- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/tests/test_link.py b/tests/test_link.py index c5a484c..ec4411a 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -354,6 +354,11 @@ class Connecting(unittest.TestCase): for body in bad: with self.assertRaises(fd.DeviceError, msg=body): fl.devices_action(self.link, body, open_setup=lambda *a: self.fail("setup ran")) + # The only headset, whose ssh alias would stay: not without removing that too. + (self.dir / "ssh" / "config").write_text("# >>> steam-frame (frame-t) >>>\nHost frame-t\n HostName localhost\n" + "Host *\n# <<< steam-frame (frame-t) <<<\n") + with self.assertRaises(fd.DeviceError): + fl.devices_action(self.link, {"action": "remove", "id": d["id"]}, None) opened = [] out = fl.devices_action(self.link, {"action": "setup", "alias": "frame-9", "host": "192.168.1.50"}, open_setup=lambda alias, host: opened.append((alias, host)) or "a terminal") diff --git a/ui/frame_link.py b/ui/frame_link.py index 9e3b18b..5ced6b9 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -904,6 +904,11 @@ def devices_action(link, body, open_setup, busy=lambda: 0): raise frame_devices.DeviceError(f"Saved, but couldn't update ~/.ssh/config: {e}") msg = f"Saved {d['name']}" elif action == "remove": + if not body.get("config") and len(reg.devices()) == 1 and reg.get(did)["alias"] in { + b["alias"] for b in frame_devices.parse_blocks(frame_devices.read_config())}: + # Its ssh alias would stay, and the app would go on using it as a bare alias. + raise frame_devices.DeviceError("This is your only headset. To remove it completely, also remove its " + "entry from ~/.ssh/config (the box below)") d = reg.remove_device(did) if is_active: link.override = None diff --git a/ui/index.html b/ui/index.html index 4fe10ae..23d54a5 100644 --- a/ui/index.html +++ b/ui/index.html @@ -923,6 +923,8 @@ const savesToDevice = () => !!(window.frameApp && window.frameApp.saveImages); // Bumped when the app moves to another headset: answers to requests made before // then are about the old one, so they're dropped rather than shown. let devGen = 0; +// Answers that don't depend on the headset (this computer's data, the catalogue, jobs). +const HEADSET_FREE = /^\/api\/(devices|connection|host|job|titles\/job|webinstall\/job|apk-versions|android\/(catalog|reports)|steam\/search)\b/; async function api(path, body) { const gen = devGen; // Changes name the headset the page was showing, so the server refuses one meant @@ -937,9 +939,9 @@ async function api(path, body) { : "Frame Control's local server isn't running. Restart the app (Frame → Restart Server)."); } const data = await r.json().catch(() => ({ error: `HTTP ${r.status}` })); - if (gen !== devGen && !path.startsWith("/api/devices") && !path.startsWith("/api/connection")) { + if (gen !== devGen && !HEADSET_FREE.test(path)) { const err = new Error("Switched headsets"); - err.offline = true; // panels show "Waiting for the Frame" and reload from the new one + err.offline = err.stale = true; // failed() leaves the panel alone: the new headset's answer fills it throw err; } if (!r.ok) { @@ -953,6 +955,7 @@ async function api(path, body) { // What a panel shows when its data couldn't be read: a quiet placeholder while // the Frame is unreachable (the banner explains), the real error otherwise. function failed(el, e) { + if (e.stale) return; // about the previous headset: leave the panel to the new one el.innerHTML = e.offline ? `
Waiting for the Frame
` : `
${esc(e.message)}
`; } async function act(label, fn, btn) { @@ -2553,6 +2556,11 @@ function onConnection(s) { // The lists behind those panels too, so filters can't bring the old ones back. gm.owned = null; gm.byId = new Map(); gm.results = []; gm.store = []; gm.storeQ = null; gm.shown = 0; gm.seq++; androidApps = []; shots.list = []; + titlesSeq++; disp.seq++; // answers to loads already on their way are ignored + if (cat.apps) filterCatalog(); // "Installed" tags were the other headset's + // Confirmations still open were checked against the other headset. + ["titleDlg", "apkAltDlg", "pwDlg", "repDlg"].forEach(id => { if ($(id) && $(id).open) $(id).close(); }); + if ($("wiDlg").open && !wi.job && !wi.starting) $("wiDlg").close(); $("dispSel").innerHTML = ""; $("dispSel").disabled = true; $("dispCtl").hidden = true; link.reload = true; // everything on the page was the other headset's $("battChip").hidden = true; @@ -2839,7 +2847,8 @@ function askRemove(d) { $("rmDevTitle").textContent = `Remove ${d.name}?`; $("rmDevText").textContent = `Frame Control forgets this headset, its ${d.addresses.length} address${d.addresses.length === 1 ? "" : "es"} and its saved identity. Its SSH keys stay.`; $("rmDevConfigText").textContent = `Also remove the '${d.alias}' entry from ~/.ssh/config (Terminal's ssh ${d.alias} stops working)`; - $("rmDevConfig").checked = false; $("rmMsg").textContent = ""; + // The only headset: its entry has to go too, or the app would go on using that alias. + $("rmDevConfig").checked = dv.list.filter(x => !x.transient).length === 1; $("rmMsg").textContent = ""; $("rmDevConfig").parentElement.hidden = !d.managed; $("rmCancel").onclick = () => $("rmDevDlg").close(); $("rmDevForm").onsubmit = async e => { diff --git a/ui/server.py b/ui/server.py index 9e319c3..9e406da 100755 --- a/ui/server.py +++ b/ui/server.py @@ -106,8 +106,13 @@ _work = [0] @contextlib.contextmanager -def working(): +def working(meant=None): + """Counts as running work. `meant`: the headset the page made this change for; if the + app has switched away from it, refuse (checked together with counting, so a switch + can't slip in between).""" with _work_lock: + if LINK and meant and meant != LINK.active_device()["id"]: + raise Failure("Frame Control switched headsets; try again on this one", 409) _work[0] += 1 try: yield @@ -689,7 +694,8 @@ def stage_title(path, temp_dir=None, name=None): raise token = secrets.token_hex(12) with _titles_lock: - _staged[token] = {"plan": plan, "dir": temp_dir, "time": time.time()} + _staged[token] = {"plan": plan, "dir": temp_dir, "time": time.time(), + "device": LINK.active_device()["id"] if LINK else None} return {"message": f"Read {plan['source']}: {plan['target']} with {plan['runtime_label']}", "token": token, "plan": frame_titles.public(plan)} @@ -730,6 +736,9 @@ def titles(body): if action == "discard": _drop_staged(entry) return {"message": "Discarded"} + if LINK and entry.get("device") != LINK.active_device()["id"]: + _drop_staged(entry) # it was checked against the other headset's titles + raise Failure("Frame Control switched headsets since this was read; drop the file again", 409) with _titles_lock: _title_jobs[token] = {"stage": "Starting", "fraction": 0, "done": False, "error": None, "message": None, "title": None, "time": time.time()} @@ -1468,15 +1477,10 @@ class Handler(BaseHTTPRequestHandler): if not self.local_request(): return path = urlparse(self.path).path - # A change the page made for a headset the app has since switched away from - # (its buttons were still showing): refuse it rather than do it to this one. meant = self.headers.get("X-Frame-Device") - if LINK and meant and path != "/api/devices" and meant != LINK.active_device()["id"]: - self.send_json({"error": "Frame Control switched headsets; try again on this one"}, 409) - return try: if path == "/api/upload": - with working(): + with working(meant): self.send_json(self.upload()) return handler = POST.get(path) @@ -1489,7 +1493,7 @@ class Handler(BaseHTTPRequestHandler): body = json.loads(self.rfile.read(length) or b"{}") if not isinstance(body, dict): raise Failure("request body must be a JSON object", 400) - with (contextlib.nullcontext() if path == "/api/devices" else working()): + with (contextlib.nullcontext() if path == "/api/devices" else working(meant)): result = handler(body) self.send_json(result) except Failure as e: