fix(escrow): circuit-break retrieves against a wedged holder

Sources/SessionEscrow.swift

@@ -733,6 +733,37 @@ final class SessionEscrowClient {
 // MARK: - App-side retrieval (issue #182 slice 2)
 
 extension SessionEscrowClient {
+    /// Time-bounded circuit breaker state for `retrieve`, keyed by holder
+    /// socket path -- see `retrieve`'s "Escrow follow-up" doc comment for
+    /// the rationale. `retrieve` itself is only ever called from the
+    /// main-thread session-restore path (`Workspace+Persistence.swift`'s
+    /// `attemptSessionReattach`), one call at a time, so nothing here is
+    /// actually contended in production; the lock exists purely so tests
+    /// (which may probe from a background accept thread) never have to
+    /// reason about a data race, at negligible cost on the real serial
+    /// restore path.
+    /// Monotonic (`DispatchTime.now()`, backed by `CLOCK_UPTIME_RAW`) rather
+    /// than wall-clock `Date` -- a backward clock jump (NTP sync, sleep/wake,
+    /// manual clock change) must not keep the circuit open longer than
+    /// `circuitBreakerWindow` actually elapsed.
+    private static var recentRetrieveTimeoutsByPath: [String: DispatchTime] = [:]
+    private static let circuitBreakerLock = NSLock()
+    /// How long a path stays "open" (skipped without connecting) after a
+    /// recorded timeout. Comfortably covers a full serial restore sweep
+    /// (bounded by panel count in a real workspace) while not permanently
+    /// blacklisting a holder that recovers -- the window doubling as the
+    /// only reset mechanism is deliberate, see `retrieve`'s doc comment.
+    private static let circuitBreakerWindow: TimeInterval = 60.0
+
+    /// Test-only: clears breaker state so test cases don't leak it into
+    /// each other, and lets a test simulate the window elapsing without an
+    /// actual 60s sleep by re-arming a fresh state instead.
+    static func resetCircuitBreakerForTesting() {
+        circuitBreakerLock.lock()
+        recentRetrieveTimeoutsByPath.removeAll()
+        circuitBreakerLock.unlock()
+    }
+
     /// One-shot fd retrieval for the reattach path
     /// (`Workspace+Persistence.swift`'s `createPanel(from:inPane:)`). Opens
     /// a FRESH connection to the holder's socket -- the previous app
@@ -747,14 +778,52 @@ extension SessionEscrowClient {
     /// unknown session, token mismatch, child already exited, timeout)
     /// returns nil so the caller can fall through to its existing
     /// spawn-fresh path unchanged.
-    static func retrieve(sessionId: String, tokenHex: String, socketPath: String) -> Int32? {
+    ///
+    /// Escrow follow-up (issue tracker #6, post-#182): app-launch restore
+    /// (`Workspace+Persistence.swift`'s `attemptSessionReattach`) calls this
+    /// SERIALLY on the main thread, once per escrowed panel, and nearly
+    /// every panel shares the SAME deterministic holder socket path. A
+    /// missing/stale socket fails fast (`connect` returns nil immediately,
+    /// see `UnixDomainFDPassing.connect`'s doc comment) -- the
+    /// `retrieveRecvTimeout` cost is only ever paid when a holder ACCEPTS
+    /// the connection but never answers (wedged). Without a breaker, N
+    /// sessions against one wedged holder cost N * `retrieveRecvTimeout` of
+    /// main-thread stall at launch. `recentRetrieveTimeoutsByPath` makes
+    /// that a one-time cost per launch per path: once a path has timed out,
+    /// every other `retrieve` call against that SAME path within
+    /// `circuitBreakerWindow` returns nil immediately, without even
+    /// attempting to connect. The window is intentionally short and
+    /// self-healing rather than a manual reset -- 60s comfortably covers a
+    /// full serial restore sweep (bounded by how many panels a real
+    /// workspace has) while not permanently blacklisting a holder that
+    /// recovers (e.g. survives a transient hang and answers normally on the
+    /// next real request after this launch).
+    static func retrieve(
+        sessionId: String,
+        tokenHex: String,
+        socketPath: String,
+        recvTimeout: TimeInterval = SessionEscrowPolicy.retrieveRecvTimeout
+    ) -> Int32? {
         let attemptStartedAt = Date()
-        let timeoutMs = Int(SessionEscrowPolicy.retrieveRecvTimeout * 1000)
+        let timeoutMs = Int(recvTimeout * 1000)
         dilog("escrow.retrieve", "attempt session=\(sessionId.prefix(8)) socket=\(socketPath) timeoutMs=\(timeoutMs)")
         func logOutcome(_ outcome: String) {
             let elapsedMs = Int(Date().timeIntervalSince(attemptStartedAt) * 1000)
             dilog("escrow.retrieve", "outcome session=\(sessionId.prefix(8)) result=\(outcome) elapsedMs=\(elapsedMs)")
         }
+
+        circuitBreakerLock.lock()
+        let recordedOpenedAt = recentRetrieveTimeoutsByPath[socketPath]
+        circuitBreakerLock.unlock()
+        if let openedAt = recordedOpenedAt {
+            let sinceNs = DispatchTime.now().uptimeNanoseconds &- openedAt.uptimeNanoseconds
+            let sinceMs = Int(sinceNs / 1_000_000)
+            if sinceNs < UInt64(circuitBreakerWindow * 1_000_000_000) {
+                dilog("escrow.retrieve", "skipped session=\(sessionId.prefix(8)) reason=circuit_open path_failed_ago_ms=\(sinceMs)")
+                return nil
+            }
+        }
+
         guard let token = decodeHexToken(tokenHex) else {
             logOutcome("error_bad_token_hex")
             return nil
@@ -767,9 +836,8 @@ extension SessionEscrowClient {
 
         // Sub-second timeout, so this must split whole seconds from the
         // fractional remainder rather than truncating straight to
-        // `tv_sec` -- a bare `Int(retrieveRecvTimeout)` would silently
-        // become 0 seconds with no microsecond budget at all.
-        let recvTimeout = SessionEscrowPolicy.retrieveRecvTimeout
+        // `tv_sec` -- a bare `Int(recvTimeout)` would silently become 0
+        // seconds with no microsecond budget at all.
         let recvTimeoutWholeSeconds = recvTimeout.rounded(.down)
         var timeout = timeval(
             tv_sec: Int(recvTimeoutWholeSeconds),
@@ -801,6 +869,10 @@ extension SessionEscrowClient {
                 return nil
             case .timeout:
                 if let receivedFD { close(receivedFD) }
+                circuitBreakerLock.lock()
+                recentRetrieveTimeoutsByPath[socketPath] = DispatchTime.now()
+                circuitBreakerLock.unlock()
+                dilog("escrow.retrieve", "circuit_opened session=\(sessionId.prefix(8)) path=\(socketPath)")
                 logOutcome("timeout")
                 return nil
             case .error:

programaTests/TerminalControllerSocketSecurityTests.swift

@@ -715,4 +715,143 @@ final class TerminalControllerSocketSecurityTests: XCTestCase {
         // the process does not survive the `send` call above.
         XCTAssertFalse(sendResult, "send to a closed peer must fail cleanly (EPIPE), not succeed")
     }
+
+    // MARK: - Session escrow: per-socket-path circuit breaker (escrow follow-up #6)
+
+    /// Thread-safe bookkeeping for the background "wedged holder" accept
+    /// loop below: it accepts every connection but never responds, so a
+    /// `retrieve()` call against it always times out rather than seeing EOF
+    /// -- the exact failure mode the circuit breaker exists for.
+    private final class AcceptedConnections: @unchecked Sendable {
+        private let lock = NSLock()
+        private var fds: [Int32] = []
+
+        func append(_ fd: Int32) {
+            lock.lock()
+            fds.append(fd)
+            lock.unlock()
+        }
+
+        var count: Int {
+            lock.lock()
+            defer { lock.unlock() }
+            return fds.count
+        }
+
+        func closeAll() {
+            lock.lock()
+            let all = fds
+            fds = []
+            lock.unlock()
+            for fd in all { close(fd) }
+        }
+
+        /// Polls (bounded) until the accept thread has recorded at least
+        /// `expected` connections, or `timeout` elapses. A plain `count`
+        /// read races the accept thread: `retrieve()` returning (its recv
+        /// timeout having elapsed) only guarantees the kernel completed the
+        /// connection, not that this thread's `accept()` call has returned
+        /// and appended the fd yet. Returns whatever count was actually
+        /// observed so the caller's assertion message stays meaningful on
+        /// a genuine failure.
+        func waitForCount(atLeast expected: Int, timeout: TimeInterval = 2.0) -> Int {
+            let deadline = Date().addingTimeInterval(timeout)
+            while true {
+                let current = count
+                if current >= expected || Date() >= deadline {
+                    return current
+                }
+                Thread.sleep(forTimeInterval: 0.005)
+            }
+        }
+    }
+
+    /// Regression for the escrow follow-up ("per-socket-path circuit
+    /// breaker for wedged holders"): app-launch restore
+    /// (`Workspace+Persistence.swift`'s `attemptSessionReattach`) calls
+    /// `SessionEscrowClient.retrieve` SERIALLY on the main thread, once per
+    /// escrowed panel, and almost every panel shares the SAME deterministic
+    /// holder socket path. Without a breaker, N sessions against one
+    /// wedged (accepts but never answers) holder would each pay the full
+    /// `retrieveRecvTimeout` -- N times the stall for one dead holder.
+    ///
+    /// This drives a real wedged holder (accepts, never responds) against
+    /// an injectable short `recvTimeout` (avoiding the real 5s
+    /// `SessionEscrowPolicy.retrieveRecvTimeout`) and asserts: the first
+    /// retrieve pays the full timeout and opens the breaker for that path;
+    /// a second retrieve against the SAME path returns near-instantly
+    /// without attempting a new connection at all (the listener sees no
+    /// second connection); and after resetting the breaker, a retrieve
+    /// against that path attempts a genuinely fresh connect again.
+    func testEscrowRetrieveCircuitBreakerSkipsSecondCallToWedgedHolder() {
+        SessionEscrowClient.resetCircuitBreakerForTesting()
+        defer { SessionEscrowClient.resetCircuitBreakerForTesting() }
+
+        let socketPath = makeSocketPath("escrow-cb")
+        defer { unlink(socketPath) }
+
+        guard let listenFD = UnixDomainFDPassing.bindListening(socketPath: socketPath) else {
+            XCTFail("bindListening failed")
+            return
+        }
+
+        let accepted = AcceptedConnections()
+        let holderThread = Thread {
+            while true {
+                let clientFD = accept(listenFD, nil, nil)
+                guard clientFD >= 0 else { break }
+                // Wedged: accept the connection but never send a response
+                // and never close it -- the client's own recv timeout is
+                // the only thing that ever ends this connection.
+                accepted.append(clientFD)
+            }
+        }
+        holderThread.start()
+        defer {
+            close(listenFD) // unblocks the accept() loop so the thread exits
+            accepted.closeAll()
+        }
+
+        let sessionId = String(repeating: "a", count: EscrowWireFormat.sessionIdSize)
+        let tokenHex = String(repeating: "00", count: EscrowWireFormat.tokenSize)
+        let shortTimeout: TimeInterval = 0.2
+
+        let firstStart = Date()
+        let firstResult = SessionEscrowClient.retrieve(
+            sessionId: sessionId,
+            tokenHex: tokenHex,
+            socketPath: socketPath,
+            recvTimeout: shortTimeout
+        )
+        let firstElapsed = Date().timeIntervalSince(firstStart)
+        XCTAssertNil(firstResult, "a wedged holder must never grant a retrieval")
+        XCTAssertGreaterThanOrEqual(firstElapsed, shortTimeout, "first retrieve must actually wait out the recv timeout")
+        XCTAssertEqual(accepted.waitForCount(atLeast: 1), 1, "the wedged holder must have accepted exactly one connection so far")
+
+        let secondStart = Date()
+        let secondResult = SessionEscrowClient.retrieve(
+            sessionId: sessionId,
+            tokenHex: tokenHex,
+            socketPath: socketPath,
+            recvTimeout: shortTimeout
+        )
+        let secondElapsed = Date().timeIntervalSince(secondStart)
+        XCTAssertNil(secondResult)
+        XCTAssertLessThan(secondElapsed, 0.15, "second retrieve against the same path must be skipped by the open breaker, not pay another timeout (still well under the 0.2s injected timeout)")
+        XCTAssertEqual(accepted.count, 1, "an open breaker must prevent a second connection attempt to the wedged holder")
+
+        SessionEscrowClient.resetCircuitBreakerForTesting()
+
+        let thirdStart = Date()
+        let thirdResult = SessionEscrowClient.retrieve(
+            sessionId: sessionId,
+            tokenHex: tokenHex,
+            socketPath: socketPath,
+            recvTimeout: shortTimeout
+        )
+        let thirdElapsed = Date().timeIntervalSince(thirdStart)
+        XCTAssertNil(thirdResult)
+        XCTAssertGreaterThanOrEqual(thirdElapsed, shortTimeout, "after resetting the breaker, retrieve must attempt a genuinely fresh connect")
+        XCTAssertEqual(accepted.waitForCount(atLeast: 2), 2, "the reset breaker must allow a new connection attempt, which the holder accepts")
+    }
 }