Make broadcast paste immune to a wedged Tailscale daemon

Sources/CopyCat/Broadcast.swift

@@ -41,6 +41,21 @@ enum Broadcast {
         let outPath = compressIfNeeded(src: pngPath, stamp: stamp, dir: cacheDir, log: Log.cmdOptV)
         let outName = outPath.lastPathComponent
 
+        // Type the moment the local file exists — before peer discovery. The
+        // typed string doesn't depend on which hosts are reachable, and the
+        // discovery call can hang for tens of seconds when tailscaled is
+        // starved (UDP-blocked networks); the user's paste must never wait on
+        // the network. Typing already races the uploads on the remote side,
+        // so this ordering widens no window that wasn't open before.
+        //
+        // Remote home dir is unknown, so always tilde-prefix. Trailing space
+        // matches the local-paste path so the user can keep typing.
+        let typed = "~/\(remoteCacheRel)/\(outName) "
+        DispatchQueue.main.async {
+            Typer.type(typed)
+        }
+        Log.cmdOptV.info("typed \(typed)")
+
         let sizeBytes = (try? FileManager.default.attributesOfItem(atPath: outPath.path)[.size] as? Int) ?? 0
         let sizeKB = sizeBytes / 1024
 
@@ -63,14 +78,6 @@ enum Broadcast {
             )
         }
 
-        // Remote home dir is unknown, so always tilde-prefix. Trailing space
-        // matches the local-paste path so the user can keep typing.
-        let typed = "~/\(remoteCacheRel)/\(outName) "
-        DispatchQueue.main.async {
-            Typer.type(typed)
-        }
-        Log.cmdOptV.info("typed \(typed)")
-
         BroadcastStatus.shared.recordRun(hosts: hosts)
 
         // Prune off the SSH-fanout path on a lower-priority queue so
@@ -190,14 +197,49 @@ struct ShellResult: Sendable {
     let stdout: String
     let stderr: String
     let exitCode: Int32
+    /// True when the watchdog killed the process for exceeding `timeout`.
+    /// `exitCode` is the signal number in that case, so `isSuccess` is
+    /// already false — this flag exists so callers can report "timed out"
+    /// instead of a misleading "exited non-zero".
+    let timedOut: Bool
 
     var isSuccess: Bool { exitCode == 0 }
 }
 
+// Process isn't Sendable, but the watchdog only calls terminate(), which is
+// safe from another thread: it forwards SIGTERM and is a no-op once the
+// process has exited.
+private final class ProcessTerminator: @unchecked Sendable {
+    private let process: Process
+    private let lock = NSLock()
+    private var fired = false
+
+    init(_ process: Process) { self.process = process }
+
+    func terminate() {
+        lock.lock()
+        fired = true
+        lock.unlock()
+        process.terminate()
+    }
+
+    var didFire: Bool {
+        lock.lock()
+        defer { lock.unlock() }
+        return fired
+    }
+}
+
 /// Runs a process with stdin detached, capturing stdout and stderr separately
 /// so callers can distinguish "no output" from "failed silently". Returns
 /// `nil` only when the process couldn't be spawned at all (bad path, etc).
-func runShell(_ exec: String, args: [String], env: [String: String]? = nil) -> ShellResult? {
+///
+/// `timeout` bounds the total run: past it the process gets SIGTERM, the
+/// pipe reads unblock on EOF, and the result comes back with `timedOut` set.
+/// Without it a hung child blocks the calling thread indefinitely.
+func runShell(
+    _ exec: String, args: [String], env: [String: String]? = nil, timeout: TimeInterval? = nil
+) -> ShellResult? {
     let p = Process()
     p.executableURL = URL(fileURLWithPath: exec)
     p.arguments = args
@@ -209,15 +251,33 @@ func runShell(_ exec: String, args: [String], env: [String: String]? = nil) -> S
     p.standardInput = FileHandle(forReadingAtPath: "/dev/null")
     do {
         try p.run()
-        let outData = outPipe.fileHandleForReading.readDataToEndOfFile()
-        let errData = errPipe.fileHandleForReading.readDataToEndOfFile()
-        p.waitUntilExit()
-        return ShellResult(
-            stdout: String(data: outData, encoding: .utf8) ?? "",
-            stderr: String(data: errData, encoding: .utf8) ?? "",
-            exitCode: p.terminationStatus
-        )
     } catch {
         return nil
     }
+
+    var watchdog: DispatchWorkItem?
+    var terminator: ProcessTerminator?
+    if let timeout {
+        let t = ProcessTerminator(p)
+        let item = DispatchWorkItem { t.terminate() }
+        DispatchQueue.global(qos: .utility).asyncAfter(deadline: .now() + timeout, execute: item)
+        terminator = t
+        watchdog = item
+    }
+
+    let outData = outPipe.fileHandleForReading.readDataToEndOfFile()
+    let errData = errPipe.fileHandleForReading.readDataToEndOfFile()
+    p.waitUntilExit()
+    watchdog?.cancel()
+
+    // didFire alone can mislabel a natural exit: the watchdog may fire in the
+    // gap between exit and cancel. Requiring uncaughtSignal confirms the kill
+    // actually took the process down.
+    let timedOut = (terminator?.didFire ?? false) && p.terminationReason == .uncaughtSignal
+    return ShellResult(
+        stdout: String(data: outData, encoding: .utf8) ?? "",
+        stderr: String(data: errData, encoding: .utf8) ?? "",
+        exitCode: p.terminationStatus,
+        timedOut: timedOut
+    )
 }

Sources/CopyCat/SettingsView.swift

@@ -368,9 +368,10 @@ private struct HostsSettingsView: View {
 
     private func refreshPeers() {
         // tailscale status --json takes ~300-500ms; keep it off the main thread
-        // so opening the Settings window doesn't stall.
+        // so opening the Settings window doesn't stall. Uncached on purpose:
+        // this backs an explicit "Refresh peers" action.
         Task.detached(priority: .userInitiated) {
-            let fresh = TailscaleDiscovery.allPeers()
+            let fresh = TailscaleDiscovery.refreshPeers()
             await MainActor.run { peers = fresh }
         }
     }

Sources/CopyCat/TailscaleDiscovery.swift

@@ -7,6 +7,44 @@ struct TailscalePeer: Identifiable, Hashable, Sendable {
     let os: String?
 }
 
+// Single-flight TTL cache for the peer list. Callers sit on latency-critical
+// paths — the paste pipeline and menu rendering — and each uncached lookup
+// spawns a subprocess that can hang for tens of seconds when tailscaled is
+// starved (UDP-blocked networks wedge its internals). The serial queue means
+// a burst of concurrent callers rides one fetch: the first spawns it, the
+// rest block until it lands and reuse the result via the TTL.
+final class TailscaleStatusCache: @unchecked Sendable {
+    private let queue = DispatchQueue(label: "com.copycat.macos.tailscale-status")
+    private let ttl: TimeInterval
+    private var cached: [TailscalePeer]?
+    private var fetchedAt = Date.distantPast
+
+    init(ttl: TimeInterval) {
+        self.ttl = ttl
+    }
+
+    /// `maxAge` overrides the TTL for one call; 0 forces a fresh fetch.
+    /// Failures ([] from a timed-out or not-logged-in daemon) are cached
+    /// too — retrying a wedged daemon every paste is exactly the stall this
+    /// cache exists to prevent, and an empty list fails open (callers skip
+    /// filtering and attempt every host).
+    func peers(
+        maxAge: TimeInterval? = nil,
+        now: Date = Date(),
+        fetch: () -> [TailscalePeer]
+    ) -> [TailscalePeer] {
+        queue.sync {
+            if let cached, now.timeIntervalSince(fetchedAt) < maxAge ?? ttl {
+                return cached
+            }
+            let fresh = fetch()
+            cached = fresh
+            fetchedAt = now
+            return fresh
+        }
+    }
+}
+
 enum TailscaleDiscovery {
     private static let candidatePaths = [
         "/Applications/Tailscale.app/Contents/MacOS/Tailscale",
@@ -20,19 +58,43 @@ enum TailscaleDiscovery {
 
     static var isAvailable: Bool { executablePath != nil }
 
-    // Full peer list, online or not. Used by the Settings UI to suggest
-    // hosts the user can pick from.
+    // Normal `status --json` answers from local daemon state in well under a
+    // second; only a wedged daemon blows past this. On timeout the peer list
+    // comes back empty, which fails open: callers attempt every configured
+    // host and SSH's ConnectTimeout does the filtering.
+    private static let statusTimeout: TimeInterval = 3
+
+    // TTL trades staleness for latency: a peer that changed state within the
+    // window is misjudged for at most 30s, and the SSH timeout already covers
+    // attempting a host that just went offline.
+    private static let cache = TailscaleStatusCache(ttl: 30)
+
+    /// Cached peer list for latency-sensitive callers (paste path, menu).
     static func allPeers() -> [TailscalePeer] {
+        cache.peers(fetch: fetchPeers)
+    }
+
+    /// Uncached fetch for the Settings "Refresh peers" button — an explicit
+    /// refresh returning 30s-old data would make the button a no-op.
+    static func refreshPeers() -> [TailscalePeer] {
+        cache.peers(maxAge: 0, fetch: fetchPeers)
+    }
+
+    private static func fetchPeers() -> [TailscalePeer] {
         guard let bin = executablePath else { return [] }
         // TAILSCALE_BE_CLI=1: when the bundled Tailscale binary is invoked from
         // a notarized app's subprocess (no TTY), it relaunches the GUI instead
         // of running as CLI. The env var forces CLI mode. See
         // tailscale/tailscale#16063 and #7140.
         let env = ["TAILSCALE_BE_CLI": "1", "PATH": "/usr/bin:/bin"]
-        guard let result = runShell(bin, args: ["status", "--json"], env: env) else {
+        guard let result = runShell(bin, args: ["status", "--json"], env: env, timeout: statusTimeout) else {
             Log.app.error("tailscale status: spawn failed for \(bin)")
             return []
         }
+        guard !result.timedOut else {
+            Log.app.error("tailscale status timed out after \(Int(statusTimeout))s — daemon unresponsive; treating peers as unknown")
+            return []
+        }
         guard result.isSuccess else {
             // Common when the user has Tailscale installed but isn't logged in
             // — info, not error, so it doesn't spam the console on every peer

Tests/CopyCatTests/ShellTests.swift

@@ -39,4 +39,32 @@ final class ShellTests: XCTestCase {
         XCTAssertTrue(result.stdout.isEmpty)
         XCTAssertTrue(result.stderr.contains("only-stderr"))
     }
+
+    // MARK: - Timeout
+
+    func testTimeoutKillsHungProcess() throws {
+        let start = Date()
+        let result = try XCTUnwrap(runShell("/bin/sleep", args: ["30"], timeout: 0.3))
+        XCTAssertTrue(result.timedOut)
+        XCTAssertFalse(result.isSuccess)
+        // Well under the sleep duration proves the watchdog, not the child,
+        // ended the run. Generous bound so a loaded CI box doesn't flake.
+        XCTAssertLessThan(Date().timeIntervalSince(start), 5)
+    }
+
+    func testFastProcessDoesNotTimeOut() throws {
+        let result = try XCTUnwrap(runShell("/bin/echo", args: ["hi"], timeout: 5))
+        XCTAssertFalse(result.timedOut)
+        XCTAssertTrue(result.isSuccess)
+        XCTAssertEqual(result.stdout.trimmingCharacters(in: .whitespacesAndNewlines), "hi")
+    }
+
+    func testNoTimeoutParameterMeansUnbounded() throws {
+        // Pin the default: omitting timeout must not kill even a slow-ish
+        // child. 1s keeps the suite fast while still outliving any plausible
+        // accidental internal deadline.
+        let result = try XCTUnwrap(runShell("/bin/sleep", args: ["1"]))
+        XCTAssertFalse(result.timedOut)
+        XCTAssertTrue(result.isSuccess)
+    }
 }

Tests/CopyCatTests/TailscaleStatusCacheTests.swift

@@ -0,0 +1,90 @@
+import XCTest
+@testable import CopyCat
+
+final class TailscaleStatusCacheTests: XCTestCase {
+    private func peer(_ name: String) -> TailscalePeer {
+        TailscalePeer(hostname: name, online: true, os: nil)
+    }
+
+    // Thread-safe call counter for the concurrency test; a captured `var`
+    // can't be mutated from concurrently-executing code under Swift 6.
+    private final class Counter: @unchecked Sendable {
+        private let lock = NSLock()
+        private var value = 0
+        func increment() {
+            lock.lock()
+            value += 1
+            lock.unlock()
+        }
+        var count: Int {
+            lock.lock()
+            defer { lock.unlock() }
+            return value
+        }
+    }
+
+    func testFirstCallFetches() {
+        let cache = TailscaleStatusCache(ttl: 30)
+        let result = cache.peers { [peer("a")] }
+        XCTAssertEqual(result.map(\.hostname), ["a"])
+    }
+
+    func testServesCachedWithinTTL() {
+        let cache = TailscaleStatusCache(ttl: 30)
+        let t0 = Date()
+        var calls = 0
+        _ = cache.peers(now: t0) { calls += 1; return [self.peer("a")] }
+        let second = cache.peers(now: t0.addingTimeInterval(29)) { calls += 1; return [self.peer("b")] }
+        XCTAssertEqual(calls, 1)
+        XCTAssertEqual(second.map(\.hostname), ["a"])
+    }
+
+    func testRefetchesPastTTL() {
+        let cache = TailscaleStatusCache(ttl: 30)
+        let t0 = Date()
+        var calls = 0
+        _ = cache.peers(now: t0) { calls += 1; return [self.peer("a")] }
+        let second = cache.peers(now: t0.addingTimeInterval(31)) { calls += 1; return [self.peer("b")] }
+        XCTAssertEqual(calls, 2)
+        XCTAssertEqual(second.map(\.hostname), ["b"])
+    }
+
+    func testMaxAgeZeroForcesRefetch() {
+        let cache = TailscaleStatusCache(ttl: 30)
+        let t0 = Date()
+        var calls = 0
+        _ = cache.peers(now: t0) { calls += 1; return [self.peer("a")] }
+        let second = cache.peers(maxAge: 0, now: t0) { calls += 1; return [self.peer("b")] }
+        XCTAssertEqual(calls, 2)
+        XCTAssertEqual(second.map(\.hostname), ["b"])
+    }
+
+    // Failure results must be cached like any other: refetching a wedged
+    // daemon on every call is the stall the cache exists to prevent.
+    func testEmptyResultIsCached() {
+        let cache = TailscaleStatusCache(ttl: 30)
+        let t0 = Date()
+        var calls = 0
+        _ = cache.peers(now: t0) { calls += 1; return [] }
+        let second = cache.peers(now: t0.addingTimeInterval(5)) { calls += 1; return [self.peer("b")] }
+        XCTAssertEqual(calls, 1)
+        XCTAssertEqual(second, [])
+    }
+
+    // A burst of concurrent callers (one per rapid ⌘⌥V press) must ride a
+    // single fetch: the first spawns it, the rest block and reuse. Each
+    // caller's default `now` predates the fetch's completion, so the TTL
+    // check passes for all of them once the result lands.
+    func testBurstCallersShareOneFetch() {
+        let cache = TailscaleStatusCache(ttl: 30)
+        let counter = Counter()
+        DispatchQueue.concurrentPerform(iterations: 5) { _ in
+            _ = cache.peers {
+                counter.increment()
+                Thread.sleep(forTimeInterval: 0.1)
+                return []
+            }
+        }
+        XCTAssertEqual(counter.count, 1)
+    }
+}