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) <noreply@anthropic.com>
2026-10-06 03:00:18 +02:00 · 2026-09-27 11:30:40 +10:00
1 parent 13187e8f6a
commit a910f83ac1
@@ -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() }
        }
@@ -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()
    }
@@ -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<Int, Error>?
    private var result: Result<Int, Error>?
    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<Int, Error>) {
@@ -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)
            }
@@ -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 {
@@ -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}`);
@@ -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"