jay
/
pi-remote-ios
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ios): T-2.10 review follow-up — B-1 blocker + nits + coverage gaps

This commit is contained in:
jay 2026-05-17 12:27:08 +02:00
parent 419ad2fec1
commit 5eaa8ef2c8
6 changed files with 385 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
Sources/App/AppState.swift
View File

@ -20,7 +20,8 @@ final class AppState: ObservableObject {
}
private init() {
// Test-only overrides
#if DEBUG
// Test-only launch-argument overrides (gated: never active in Release).
let args = ProcessInfo.processInfo.arguments
if args.contains("--reset-state") {
Keychain.shared.delete(key: "piremote.credential")
@ -32,10 +33,12 @@ final class AppState: ObservableObject {
if args.contains("--force-lock") {
isLocked = true
}
#endif
// Try loading persisted credential on launch
credential = try? Keychain.shared.load(key: "piremote.credential")
#if DEBUG
// T-2.10: inject stub credential for UI tests that need MainTerminalView
if args.contains("--uitest-with-stub-connection") {
credential = SidecarCredential(
@ -48,6 +51,7 @@ final class AppState: ObservableObject {
pairedAt: Date()
)
}
#endif
}
func didPair(credential: SidecarCredential) {
@ -68,11 +72,15 @@ final class AppState: ObservableObject {
}
func appWillForeground() async {
_lifecycleTransitions.send(false) // T-2.10: always emit before Face-ID gate
_lifecycleTransitions.send(false) // T-2.10: emit before Face-ID gate
let elapsed = Date().timeIntervalSince(lastForegroundedAt)
guard elapsed > 60 else { return } // within 60s → no re-auth
isLocked = true
let ok = await FaceIDGate.authenticate()
isLocked = !ok
// N-2: after successful Face-ID auth the first emission fired when
// isLocked=true (so MainTerminalView skipped the reconnect). Re-emit
// now that isLocked=false so the connection actually resumes.
if ok { _lifecycleTransitions.send(false) }
}
}

4
Sources/App/piRemoteApp.swift
View File

@ -16,7 +16,8 @@ struct piRemoteApp: App {
notificationDelegate.setup()
UIApplication.shared.registerForRemoteNotifications()
// Test-only: auto-pair if argument present
#if DEBUG
// Test-only: auto-pair if argument present (never active in Release).
if let pairArgIndex = ProcessInfo.processInfo.arguments.firstIndex(of: "--pair-with-url"),
pairArgIndex + 1 < ProcessInfo.processInfo.arguments.count {
let urlString = ProcessInfo.processInfo.arguments[pairArgIndex + 1]
@ -24,6 +25,7 @@ struct piRemoteApp: App {
handlePairingURL(url)
}
}
#endif
}
.onOpenURL { url in
handlePairingURL(url)

51
Sources/Core/Sessions/SessionConnection.swift
View File

@ -53,9 +53,11 @@ public final class SessionConnection: ObservableObject {
// MARK: - Internal test/UI-test hook
#if DEBUG
/// Set to `true` in UI-test stub mode to bypass the real WebSocket and
/// drive connection-state transitions in-process.
internal var stubMode = false
#endif
// MARK: - Scrollback
@ -65,6 +67,7 @@ public final class SessionConnection: ObservableObject {
// MARK: - Private
private let credential: SidecarCredential
private let cursor: ResumeCursor
private var client: WebSocketClient?
private var cancellables = Set<AnyCancellable>()
private var keepAliveTask: Task<Void, Never>?
@ -74,9 +77,13 @@ public final class SessionConnection: ObservableObject {
/// Creates a `SessionConnection` for `id` authenticated with `credential`.
///
/// Does **not** open a WebSocket. Call `resume(from:)` to connect.
init(id: String, credential: SidecarCredential) {
///
/// - Parameter cursor: Injected `ResumeCursor` (defaults to the shared
/// UserDefaults-backed instance; override in tests for isolation).
init(id: String, credential: SidecarCredential, cursor: ResumeCursor = ResumeCursor()) {
self.id = id
self.credential = credential
self.cursor = cursor
self.scrollback = ScrollbackCache(sessionId: id)
}
@ -101,6 +108,7 @@ public final class SessionConnection: ObservableObject {
}
}
#if DEBUG
// T-2.10: stub mode — drive states in-process without a real WebSocket.
if stubMode {
connectionState = .connecting
@ -114,6 +122,7 @@ public final class SessionConnection: ObservableObject {
}
return
}
#endif
guard let url = streamURL else {
#if DEBUG
@ -135,13 +144,11 @@ public final class SessionConnection: ObservableObject {
}
.store(in: &cancellables)
// Binary frames → scrollback + downstream `stream` subject.
// Binary frames → scrollback + cursor + downstream `stream` subject.
ws.incomingBinary
.sink { [weak self] frame in
guard let self else { return }
self.isStreamFrozen = false // T-2.10: first delta clears freeze
self.scrollback.append(frame.data)
self.stream.send(frame.data)
self.handleBinaryFrame(frame)
}
.store(in: &cancellables)
@ -210,6 +217,40 @@ public final class SessionConnection: ObservableObject {
connectionState = .disconnected
}
// MARK: - Binary frame processing
/// Central handler for incoming binary frames.
///
/// Called from the `incomingBinary` sink (production) and from the
/// `#if DEBUG` test helper below (unit tests).
///
/// **CG-3 decision — informational, not blocking:**
/// `isStreamFrozen` is cleared on the first binary frame and the frame IS
/// forwarded to `stream`. In the IC-1 protocol the server only starts
/// sending data after it has processed our `resume` frame, so there are no
/// "stale" bytes that arrive while frozen — the first binary frame IS the
/// first meaningful delta. Stream delivery is therefore never blocked;
/// `isStreamFrozen` serves the status bar ("Reconnecting…") and the
/// `isStreamFrozen` unit tests, not as a gate on `stream`.
private func handleBinaryFrame(_ frame: BinaryFrame) {
isStreamFrozen = false // T-2.10: first delta thaws freeze
scrollback.append(frame.data)
cursor.update(sessionId: id, seq: frame.seq) // B-1: persist seq
stream.send(frame.data)
}
#if DEBUG
/// Test-only: simulate a binary frame arriving via the full production
/// code path (`handleBinaryFrame`), without needing a real WebSocket.
///
/// This drives the same `cursor.update` + `stream.send` logic that the
/// `incomingBinary` sink uses, so a regression in either is caught by
/// both production and test paths.
func _testOnly_receiveBinaryFrame(_ frame: BinaryFrame) {
handleBinaryFrame(frame)
}
#endif
// MARK: - URL construction
/// Builds `ws://<host>:<port>/sessions/<id>/stream?token=<bearerToken>`.

20
Sources/UI/Terminal/MainTerminalView.swift
View File

@ -25,14 +25,26 @@ struct MainTerminalView: View {
/// True when running under XCUITest. Skips the live SwiftTerm view + WS
/// connection, which otherwise keep the app non-idle and cause every
/// XCUITest interaction to block for ~120 s.
///
/// Always `false` in Release builds (test hooks are `#if DEBUG` only).
private var isUITest: Bool {
ProcessInfo.processInfo.arguments.contains("--uitest")
#if DEBUG
return ProcessInfo.processInfo.arguments.contains("--uitest")
#else
return false
#endif
}
/// True when the stub-connection mode is active (uitest + background lifecycle).
///
/// Always `false` in Release builds.
private var isUITestStub: Bool {
#if DEBUG
let args = ProcessInfo.processInfo.arguments
return args.contains("--uitest") && args.contains("--uitest-with-stub-connection")
#else
return false
#endif
}
var body: some View {
@ -118,6 +130,7 @@ struct MainTerminalView: View {
// MARK: - Bootstrap
private func initialBootstrap() async {
#if DEBUG
// UI-test mode: skip the live WebSocket. SwiftTerm's constant redraw
// on incoming frames prevents XCUITest's idle wait from completing,
// making every .tap() block for ~120 s. Status bar, modifier bar, and
@ -131,6 +144,7 @@ struct MainTerminalView: View {
}
return
}
#endif
statusText = "Looking for sessions…"
do {
let sessionId = try await resolveSession()
@ -244,8 +258,11 @@ struct MainTerminalView: View {
// MARK: - Stub connection (uitest-with-stub-connection)
#if DEBUG
/// Creates an in-process stub SessionConnection for UI tests that need
/// to exercise the background/foreground lifecycle without a real sidecar.
///
/// Gated with `#if DEBUG` — never compiled into Release builds.
private func bootstrapStubConnection() async {
let stubId = "stub"
sessionName = stubId
@ -267,6 +284,7 @@ struct MainTerminalView: View {
connection = conn
await conn.resume(from: nil)
}
#endif
// MARK: - Session resolution

85
Tests/CoreTests/AppStateLifecycleTests.swift
View File

@ -18,11 +18,10 @@ import XCTest
@MainActor
final class AppStateLifecycleTests: XCTestCase {
private nonisolated(unsafe) var cancellables = Set<AnyCancellable>()
private var cancellables = Set<AnyCancellable>()
override func tearDown() {
override func tearDown() async throws {
cancellables.removeAll()
super.tearDown()
}
// =========================================================================
@ -171,3 +170,83 @@ final class AppStateLifecycleTests: XCTestCase {
// MARK: - Stub removed
// lifecycleTransitions is now implemented on AppState directly (T-2.10 impl).
// MARK: - CG-2: Post-Face-ID reconnect signal
@MainActor
final class PostFaceIDReconnectTests: XCTestCase {
private var cancellables = Set<AnyCancellable>()
override func tearDown() async throws {
cancellables.removeAll()
}
/// CG-2 (depends on N-2): When elapsed > 60 s and Face-ID succeeds,
/// appWillForeground must emit TWO `false` transitions:
/// 1. The first emission fires before Face-ID (isLocked=true at that
/// point — MainTerminalView skips the reconnect via the guard).
/// 2. After successful auth isLocked=false, a second emission fires
/// so MainTerminalView actually re-resumes the connection.
///
/// Face-ID is disabled in this test (faceid.enabled not set), so
/// FaceIDGate.authenticate() returns `true` immediately without UI.
func test_cg2_postFaceID_emitsForegroundTwiceOnAuthSuccess() async throws {
let state = AppState.shared
// Ensure Face ID is disabled → FaceIDGate.authenticate() returns true.
UserDefaults.standard.removeObject(forKey: "faceid.enabled")
// Prime a "recent" background event, then rewind the timestamp so
// elapsed > 60 s (bypasses the grace-period guard).
state.appDidBackground()
state.lastForegroundedAt = Date().addingTimeInterval(-70)
var falseEmissions = 0
let exp = expectation(description: "two false emissions after Face-ID success")
exp.expectedFulfillmentCount = 2
state.lifecycleTransitions
.filter { $0 == false }
.sink { _ in
falseEmissions += 1
if falseEmissions <= 2 { exp.fulfill() }
}
.store(in: &cancellables)
await state.appWillForeground()
await fulfillment(of: [exp], timeout: 3.0)
XCTAssertEqual(falseEmissions, 2,
"CG-2 / N-2: exactly two false emissions expected " +
"(one pre-auth, one post-auth-success)")
XCTAssertFalse(state.isLocked,
"CG-2: isLocked must be false after successful Face-ID auth")
}
/// CG-2 guard: within the 60 s grace period only ONE false emission occurs
/// (no spurious second emission from the N-2 fix).
func test_cg2_withinGracePeriod_emitsForegroundOnce() async throws {
let state = AppState.shared
UserDefaults.standard.removeObject(forKey: "faceid.enabled")
state.appDidBackground()
// Do NOT rewind timestamp — elapsed ≈ 0 s, within grace period.
var falseCount = 0
state.lifecycleTransitions
.filter { $0 == false }
.sink { _ in falseCount += 1 }
.store(in: &cancellables)
await state.appWillForeground()
// Allow any async propagation to settle.
try await Task.sleep(nanoseconds: 100_000_000)
XCTAssertEqual(falseCount, 1,
"CG-2 guard: within grace period must emit exactly ONE false " +
"(no spurious second emission from N-2 fix)")
}
}

228
Tests/CoreTests/SessionConnectionLifecycleTests.swift
View File

@ -30,11 +30,10 @@ private func fakeCredential() -> SidecarCredential {
@MainActor
final class SessionConnectionLifecycleTests: XCTestCase {
private nonisolated(unsafe) var cancellables = Set<AnyCancellable>()
private var cancellables = Set<AnyCancellable>()
override func tearDown() {
override func tearDown() async throws {
cancellables.removeAll()
super.tearDown()
}
// =========================================================================
@ -234,3 +233,226 @@ final class SessionConnectionLifecycleTests: XCTestCase {
// isStreamFrozen and isKeepAliveActive are now implemented on SessionConnection
// directly (T-2.10 impl). The extension stubs above would cause a redeclaration
// error and are therefore removed.
// MARK: - CG-1: End-to-end real-seq path
@MainActor
final class EndToEndSeqPathTests: XCTestCase {
private var cancellables = Set<AnyCancellable>()
override func tearDown() async throws {
cancellables.removeAll()
}
/// CG-1: Verifies the full production path:
/// binary frame received
/// → ResumeCursor.update (B-1 fix)
/// → background (lastCapturedSeq = cursor.lastSeq)
/// → foreground resume(from: nonNil)
/// → isStreamFrozen = true
/// → first delta received → isStreamFrozen = false, forwarded to stream
///
/// Uses no hardcoded seq sentinels and exercises the real cursor update
/// path via `_testOnly_receiveBinaryFrame` (which calls `handleBinaryFrame`,
/// the same private method the production `incomingBinary` sink invokes).
func test_cg1_endToEnd_realSeqFlowsThrough() async throws {
// Isolated cursor so this test doesn't pollute real UserDefaults.
let suiteName = "CG1.\(UUID().uuidString)"
let defaults = UserDefaults(suiteName: suiteName)!
let cursor = ResumeCursor(defaults: defaults)
defer { defaults.removePersistentDomain(forName: suiteName) }
let conn = SessionConnection(
id: "cg1-session",
credential: fakeCredential(),
cursor: cursor
)
// ── Step 1: receive a binary frame with a real seq ────────────────
let seq1: UInt64 = 42
let frame1 = BinaryFrame(seq: seq1, data: Data("hello".utf8))
conn._testOnly_receiveBinaryFrame(frame1)
// ── Step 2: cursor is now updated (what MainTerminalView reads on bg)
let persistedSeq = cursor.lastSeq(for: "cg1-session")
XCTAssertEqual(persistedSeq, seq1,
"CG-1 / B-1: ResumeCursor must be updated with the frame's seq.")
// ── Step 3: simulate background — capture the cursor value ────────
let lastCapturedSeq = persistedSeq // non-nil because B-1 is fixed
XCTAssertNotNil(lastCapturedSeq,
"CG-1: lastCapturedSeq must be non-nil after a real binary frame")
// ── Step 4: foreground — resume from captured seq ─────────────────
await conn.resume(from: lastCapturedSeq)
try await Task.sleep(nanoseconds: 50_000_000) // 50 ms
XCTAssertTrue(conn.isStreamFrozen,
"CG-1: isStreamFrozen must be true immediately after resume(from: nonNil)")
// ── Step 5: first delta arrives → freeze clears, stream receives data
var receivedData: [Data] = []
let streamExp = expectation(description: "first delta forwarded to stream")
conn.stream
.first()
.sink { data in
receivedData.append(data)
streamExp.fulfill()
}
.store(in: &cancellables)
let frame2 = BinaryFrame(seq: seq1 + 1, data: Data("world".utf8))
conn._testOnly_receiveBinaryFrame(frame2)
await fulfillment(of: [streamExp], timeout: 1.0)
XCTAssertFalse(conn.isStreamFrozen,
"CG-1: isStreamFrozen must clear on first binary frame (first delta IS forwarded)")
XCTAssertEqual(receivedData.count, 1,
"CG-1: Exactly one frame must reach stream")
XCTAssertEqual(receivedData.first, Data("world".utf8),
"CG-1: The delta payload must be forwarded unchanged")
XCTAssertEqual(cursor.lastSeq(for: "cg1-session"), seq1 + 1,
"CG-1: Cursor must be updated to the new seq after the first delta")
await conn.suspend()
}
}
// MARK: - CG-3: isStreamFrozen gating behaviour
@MainActor
final class StreamFrozenGatingTests: XCTestCase {
private var cancellables = Set<AnyCancellable>()
override func tearDown() async throws {
cancellables.removeAll()
}
/// CG-3 decision: INFORMATIONAL (not a hard gate on stream delivery).
///
/// In the IC-1 protocol the server only starts sending bytes after it
/// processes our `resume` frame, so there are no stale bytes that could
/// arrive while `isStreamFrozen == true`. The first binary frame IS
/// the first meaningful delta and IS forwarded to `stream`.
///
/// `isStreamFrozen` drives the status-bar label ("Reconnecting…" vs
/// "Connecting…") and exposes the freeze state for observability, but
/// does not technically block bytes from reaching `stream.send()`.
///
/// This test verifies:
/// - After resume(from: nonNil): isStreamFrozen = true
/// - After the first binary frame: isStreamFrozen = false
/// - The first binary frame IS forwarded to stream (first delta IS forwarded)
func test_cg3_firstDeltaThawsAndIsForwarded() async throws {
let conn = SessionConnection(id: "cg3", credential: fakeCredential())
await conn.resume(from: 50) // non-nil → freeze
try await Task.sleep(nanoseconds: 50_000_000)
XCTAssertTrue(conn.isStreamFrozen, "Must be frozen after resume(from: nonNil)")
var received: [Data] = []
conn.stream
.sink { received.append($0) }
.store(in: &cancellables)
// First delta arrives while frozen: must thaw AND be forwarded.
let firstDelta = BinaryFrame(seq: 51, data: Data("delta".utf8))
conn._testOnly_receiveBinaryFrame(firstDelta)
XCTAssertFalse(conn.isStreamFrozen,
"CG-3: First binary frame must clear isStreamFrozen")
XCTAssertEqual(received.count, 1,
"CG-3: First delta must be forwarded to stream (not dropped)")
// Subsequent frames also flow normally.
conn._testOnly_receiveBinaryFrame(BinaryFrame(seq: 52, data: Data("more".utf8)))
XCTAssertEqual(received.count, 2,
"CG-3: Subsequent frames must also be forwarded")
await conn.suspend()
}
/// Additional CG-3 guard: fresh attach (nil seq) must never set isStreamFrozen.
func test_cg3_freshAttach_neverFreezes() async throws {
let conn = SessionConnection(id: "cg3-fresh", credential: fakeCredential())
await conn.resume(from: nil) // nil → no freeze
try await Task.sleep(nanoseconds: 50_000_000)
XCTAssertFalse(conn.isStreamFrozen,
"CG-3: Fresh attach must not freeze stream")
await conn.suspend()
}
}
// MARK: - CG-4: Multi-cycle reconnect with seq progression
@MainActor
final class MultiCycleSeqTests: XCTestCase {
private var cancellables = Set<AnyCancellable>()
override func tearDown() async throws {
cancellables.removeAll()
}
/// CG-4: Two full suspend/resume cycles with seq advancing on each.
/// Cycle 1: receive seq=100 → suspend → resume(from:100) → frozen
/// Cycle 2: receive seq=120 → suspend → resume(from:120) → frozen
/// Verifies cursor advances and isStreamFrozen toggles correctly.
func test_cg4_multiCycle_seqProgression() async throws {
let suiteName = "CG4.\(UUID().uuidString)"
let defaults = UserDefaults(suiteName: suiteName)!
let cursor = ResumeCursor(defaults: defaults)
defer { defaults.removePersistentDomain(forName: suiteName) }
let conn = SessionConnection(
id: "cg4-session",
credential: fakeCredential(),
cursor: cursor
)
// ──────────────────────────────── Cycle 1 ────────────────────────────
// Receive a frame with seq=100.
conn._testOnly_receiveBinaryFrame(BinaryFrame(seq: 100, data: Data("a".utf8)))
XCTAssertEqual(cursor.lastSeq(for: "cg4-session"), 100,
"CG-4 cycle 1: cursor must be 100 after receiving seq=100")
// Suspend (isStreamFrozen cleared by suspend).
await conn.suspend()
XCTAssertFalse(conn.isStreamFrozen, "suspend() must clear isStreamFrozen")
// Resume from captured seq=100 → frozen.
await conn.resume(from: 100)
try await Task.sleep(nanoseconds: 50_000_000)
XCTAssertTrue(conn.isStreamFrozen,
"CG-4 cycle 1: must be frozen after resume(from: 100)")
// Receive seq=120 → thaws.
conn._testOnly_receiveBinaryFrame(BinaryFrame(seq: 120, data: Data("b".utf8)))
XCTAssertFalse(conn.isStreamFrozen,
"CG-4 cycle 1: first delta must clear freeze")
XCTAssertEqual(cursor.lastSeq(for: "cg4-session"), 120,
"CG-4 cycle 1: cursor must advance to 120")
// ──────────────────────────────── Cycle 2 ────────────────────────────
await conn.suspend()
XCTAssertFalse(conn.isStreamFrozen, "suspend() must clear isStreamFrozen")
// Resume from captured seq=120 → frozen.
await conn.resume(from: 120)
try await Task.sleep(nanoseconds: 50_000_000)
XCTAssertTrue(conn.isStreamFrozen,
"CG-4 cycle 2: must be frozen after resume(from: 120)")
// One more frame → thaws and cursor advances.
conn._testOnly_receiveBinaryFrame(BinaryFrame(seq: 150, data: Data("c".utf8)))
XCTAssertFalse(conn.isStreamFrozen,
"CG-4 cycle 2: second cycle delta must also clear freeze")
XCTAssertEqual(cursor.lastSeq(for: "cg4-session"), 150,
"CG-4 cycle 2: cursor must advance to 150")
await conn.suspend()
}
}