Devices: fixes from review round 8

- A batch of dropped files stays with the headset it was dropped on, and
  stops if the app switches.
- Removing or moving the address in use reroutes at once; a headset with no
  addresses reaches nothing rather than whatever ~/.ssh/config says.
- A late answer to an older device-list request is ignored.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
saphidandClaude Opus 5.5 committed 2026-09-28 22:45:34 +10:00
1 parent 51ef5d8283
commit 175c39a2b0
3 files changed
+37 -8

No files matched your search

+11
View File
@@ -241,6 +241,17 @@ class Connecting(unittest.TestCase):
self.assertIn("No headset", s["error"]["message"]) self.assertIn("No headset", s["error"]["message"])
self.assertEqual(self.routes[-1], ("frame-control-no-headset", ["-o", "HostName=no-headset.invalid"])) self.assertEqual(self.routes[-1], ("frame-control-no-headset", ["-o", "HostName=no-headset.invalid"]))
def test_a_headset_without_addresses_reaches_nothing(self):
d = self.device("localhost")
self.hosts({"localhost": "ok"})
self.link.connect(["start"])
fl.devices_action(self.link, {"action": "address-remove", "id": d["id"], "host": "localhost"}, None)
self.assertIn("HostName=no-address.invalid", self.routes[-1][1]) # at once, not after a retry
self.link.connect(["switch"])
s = self.link.snapshot()
self.assertEqual((s["phase"], s["retry_at"]), ("failed", None))
self.assertIn("no addresses", s["error"]["message"])
def test_a_bare_alias_lets_ssh_config_decide(self): def test_a_bare_alias_lets_ssh_config_decide(self):
self.link.override = "frame-bare" self.link.override = "frame-bare"
self.hosts({"frame-bare": "ok"}) # the stand-in ssh has no config: the alias is the host self.hosts({"frame-bare": "ok"}) # the stand-in ssh has no config: the alias is the host
+12 -4
View File
@@ -338,8 +338,10 @@ class Link:
"""What every ssh command adds to reach DEVICE at HOST.""" """What every ssh command adds to reach DEVICE at HOST."""
if device.get("none"): if device.get("none"):
return ["-o", "HostName=no-headset.invalid"] # fails at once, with ssh's own "can't resolve" return ["-o", "HostName=no-headset.invalid"] # fails at once, with ssh's own "can't resolve"
if device.get("transient") or not host: if device.get("transient"):
return [] return []
if not host: # a headset with no addresses: reach nothing, not whatever ~/.ssh/config says
return ["-o", "HostName=no-address.invalid"]
return ["-o", f"HostName={frame_devices.ssh_host(host)}", return ["-o", f"HostName={frame_devices.ssh_host(host)}",
"-o", f"HostKeyAlias={frame_devices.host_key_alias(device['id'])}", "-o", f"HostKeyAlias={frame_devices.host_key_alias(device['id'])}",
"-o", f"UserKnownHostsFile={frame_devices.known_hosts_opt(device['id'])}", "-o", "HashKnownHosts=no", "-o", f"UserKnownHostsFile={frame_devices.known_hosts_opt(device['id'])}", "-o", "HashKnownHosts=no",
@@ -454,7 +456,7 @@ class Link:
self.state.update(phase="connected", retry_at=None, error=None) self.state.update(phase="connected", retry_at=None, error=None)
else: else:
self.fails += 1 self.fails += 1
self.state.update(phase="failed", retry_at=None if device.get("none") else self.state.update(phase="failed", retry_at=None if device.get("none") or not (device.get("transient") or device["addresses"]) else
now() + RETRY[min(self.fails, len(RETRY)) - 1]) now() + RETRY[min(self.fails, len(RETRY)) - 1])
if not self.state["error"]: if not self.state["error"]:
self.state["error"] = {"stage": "find", "message": "Couldn't connect", "raw": ""} self.state["error"] = {"stage": "find", "message": "Couldn't connect", "raw": ""}
@@ -485,6 +487,9 @@ class Link:
if device.get("none"): if device.get("none"):
self.fail("find", "No headset is set up. Add one on the Devices tab.") self.fail("find", "No headset is set up. Add one on the Devices tab.")
return False return False
if not device.get("transient") and not device["addresses"]:
self.fail("find", f"{device['name']} has no addresses. Add one on the Devices tab.")
return False
# 1. this computer's network # 1. this computer's network
self.stage("network", "active") self.stage("network", "active")
net = frame_network.current_network(self.last_fp) net = frame_network.current_network(self.last_fp)
@@ -506,8 +511,7 @@ class Link:
# 2. find the headset # 2. find the headset
self.stage("find", "active") self.stage("find", "active")
port = device.get("port") or 22 port = device.get("port") or 22
bare = device.get("transient") or not device["addresses"] if device.get("transient"):
if bare:
host, port, user, proxied = ssh_g(device["alias"]) host, port, user, proxied = ssh_g(device["alias"])
if user and not device.get("user"): if user and not device.get("user"):
device["user"] = user device["user"] = user
@@ -951,9 +955,13 @@ def devices_action(link, body, open_setup, busy=lambda: 0):
elif action == "address-update": elif action == "address-update":
a = reg.update_address(did, body.get("host"), new_host=body.get("newHost"), kind=body.get("kind"), a = reg.update_address(did, body.get("host"), new_host=body.get("newHost"), kind=body.get("kind"),
label=body.get("label")) label=body.get("label"))
if is_active and a["host"] != body.get("host"):
link.invalidate() # the address in use may have moved
msg = f"Saved {a['host']}" msg = f"Saved {a['host']}"
elif action == "address-remove": elif action == "address-remove":
reg.remove_address(did, body.get("host")) reg.remove_address(did, body.get("host"))
if is_active:
link.invalidate() # it may be the address in use: stop using it now
msg = f"Removed {body.get('host')}" msg = f"Removed {body.get('host')}"
elif action == "address-move": elif action == "address-move":
delta = body.get("delta") delta = body.get("delta")
+14 -4
View File
@@ -1537,12 +1537,12 @@ window.addEventListener("drop", e => {
const dirs = new Set(items.map((it, i) => it.webkitGetAsEntry && it.webkitGetAsEntry()?.isDirectory ? i : -1).filter(i => i >= 0)); const dirs = new Set(items.map((it, i) => it.webkitGetAsEntry && it.webkitGetAsEntry()?.isDirectory ? i : -1).filter(i => i >= 0));
sendFiles([...e.dataTransfer.files], dirs); sendFiles([...e.dataTransfer.files], dirs);
}); });
function upload(file, mode) { function upload(file, mode, dev = window.connDevice) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const xhr = new XMLHttpRequest(); const xhr = new XMLHttpRequest();
xhr.open("POST", "/api/upload"); xhr.open("POST", "/api/upload");
xhr.setRequestHeader("X-Frame-UI", UI_KEY); xhr.setRequestHeader("X-Frame-UI", UI_KEY);
if (window.connDevice) xhr.setRequestHeader("X-Frame-Device", window.connDevice); if (dev) xhr.setRequestHeader("X-Frame-Device", dev);
xhr.setRequestHeader("X-Filename", encodeURIComponent(file.name)); xhr.setRequestHeader("X-Filename", encodeURIComponent(file.name));
xhr.setRequestHeader("X-Mode", mode); xhr.setRequestHeader("X-Mode", mode);
const bar = $("prog").firstElementChild; const bar = $("prog").firstElementChild;
@@ -1603,7 +1603,12 @@ $("apkAltClose").onclick = () => $("apkAltDlg").close();
const TITLE_EXT = /\.(zip|exe)$/i; const TITLE_EXT = /\.(zip|exe)$/i;
async function sendFiles(files, dirs = new Set()) { async function sendFiles(files, dirs = new Set()) {
const dev = window.connDevice; // the whole batch is for the headset it was dropped on
for (const [i, f] of files.entries()) { for (const [i, f] of files.entries()) {
if (window.connDevice !== dev) {
toast(`Switched headsets: didn't send ${files.length - i} remaining file${files.length - i === 1 ? "" : "s"}`, true);
break;
}
const path = window.frameApp?.pathForFile ? window.frameApp.pathForFile(f) : ""; const path = window.frameApp?.pathForFile ? window.frameApp.pathForFile(f) : "";
if (dirs.has(i)) { if (dirs.has(i)) {
if (path) await sideload(f, path, true); if (path) await sideload(f, path, true);
@@ -1613,7 +1618,7 @@ async function sendFiles(files, dirs = new Set()) {
if (!f.size) { toast(`${f.name}: empty files aren't supported`, true); continue; } if (!f.size) { toast(`${f.name}: empty files aren't supported`, true); continue; }
if (TITLE_EXT.test(f.name)) { await sideload(f, path); continue; } if (TITLE_EXT.test(f.name)) { await sideload(f, path); continue; }
const apk = f.name.toLowerCase().endsWith(".apk"); const apk = f.name.toLowerCase().endsWith(".apk");
await act(apk ? `Install ${f.name}` : `Copy ${f.name} to ~/Downloads`, () => upload(f, apk ? "apk" : "push")); await act(apk ? `Install ${f.name}` : `Copy ${f.name} to ~/Downloads`, () => upload(f, apk ? "apk" : "push", dev));
} }
$("fileInput").value = ""; $("fileInput").value = "";
refresh(); refresh();
@@ -2639,9 +2644,14 @@ $("connSetup").onclick = setUpActive;
// ---- Devices tab ---- // ---- Devices tab ----
PAGES.push("devices"); PAGES.push("devices");
let devicesSeq = 0; // an older answer arriving late mustn't bring back the old selection
async function loadDevices() { async function loadDevices() {
if (!link.live) return; if (!link.live) return;
try { dv.data = await api("/api/devices"); } catch (e) { return failed($("devList"), e); } const seq = ++devicesSeq;
let data;
try { data = await api("/api/devices"); } catch (e) { if (seq === devicesSeq) failed($("devList"), e); return; }
if (seq !== devicesSeq) return;
dv.data = data;
dv.list = dv.data.devices; dv.list = dv.data.devices;
if (!dv.list.some(d => d.id === dv.sel)) dv.sel = dv.data.active; if (!dv.list.some(d => d.id === dv.sel)) dv.sel = dv.data.active;
const sel = $("devSel"); const sel = $("devSel");