From a910f83ac17f71faebfebe3f85b94d1ddbb75b2c Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Sun, 27 Sep 2026 11:30:40 +1000 Subject: [PATCH] Fix the iPhone review's findings - Cancelling (Change headset, Forget) invalidates the attempt in flight; a connection only becomes the app's once every step finished for it. - A server that stops while the tunnel opens is noticed (exit callbacks are synchronised and replayed), and the app isn't left showing a dead page. - The port is read only once the digits are complete, and range-checked. - Other versions in ~/.cache/frame-control stay unless untouched for 14 days, so a second phone or iPad isn't cut off. - The key is appended on its own line even if authorized_keys lacks a final newline. - The app checks every 20 s, and on returning to the foreground, that the SSH session still answers, and reconnects if not. - The ssh stand-in runs the command as its own process group and passes a TERM on to all of it, so a live-video ffmpeg stops with its stream. - Touch screens show the library's Play buttons (the rule now follows the base one); the failure screen only says it's retrying when it is; the page says the app reconnects rather than naming a desktop menu. Co-Authored-By: Claude Opus 5.5 (1M context) --- ios/FrameControl/App/AppModel.swift | 125 +++++++++++++----- ios/FrameControl/SSH/FrameLink.swift | 12 ++ ios/FrameControl/SSH/HeadsetServer.swift | 60 +++++++-- ios/FrameControl/Views/RootView.swift | 5 +- ios/FrameControlTests/FrameControlTests.swift | 2 + ui/index.html | 9 +- ui/local-bin/ssh | 15 ++- 7 files changed, 174 insertions(+), 54 deletions(-) diff --git a/ios/FrameControl/App/AppModel.swift b/ios/FrameControl/App/AppModel.swift index 323ff28..b019b5e 100644 --- a/ios/FrameControl/App/AppModel.swift +++ b/ios/FrameControl/App/AppModel.swift @@ -41,12 +41,12 @@ final class AppModel: ObservableObject { /// ~/.ssh/authorized_keys, record the Frame's host key, then connect with the key. func pair(host: String, user: String, password: String) async { guard let target = Self.parse(host: host, user: user) else { - phase = .failed("Enter the headset's address and user name.") + fail("Enter the headset's address and user name.", retry: false) return } - await teardown() - attempt += 1 + invalidate() let mine = attempt + await teardown() phase = .connecting("Signing in to \(target.host)") let pin = PinnedHostKey(expected: nil) do { @@ -55,8 +55,12 @@ final class AppModel: ObservableObject { guard mine == attempt else { return } phase = .connecting("Adding this \(deviceName)'s key") let line = authorizedKeysLine - try await link.check("umask 077; mkdir -p ~/.ssh && touch ~/.ssh/authorized_keys && " - + "(grep -qxF \(shellQuote(line)) ~/.ssh/authorized_keys || echo \(shellQuote(line)) >> ~/.ssh/authorized_keys)", + // A file whose last line has no newline would otherwise swallow the key. + let file = "~/.ssh/authorized_keys" + try await link.check("umask 077; mkdir -p ~/.ssh && touch \(file) && " + + "{ grep -qxF \(shellQuote(line)) \(file) || { " + + "[ -s \(file) ] && [ -n \"$(tail -c 1 \(file))\" ] && printf '\\n' >> \(file); " + + "printf '%s\\n' \(shellQuote(line)) >> \(file); }; }", "Couldn't add the key on the Frame") guard let seen = pin.seen else { throw FrameFailure("The Frame didn't show a host key") } UserDefaults.standard.set(seen, forKey: Self.hostKeyKey) @@ -64,7 +68,7 @@ final class AppModel: ObservableObject { settings = target } catch { guard mine == attempt else { return } - phase = .failed((error as? FrameFailure)?.message ?? FrameLink.describe(error, host: target.host)) + fail((error as? FrameFailure)?.message ?? FrameLink.describe(error, host: target.host), retry: false) return } await connect() @@ -74,7 +78,7 @@ final class AppModel: ObservableObject { /// The Frame's host key is recorded on this first connection. func useKey(host: String, user: String) async { guard let target = Self.parse(host: host, user: user) else { - phase = .failed("Enter the headset's address and user name.") + fail("Enter the headset's address and user name.", retry: false) return } UserDefaults.standard.removeObject(forKey: Self.hostKeyKey) @@ -107,6 +111,7 @@ final class AppModel: ObservableObject { /// Forget the headset: back to the pairing screen. The Frame keeps the key line; /// remove it from ~/.ssh/authorized_keys there to revoke this phone. func forget() async { + invalidate() await teardown() UserDefaults.standard.removeObject(forKey: Self.settingsKey) UserDefaults.standard.removeObject(forKey: Self.hostKeyKey) @@ -115,65 +120,116 @@ final class AppModel: ObservableObject { } func showSetup() { + invalidate() Task { await teardown() } phase = .setup } + /// Whether the failure screen is retrying on its own. + @Published private(set) var retrying = false + // MARK: connecting + /// Every connection attempt has a number; anything that finishes after a newer + /// attempt started (or the user went back to setup) closes what it made and stops. + private func invalidate() { + attempt += 1 + retrying = false + } + /// quiet: a background retry, which leaves the failure screen up until it works. func connect(quiet: Bool = false) async { guard let settings else { phase = .setup return } - await teardown() - attempt += 1 + invalidate() let mine = attempt - func step(_ s: String) { if mine == attempt && !quiet { phase = .connecting(s) } } + await teardown() + func current() -> Bool { mine == attempt } + func step(_ s: String) { if current() && !quiet { phase = .connecting(s) } } step("Connecting to \(settings.host)") + var link: FrameLink? + var forwarder: PortForwarder? do { let bundle = try HeadsetServer.Bundle.fromApp() let auth = SSHAuthenticationMethod.ed25519(username: settings.user, privateKey: DeviceKey.loadOrCreate()) let pin = PinnedHostKey(expected: hostKey) - let link = try await FrameLink.connect(settings, auth: auth, hostKey: pin) - guard mine == attempt else { await link.close(); return } - self.link = link + let l = try await FrameLink.connect(settings, auth: auth, hostKey: pin) + link = l + guard current() else { throw CancellationError() } if hostKey == nil, let seen = pin.seen { UserDefaults.standard.set(seen, forKey: Self.hostKeyKey) } - let dir = try await HeadsetServer.deploy(bundle, over: link) { s in Task { @MainActor in step(s) } } + let dir = try await HeadsetServer.deploy(bundle, over: l) { s in Task { @MainActor in step(s) } } + guard current() else { throw CancellationError() } step("Starting Frame Control on the headset") let key = Self.randomKey() - let server = try await HeadsetServer.start(in: dir, over: link, key: key, device: deviceName) + let server = try await HeadsetServer.start(in: dir, over: l, key: key, device: deviceName) + guard current() else { throw CancellationError() } + let f = try await PortForwarder.start(over: l, to: server.port) + forwarder = f + guard current() else { throw CancellationError() } + // Only now does this attempt's connection become the app's. + self.link = l self.server = server - let forwarder = try await PortForwarder.start(over: link, to: server.port) - self.forwarder = forwarder - guard mine == attempt else { return } - server.onExit = { [weak self] tail in + self.forwarder = f + readySince = Date() + // Runs at once if the server already stopped while the tunnel was opening. + server.whenExited { [weak self] tail in Task { @MainActor in self?.lost(mine, "Frame Control on the headset stopped. \(tail.suffix(200))") } } - readySince = Date() - phase = .ready(URL(string: "http://127.0.0.1:\(forwarder.localPort)/?key=\(key)")!) + guard server.exited == nil else { return } + phase = .ready(URL(string: "http://127.0.0.1:\(f.localPort)/?key=\(key)")!) + watchHealth(mine) } catch { - guard mine == attempt else { return } - await teardown() - phase = .failed((error as? FrameFailure)?.message ?? FrameLink.describe(error, host: settings.host)) - // Keep trying quietly while the app is open: the Frame may just be asleep. - // A new task each time, so retrying for hours doesn't nest awaits. - Task { [weak self] in - try? await Task.sleep(nanoseconds: 10_000_000_000) - guard let self, mine == self.attempt, case .failed = self.phase, - UIApplication.shared.applicationState == .active else { return } - await self.connect(quiet: true) + forwarder?.stop() + if let link { await link.close() } // ends its server too + guard current(), !(error is CancellationError) else { return } + fail((error as? FrameFailure)?.message ?? FrameLink.describe(error, host: settings.host), retry: true) + } + } + + private func fail(_ message: String, retry: Bool) { + phase = .failed(message) + retrying = retry && settings != nil + guard retrying else { return } + // Keep trying quietly while the app is open: the Frame may just be asleep. + // A new task each time, so retrying for hours doesn't nest awaits. + let mine = attempt + Task { [weak self] in + try? await Task.sleep(nanoseconds: 10_000_000_000) + guard let self, mine == self.attempt, case .failed = self.phase, + UIApplication.shared.applicationState == .active else { return } + await self.connect(quiet: true) + } + } + + /// While connected, check every 20 s that the SSH session still answers: a + /// network change can leave it looking open while nothing gets through. + private func watchHealth(_ mine: Int) { + Task { [weak self] in + while true { + try? await Task.sleep(nanoseconds: 20_000_000_000) + guard let self, mine == self.attempt, case .ready = self.phase else { return } + if UIApplication.shared.applicationState != .active { continue } + if await !(self.link?.answers() ?? false) { + guard mine == self.attempt else { return } + await self.connect(quiet: true) + return + } } } } /// Called when the app comes back to the foreground: iOS may have dropped the - /// connection while it was in the background. + /// connection, or left it looking open, while it was in the background. func resume() { switch phase { case .ready: - if link?.isConnected != true || server?.exited != nil { Task { await connect() } } + let mine = attempt + Task { + let ok = await link?.answers() ?? false + if (!ok || server?.exited != nil), mine == attempt { await connect() } + } case .failed: if settings != nil { Task { await connect() } } default: @@ -187,8 +243,9 @@ final class AppModel: ObservableObject { guard which == attempt, case .ready = phase else { return } // Restart it once; if it dies again straight away, say so instead of looping. if Date().timeIntervalSince(readySince) < 20 { + invalidate() Task { await teardown() } - phase = .failed(why) + fail(why, retry: false) } else { Task { await connect() } } diff --git a/ios/FrameControl/SSH/FrameLink.swift b/ios/FrameControl/SSH/FrameLink.swift index 28c4455..12f4e84 100644 --- a/ios/FrameControl/SSH/FrameLink.swift +++ b/ios/FrameControl/SSH/FrameLink.swift @@ -80,6 +80,18 @@ final class FrameLink: @unchecked Sendable { var isConnected: Bool { client.isConnected } + /// Whether the Frame answers a trivial command within a few seconds. + func answers(within seconds: Double = 6) async -> Bool { + guard client.isConnected else { return false } + return await withTaskGroup(of: Bool.self) { group in + group.addTask { (try? await self.run("true").status) == 0 } + group.addTask { try? await Task.sleep(nanoseconds: UInt64(seconds * 1e9)); return false } + let first = await group.next() ?? false + group.cancelAll() + return first + } + } + func close() async { try? await client.close() } diff --git a/ios/FrameControl/SSH/HeadsetServer.swift b/ios/FrameControl/SSH/HeadsetServer.swift index 43d23ef..d273ee2 100644 --- a/ios/FrameControl/SSH/HeadsetServer.swift +++ b/ios/FrameControl/SSH/HeadsetServer.swift @@ -10,12 +10,31 @@ final class HeadsetServer: @unchecked Sendable { let port: Int private let lock = NSLock() private var _exited: String? + private var onExit: (@Sendable (String) -> Void)? /// Set once the server stops, with its last output. var exited: String? { lock.withLock { _exited } } - var onExit: (@Sendable (String) -> Void)? private init(port: Int) { self.port = port } + /// Calls back once when the server stops, at once if it already has. + func whenExited(_ callback: @escaping @Sendable (String) -> Void) { + let already: String? = lock.withLock { + if _exited == nil { onExit = callback } + return _exited + } + if let already { callback(already) } + } + + fileprivate func markExited(_ tail: String) { + let callback: (@Sendable (String) -> Void)? = lock.withLock { + guard _exited == nil else { return nil } + _exited = tail + defer { onExit = nil } + return onExit + } + callback?(tail) + } + static let cacheDir = ".cache/frame-control" struct Bundle { @@ -50,8 +69,11 @@ final class HeadsetServer: @unchecked Sendable { try await link.check("rm -rf \(dir).tmp && mkdir \(dir).tmp && tar xzf \(archive) -C \(dir).tmp && rm -f \(archive) " + "&& rm -rf \(dir) && mv \(dir).tmp \(dir)", "Couldn't unpack Frame Control on the headset") } - // Older versions: only this one is used from now on. - _ = try? await link.run("cd \(cacheDir) && for d in */; do [ \"${d%/}\" = \(bundle.version) ] || rm -rf -- \"$d\"; done") + // Another phone or iPad may be running a different version right now, so only + // versions (and interrupted unpacks) untouched for two weeks go. This one is + // marked as used. + _ = try? await link.run("touch \(dir) && find \(cacheDir) -mindepth 1 -maxdepth 1 -type d ! -name \(bundle.version) " + + "-mtime +14 -exec rm -rf {} +") return dir } @@ -83,17 +105,16 @@ final class HeadsetServer: @unchecked Sendable { reader.cancel() throw error } - box.onEnd = { [weak server] tail in - guard let server else { return } - server.lock.withLock { server._exited = tail } - server.onExit?(tail) - } + box.whenEnded { [weak server] tail in server?.markExited(tail) } return server } + /// The port from the server's first line. Output arrives in chunks, so the digits + /// only count once something follows them (the line goes on after the port). static func port(in text: String) -> Int? { - guard let r = text.range(of: #"Frame Control on http://127\.0\.0\.1:(\d+)"#, options: .regularExpression) else { return nil } - return Int(text[r].split(separator: ":").last ?? "") + guard let r = text.range(of: #"Frame Control on http://127\.0\.0\.1:[0-9]+\s"#, options: .regularExpression), + let port = Int(text[r].dropLast().split(separator: ":").last ?? ""), (1...65535).contains(port) else { return nil } + return port } } @@ -103,17 +124,28 @@ private final class PortWaiter: @unchecked Sendable { private var continuation: CheckedContinuation? private var result: Result? private var endedTail: String? - var onEnd: (@Sendable (String) -> Void)? { - didSet { if let tail = lock.withLock({ endedTail }) { onEnd?(tail) } } + private var onEnd: (@Sendable (String) -> Void)? + + /// Calls back when the output ends, at once if it already has. + func whenEnded(_ callback: @escaping @Sendable (String) -> Void) { + let already: String? = lock.withLock { + if endedTail == nil { onEnd = callback } + return endedTail + } + if let already { callback(already) } } func found(_ port: Int) { finish(.success(port)) } func ended(_ text: String) { let tail = String(text.suffix(600)).trimmingCharacters(in: .whitespacesAndNewlines) - lock.withLock { endedTail = tail } + let callback: (@Sendable (String) -> Void)? = lock.withLock { + endedTail = tail + defer { onEnd = nil } + return onEnd + } finish(.failure(FrameFailure("Frame Control's server on the headset stopped: \(tail.isEmpty ? "no output" : tail)"))) - onEnd?(tail) + callback?(tail) } private func finish(_ r: Result) { diff --git a/ios/FrameControl/Views/RootView.swift b/ios/FrameControl/Views/RootView.swift index 15b938c..0d628bf 100644 --- a/ios/FrameControl/Views/RootView.swift +++ b/ios/FrameControl/Views/RootView.swift @@ -12,7 +12,7 @@ struct RootView: View { case .connecting(let step): ConnectingView(step: step, host: model.settings?.host) { model.showSetup() } case .failed(let message): - FailedView(message: message, canRetry: model.settings != nil, + FailedView(message: message, canRetry: model.settings != nil, retrying: model.retrying, retry: { Task { await model.connect() } }, change: { model.showSetup() }) case .ready(let url): WebShell(url: url, model: model).ignoresSafeArea() @@ -50,6 +50,7 @@ struct ConnectingView: View { struct FailedView: View { let message: String let canRetry: Bool + let retrying: Bool let retry: () -> Void let change: () -> Void @@ -58,7 +59,7 @@ struct FailedView: View { Image(systemName: "wifi.exclamationmark").font(.system(size: 44)).foregroundStyle(.orange) Text("Can't reach the Frame").font(.title3.bold()) Text(message).multilineTextAlignment(.center).foregroundStyle(Color.frameMuted) - if canRetry { Text("Trying again every few seconds.").font(.footnote).foregroundStyle(Color.frameMuted) } + if retrying { Text("Trying again every few seconds.").font(.footnote).foregroundStyle(Color.frameMuted) } if canRetry { Button("Try again", action: retry).buttonStyle(.borderedProminent).controlSize(.large) } diff --git a/ios/FrameControlTests/FrameControlTests.swift b/ios/FrameControlTests/FrameControlTests.swift index d1d5a99..2e90496 100644 --- a/ios/FrameControlTests/FrameControlTests.swift +++ b/ios/FrameControlTests/FrameControlTests.swift @@ -29,6 +29,8 @@ final class HeadsetServerTests: XCTestCase { func testReadsThePortTheServerPrints() { XCTAssertEqual(HeadsetServer.port(in: "Frame Control on http://127.0.0.1:41234 (alias: frame; Ctrl-C to stop)\n"), 41234) XCTAssertNil(HeadsetServer.port(in: "Traceback (most recent call last):")) + XCTAssertNil(HeadsetServer.port(in: "Frame Control on http://127.0.0.1:4")) // more digits may follow + XCTAssertNil(HeadsetServer.port(in: "Frame Control on http://127.0.0.1:99999 ")) } func testBundleIsInTheApp() throws { diff --git a/ui/index.html b/ui/index.html index c97603d..e980e69 100644 --- a/ui/index.html +++ b/ui/index.html @@ -68,8 +68,6 @@ .mobile-only { display: none; } body.mobile .mobile-only { display: revert; } body.mobile .desk-only { display: none; } - /* Touch screens can't hover: keep the library's name and Play button showing. */ - @media (hover: none) { .capsule .over { opacity: 1; } .capsule:hover { transform: none; } } /* ---- pages: one per tab; the header nav switches between them ---- */ @@ -322,6 +320,8 @@ .disp .seg button { padding: 0 10px; } .disp input[type=number] { width: 72px; background: rgba(0,0,0,.28); color: var(--text); border: 1px solid transparent; border-radius: 3px; height: 32px; padding: 0 8px; font: inherit; font-size: 13px; } + /* Touch screens can't hover: keep the library's name and Play button showing. */ + @media (hover: none) { .capsule .over { opacity: 1; } .capsule:hover { transform: none; } } /* ---- phones: tabs move to a bottom bar, as in iOS apps; the page clears the notch and home indicator ---- */ @media (max-width: 640px) { /* No blur here: it would make the header the containing block of the fixed tab bar. */ @@ -798,7 +798,10 @@ async function api(path, body) { method: "POST", headers: {"Content-Type": "application/json", "X-Frame-UI": UI_KEY}, body: JSON.stringify(body) }; let r; try { r = await fetch(path, opts); } - catch { throw new Error("Frame Control's local server isn't running. Restart the app (Frame → Restart Server)."); } + catch { + throw new Error(window.frameApp?.platform === "ios" ? "Lost the connection to the Frame. The app reconnects on its own." + : "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 (!r.ok) { const err = new Error(data.error || `HTTP ${r.status}`); diff --git a/ui/local-bin/ssh b/ui/local-bin/ssh index a1733bb..f73b5bf 100755 --- a/ui/local-bin/ssh +++ b/ui/local-bin/ssh @@ -13,4 +13,17 @@ done [ $# -gt 0 ] && shift # the host alias cd "$HOME" || exit 255 [ $# -eq 0 ] && exit 0 # -N: nothing to run -exec "${SHELL:-/bin/sh}" -c "$*" +# Stopping ssh ends everything the command started (sshd hangs up the session). +# To do the same, run COMMAND as its own process group and pass on a TERM, HUP +# or INT to all of it, so e.g. a live-video ffmpeg can't outlive its stream. +set -m # job control: the background job gets its own process group, and keeps stdin +"${SHELL:-/bin/sh}" -c "$*" & +child=$! +trap 'kill -TERM -- "-$child" 2>/dev/null' TERM HUP INT +wait "$child" +status=$? +while kill -0 "$child" 2>/dev/null; do # a trapped signal ends `wait` early + wait "$child" + status=$? +done +exit "$status"