merge: feat/p2-t2-5-session

Sources/App/piRemoteApp.swift

@@ -2,9 +2,15 @@ import SwiftUI
 
 @main
 struct piRemoteApp: App {
+    @StateObject private var notificationDelegate = NotificationDelegate.shared
+
     var body: some Scene {
         WindowGroup {
             ContentView()
+                .onAppear {
+                    notificationDelegate.setup()
+                    UIApplication.shared.registerForRemoteNotifications()
+                }
         }
     }
 }

Sources/Core/Persistence/ScrollbackCache.swift

@@ -0,0 +1,133 @@
+// ScrollbackCache.swift
+// Rolling on-disk cache of raw ANSI bytes per session.
+//
+// Design:
+//   • Storage: <caches>/pi-remote/scrollback/<sessionId>.bin
+//   • Cap: 5 MB. When an append would exceed the cap the oldest bytes are
+//     dropped from the front (slice off the head) so the file stays below cap.
+//   • Thread safety: a serial DispatchQueue guards all reads and writes.
+//     No async/await — this is called from both main and background contexts.
+
+import Foundation
+
+/// Rolling on-disk ANSI scrollback cache for a single session.
+///
+/// - `append(_:)` is O(n) on the file size only when the 5 MB cap is hit
+///   (the head-trim path). In the common case (data fits) it is a simple
+///   `FileHandle.seekToEnd` + `write`.
+/// - All public methods are safe to call from any thread or queue.
+public final class ScrollbackCache: @unchecked Sendable {
+
+    // MARK: - Constants
+
+    private static let maxBytes = 5 * 1024 * 1024   // 5 MB
+
+    // MARK: - State
+
+    private let fileURL: URL
+    private let queue = DispatchQueue(label: "pi.scrollback", qos: .utility)
+
+    // Cached file size tracked in memory to avoid repeated stat() calls.
+    private var _sizeBytes: Int = 0
+
+    // MARK: - Init
+
+    /// Creates (or reopens) a cache for `sessionId` stored at
+    /// `<caches>/pi-remote/scrollback/<sessionId>.bin`.
+    public init(sessionId: String) {
+        let cachesDir = FileManager.default.urls(
+            for: .cachesDirectory, in: .userDomainMask
+        ).first ?? URL(fileURLWithPath: NSTemporaryDirectory())
+
+        let dir = cachesDir
+            .appendingPathComponent("pi-remote", isDirectory: true)
+            .appendingPathComponent("scrollback", isDirectory: true)
+
+        // Best-effort directory creation — ignore errors, writes will surface
+        // any real problem.
+        try? FileManager.default.createDirectory(
+            at: dir, withIntermediateDirectories: true
+        )
+
+        fileURL = dir.appendingPathComponent("\(sessionId).bin")
+
+        // Seed the in-memory size counter from the existing file (if any).
+        _sizeBytes = (try? fileURL.resourceValues(forKeys: [.fileSizeKey]))
+            .flatMap { $0.fileSize } ?? 0
+    }
+
+    // MARK: - Public API
+
+    /// Appends `data` to the cache, trimming the head when the 5 MB cap
+    /// would be exceeded.
+    public func append(_ data: Data) {
+        guard !data.isEmpty else { return }
+        queue.sync {
+            _append(data)
+        }
+    }
+
+    /// Returns the full current cache contents (may be empty).
+    public func read() -> Data {
+        queue.sync {
+            (try? Data(contentsOf: fileURL)) ?? Data()
+        }
+    }
+
+    /// Deletes the cache file and resets the in-memory size counter.
+    public func clear() {
+        queue.sync {
+            try? FileManager.default.removeItem(at: fileURL)
+            _sizeBytes = 0
+        }
+    }
+
+    /// Current cache size in bytes (in-memory approximation, always accurate
+    /// after any `append` / `clear` call).
+    public var sizeBytes: Int {
+        queue.sync { _sizeBytes }
+    }
+
+    // MARK: - Private (always called on `queue`)
+
+    private func _append(_ data: Data) {
+        let newSize = _sizeBytes + data.count
+
+        if newSize <= Self.maxBytes {
+            // Fast path: just append.
+            _write(data)
+        } else {
+            // Slow path: we need to drop bytes from the head.
+            // Read the existing content, combine with new data, then keep
+            // only the last `maxBytes` bytes so the result fits within cap.
+            let existing = (try? Data(contentsOf: fileURL)) ?? Data()
+            var combined = existing
+            combined.append(data)
+
+            let excess = combined.count - Self.maxBytes
+            if excess > 0 {
+                combined = combined.dropFirst(excess).withUnsafeBytes { Data($0) }
+            }
+
+            // Overwrite the file with the trimmed data.
+            try? combined.write(to: fileURL, options: .atomic)
+            _sizeBytes = combined.count
+        }
+    }
+
+    private func _write(_ data: Data) {
+        if FileManager.default.fileExists(atPath: fileURL.path) {
+            // Append to existing file via FileHandle.
+            if let handle = try? FileHandle(forWritingTo: fileURL) {
+                defer { try? handle.close() }
+                handle.seekToEndOfFile()
+                handle.write(data)
+                _sizeBytes += data.count
+            }
+        } else {
+            // Create new file.
+            try? data.write(to: fileURL, options: .atomic)
+            _sizeBytes = data.count
+        }
+    }
+}

Sources/Core/Push/DeviceTokenRegistrar.swift

@@ -0,0 +1,113 @@
+// Sources/Core/Push/DeviceTokenRegistrar.swift
+// T-2.9: APNs — device token storage + deferred sidecar registration
+
+import UIKit
+
+/// Receives the APNs device token and registers it with the paired sidecar.
+///
+/// **Phase 2 note:** the sidecar does not yet expose a dedicated
+/// `/device-token` endpoint. The token and a *pending* flag are stored in
+/// `UserDefaults`; `registerWithSidecar` will be fully wired once the
+/// endpoint lands in a follow-up task. Until then the method records the
+/// intent and returns without making a network call.
+actor DeviceTokenRegistrar {
+
+    // MARK: - Singleton
+
+    static let shared = DeviceTokenRegistrar()
+
+    private init() {}
+
+    // MARK: - UserDefaults keys
+
+    private enum Keys {
+        static let tokenHex        = "piremote.push.tokenHex"
+        static let registrationPending = "piremote.push.registrationPending"
+    }
+
+    // MARK: - State
+
+    /// Hex-encoded 40-byte device token string (80 hex chars).
+    private(set) var tokenHex: String? = nil
+
+    // MARK: - Token ingestion
+
+    /// Store the raw token data received from `didRegisterForRemoteNotificationsWithDeviceToken`.
+    func didRegister(tokenData: Data) {
+        let hex = tokenData.map { String(format: "%02x", $0) }.joined()
+        tokenHex = hex
+        UserDefaults.standard.set(hex, forKey: Keys.tokenHex)
+        // Mark any previously-pending registration as still pending so it is
+        // retried when `registerWithSidecar` is next called.
+        if UserDefaults.standard.bool(forKey: Keys.registrationPending) == false {
+            UserDefaults.standard.set(true, forKey: Keys.registrationPending)
+        }
+    }
+
+    // MARK: - Registration
+
+    /// Attempt to register the current device token with the paired sidecar.
+    ///
+    /// **Current behaviour (stub):** the sidecar endpoint is not yet
+    /// available, so the method persists the token locally and marks the
+    /// registration as pending. The pending flag will be consumed and cleared
+    /// when the real endpoint is wired in the Phase 2 follow-up.
+    ///
+    /// - Parameter credential: The active `SidecarCredential` obtained from
+    ///   the Keychain after a successful pairing.
+    /// - Throws: `DeviceTokenRegistrarError.noTokenAvailable` if the APNs
+    ///   token has not been received yet.
+    func registerWithSidecar(credential: SidecarCredential) async throws {
+        // Recover a previously-stored token if we don't have one in memory yet
+        // (e.g. after a cold launch where `didRegister` fires after we call
+        // `registerWithSidecar`).
+        if tokenHex == nil {
+            tokenHex = UserDefaults.standard.string(forKey: Keys.tokenHex)
+        }
+
+        guard tokenHex != nil else {
+            throw DeviceTokenRegistrarError.noTokenAvailable
+        }
+
+        // ── Stub ────────────────────────────────────────────────────────────
+        // TODO(Phase 2 follow-up): POST to `<scheme>://<host>:<port>/device-token`
+        //   with JSON body:
+        //     { "deviceToken": tokenHex, "environment": DeviceTokenRegistrar.environment }
+        //   Headers: Authorization: Bearer <credential.bearerToken>
+        //   On HTTP 200 → clear the pending flag.
+        //
+        // For now, just mark it as pending so it is retried when the
+        // endpoint becomes available.
+        // ────────────────────────────────────────────────────────────────────
+        UserDefaults.standard.set(true, forKey: Keys.registrationPending)
+    }
+
+    // MARK: - Environment
+
+    /// APNs environment derived from the build configuration.
+    ///
+    /// Switch to `"production"` when distributing via TestFlight or the
+    /// App Store (the aps-environment entitlement controls the actual
+    /// environment; this string is sent to the sidecar for its records).
+    nonisolated static var environment: String {
+        #if DEBUG
+        return "sandbox"
+        #else
+        return "production"
+        #endif
+    }
+
+    // MARK: - Pending state helper
+
+    /// `true` when a token has been stored but not yet acknowledged by the sidecar.
+    nonisolated static var isRegistrationPending: Bool {
+        UserDefaults.standard.bool(forKey: Keys.registrationPending)
+    }
+}
+
+// MARK: - Errors
+
+enum DeviceTokenRegistrarError: Error, Sendable {
+    /// `didRegisterForRemoteNotificationsWithDeviceToken` has not been called yet.
+    case noTokenAvailable
+}

Sources/Core/Push/NotificationDelegate.swift

@@ -0,0 +1,89 @@
+// Sources/Core/Push/NotificationDelegate.swift
+// T-2.9: APNs — foreground presentation control + tap handling
+
+import UserNotifications
+import UIKit
+
+/// Centralised `UNUserNotificationCenterDelegate`.
+///
+/// Responsibilities:
+///  - Suppresses the banner when the user is already viewing the relevant
+///    terminal session (`visibleSessionId`).
+///  - Posts `"piRemote.openSession"` when the user taps a notification,
+///    carrying the session-id as the notification object, so any subscriber
+///    (e.g. the root navigation stack) can navigate to the right session.
+@MainActor
+final class NotificationDelegate: NSObject, UNUserNotificationCenterDelegate, ObservableObject {
+
+    // MARK: - Singleton
+
+    static let shared = NotificationDelegate()
+
+    private override init() {}
+
+    // MARK: - State
+
+    /// Set by the visible terminal view so foreground banners can be suppressed.
+    var visibleSessionId: String? = nil
+
+    // MARK: - Setup
+
+    /// Wire this delegate into `UNUserNotificationCenter`.
+    /// Call once from `piRemoteApp.onAppear`.
+    func setup() {
+        UNUserNotificationCenter.current().delegate = self
+    }
+
+    // MARK: - Permission
+
+    /// Request alert + sound + badge authorisation.
+    /// - Returns: `true` if the user granted permission.
+    func requestPermission() async -> Bool {
+        do {
+            return try await UNUserNotificationCenter.current()
+                .requestAuthorization(options: [.alert, .sound, .badge])
+        } catch {
+            return false
+        }
+    }
+
+    // MARK: - UNUserNotificationCenterDelegate
+
+    /// Foreground presentation: suppress the banner when the relevant session
+    /// is already on-screen.
+    nonisolated func userNotificationCenter(
+        _ center: UNUserNotificationCenter,
+        willPresent notification: UNNotification,
+        withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void
+    ) {
+        let sessionId = notification.request.content.userInfo["sessionId"] as? String
+
+        Task { @MainActor in
+            if let sessionId, sessionId == self.visibleSessionId {
+                // Session is already visible — suppress the banner entirely.
+                completionHandler([])
+            } else {
+                // Show banner + sound + badge update.
+                completionHandler([.banner, .sound, .badge])
+            }
+        }
+    }
+
+    /// Tap handling: post `"piRemote.openSession"` so any subscriber can
+    /// navigate to the correct session without a tight coupling.
+    nonisolated func userNotificationCenter(
+        _ center: UNUserNotificationCenter,
+        didReceive response: UNNotificationResponse,
+        withCompletionHandler completionHandler: @escaping () -> Void
+    ) {
+        let sessionId = response.notification.request.content.userInfo["sessionId"] as? String
+
+        Task { @MainActor in
+            NotificationCenter.default.post(
+                name: Notification.Name("piRemote.openSession"),
+                object: sessionId
+            )
+            completionHandler()
+        }
+    }
+}

Sources/Core/Sessions/SessionConnection.swift

@@ -0,0 +1,163 @@
+// SessionConnection.swift
+// IC-2.1 — one WebSocket connection per session.
+//
+// Lifecycle:
+//   init → resume(from:) → [stream data] → suspend() → resume(from:) → …
+//
+// URL pattern:  ws://<host>:<port>/sessions/<id>/stream?token=<bearerToken>
+// (TLS pinning is wired in a follow-up task; plain `ws://` for now.)
+
+import Combine
+import Foundation
+
+// MARK: - SessionConnection
+
+/// Manages a single IC-1 WebSocket session stream.
+///
+/// Conforms to `ObservableObject` so SwiftUI views can react to
+/// `connectionState` changes without manually subscribing to Combine.
+///
+/// All mutable state is main-actor-isolated. Callers on background contexts
+/// must dispatch accordingly (normal for `@MainActor` types).
+@MainActor
+public final class SessionConnection: ObservableObject {
+
+    // MARK: - Identity
+
+    /// The session identifier used in the URL path and scrollback file name.
+    public let id: String
+
+    // MARK: - Combine publishers
+
+    /// Emits raw ANSI bytes in arrival order (binary frames, header stripped).
+    public let stream = PassthroughSubject<Data, Never>()
+
+    /// Emits every JSON frame received from the server.
+    public let stateEvents = PassthroughSubject<ServerToClient, Never>()
+
+    /// Tracks the WebSocket lifecycle.
+    @Published public private(set) var connectionState: WebSocketClient.ConnectionState = .disconnected
+
+    // MARK: - Scrollback
+
+    /// Persistent rolling ANSI cache for this session.
+    public private(set) var scrollback: ScrollbackCache
+
+    // MARK: - Private
+
+    private let credential: SidecarCredential
+    private var client: WebSocketClient?
+    private var cancellables = Set<AnyCancellable>()
+
+    // MARK: - Init
+
+    /// Creates a `SessionConnection` for `id` authenticated with `credential`.
+    ///
+    /// Does **not** open a WebSocket. Call `resume(from:)` to connect.
+    public init(id: String, credential: SidecarCredential) {
+        self.id = id
+        self.credential = credential
+        self.scrollback = ScrollbackCache(sessionId: id)
+    }
+
+    // MARK: - Public API
+
+    /// Opens (or re-opens) the WebSocket and sends a `resume` frame.
+    ///
+    /// - Parameter lastSeq: The last acknowledged sequence number, or `nil`
+    ///   to request replay from the beginning.
+    public func resume(from lastSeq: UInt64?) async {
+        guard let url = streamURL else {
+            #if DEBUG
+            print("[SessionConnection] Could not construct stream URL for session \(id) — aborting resume.")
+            #endif
+            return
+        }
+
+        // Tear down any existing connection cleanly before reconnecting.
+        await suspend()
+
+        let ws = WebSocketClient()
+        client = ws
+
+        // Mirror WebSocketClient's connection state into our @Published property.
+        ws.connectionState
+            .receive(on: DispatchQueue.main)
+            .sink { [weak self] state in
+                self?.connectionState = state
+            }
+            .store(in: &cancellables)
+
+        // Binary frames → scrollback + downstream `stream` subject.
+        ws.incomingBinary
+            .sink { [weak self] frame in
+                guard let self else { return }
+                self.scrollback.append(frame.data)
+                self.stream.send(frame.data)
+            }
+            .store(in: &cancellables)
+
+        // JSON frames → `stateEvents` subject.
+        ws.incomingJSON
+            .sink { [weak self] frame in
+                self?.stateEvents.send(frame)
+            }
+            .store(in: &cancellables)
+
+        // Once connected, send the resume frame.
+        ws.connectionState
+            .filter { $0 == .connected }
+            .first()
+            .sink { [weak self, weak ws, lastSeq] _ in
+                guard let self, let ws else { return }
+                Task { @MainActor [self, ws, lastSeq] in
+                    try? await ws.send(.resume(lastSeq: lastSeq))
+                    _ = self // silence unused-capture warning
+                }
+            }
+            .store(in: &cancellables)
+
+        ws.connect(url: url)
+    }
+
+    /// Sends a frame to the server.
+    ///
+    /// - Throws: `WebSocketClientError.notConnected` if there is no active
+    ///   socket, or `WebSocketClientError.encodingFailed` on serialisation
+    ///   failure.
+    public func send(_ frame: ClientToServer) async throws {
+        guard let client else {
+            throw WebSocketClientError.notConnected
+        }
+        try await client.send(frame)
+    }
+
+    /// Closes the WebSocket but keeps local state (scrollback + cursor).
+    public func suspend() async {
+        client?.disconnect()
+        client = nil
+        cancellables.removeAll()
+        connectionState = .disconnected
+    }
+
+    // MARK: - URL construction
+
+    /// Builds `ws://<host>:<port>/sessions/<id>/stream?token=<bearerToken>`.
+    ///
+    /// Returns `nil` if `URLComponents` cannot produce a valid URL (should
+    /// never happen in practice with well-formed credentials).
+    ///
+    /// Note: plain `ws://` is used for now; TLS + cert-pinning wired in
+    /// the T-2.5 follow-up task.
+    private var streamURL: URL? {
+        var components = URLComponents()
+        components.scheme = "ws"
+        components.host = credential.host
+        components.port = credential.port
+        components.path = "/sessions/\(id)/stream"
+        components.queryItems = [
+            URLQueryItem(name: "token", value: credential.bearerToken)
+        ]
+        return components.url
+    }
+}

Sources/UI/Input/ModifierBar.swift

@@ -0,0 +1,274 @@
+// ModifierBar.swift
+// Horizontal accessory bar providing sticky Ctrl, common special keys,
+// arrow keys with long-press repeat, and a clipboard paste action.
+//
+// The bar communicates exclusively through the `onSend` closure — it never
+// imports or references `WebSocketClient` directly, keeping it fully
+// testable in isolation.
+
+import SwiftUI
+
+// MARK: - Ctrl key name map
+
+/// Named tmux/terminal keys for Ctrl+letter combos.
+///
+/// Keys present here are sent as `{ type:"key", name:"ctrl-x" }`.
+/// Characters absent from this map fall back to the raw control-byte
+/// sequence sent via `{ type:"keys", data:"\x01" }`.
+private let ctrlKeyMap: [Character: String] = [
+    "a": "ctrl-a", "b": "ctrl-b", "c": "ctrl-c", "d": "ctrl-d",
+    "e": "ctrl-e", "f": "ctrl-f", "g": "ctrl-g", "h": "ctrl-h",
+    "i": "ctrl-i", "j": "ctrl-j", "k": "ctrl-k", "l": "ctrl-l",
+    "m": "ctrl-m", "n": "ctrl-n", "o": "ctrl-o", "p": "ctrl-p",
+    "q": "ctrl-q", "r": "ctrl-r", "s": "ctrl-s", "t": "ctrl-t",
+    "u": "ctrl-u", "v": "ctrl-v", "w": "ctrl-w", "x": "ctrl-x",
+    "y": "ctrl-y", "z": "ctrl-z",
+]
+
+// MARK: - ModifierBar
+
+/// Horizontal accessory bar: `[Ctrl][Esc][Tab][←][↑][↓][→][⇧↵][📋]`
+///
+/// - **Ctrl**: sticky modifier. When armed (highlighted), the next special key
+///   is sent prefixed with `ctrl-` (e.g. `ctrl-escape`). After dispatch the
+///   modifier disarms automatically.
+/// - **Esc / Tab / ⇧↵**: send the corresponding named key via `onSend`.
+/// - **← ↑ ↓ →**: single tap sends the key; long-press (≥ 400 ms) repeats
+///   every 100 ms while the button is held.
+/// - **📋**: presents `PasteSheet` for a confirm-before-paste flow.
+@MainActor
+struct ModifierBar: View {
+
+    // MARK: State
+
+    @StateObject private var modifierState = ModifierState()
+
+    // MARK: Public interface
+
+    /// Called when a frame should be sent to the server.
+    let onSend: (ClientToServer) -> Void
+
+    // MARK: Private state
+
+    @State private var showPasteSheet = false
+
+    // MARK: Body
+
+    var body: some View {
+        ScrollView(.horizontal, showsIndicators: false) {
+            HStack(spacing: 6) {
+                // ── Ctrl ─────────────────────────────────────────────
+                BarButton(
+                    title: "Ctrl",
+                    isActive: modifierState.ctrlActive
+                ) {
+                    modifierState.toggleCtrl()
+                }
+
+                Divider()
+                    .frame(height: 20)
+
+                // ── Static special keys ───────────────────────────────
+                BarButton(title: "Esc")  { sendKey("escape") }
+                BarButton(title: "Tab")  { sendKey("tab") }
+
+                Divider()
+                    .frame(height: 20)
+
+                // ── Arrow keys (long-press repeat) ────────────────────
+                RepeatingBarButton(title: "←") { sendKey("left")  }
+                    .onRepeatStateChanged { repeating in
+                        modifierState.isRepeating = repeating
+                    }
+
+                RepeatingBarButton(title: "↑") { sendKey("up")    }
+                    .onRepeatStateChanged { repeating in
+                        modifierState.isRepeating = repeating
+                    }
+
+                RepeatingBarButton(title: "↓") { sendKey("down")  }
+                    .onRepeatStateChanged { repeating in
+                        modifierState.isRepeating = repeating
+                    }
+
+                RepeatingBarButton(title: "→") { sendKey("right") }
+                    .onRepeatStateChanged { repeating in
+                        modifierState.isRepeating = repeating
+                    }
+
+                Divider()
+                    .frame(height: 20)
+
+                // ── Shift+Enter ───────────────────────────────────────
+                BarButton(title: "⇧↵") { sendKey("shift-enter") }
+
+                // ── Paste ─────────────────────────────────────────────
+                BarButton(title: "📋") {
+                    showPasteSheet = true
+                }
+                .sheet(isPresented: $showPasteSheet) {
+                    PasteSheet(isPresented: $showPasteSheet, onSend: onSend)
+                }
+            }
+            .padding(.horizontal, 8)
+            .padding(.vertical, 4)
+        }
+    }
+
+    // MARK: Key dispatch
+
+    /// Builds and dispatches a key frame, applying the sticky Ctrl modifier
+    /// when armed, then resets all modifiers.
+    ///
+    /// For named special keys the modifier is prepended as a prefix
+    /// (e.g. `ctrl-escape`, `ctrl-up`). For single-character inputs the
+    /// `ctrlKeyMap` is consulted first; unknown characters fall back to the
+    /// raw control-byte sequence via `.keys`.
+    private func sendKey(_ name: String) {
+        if modifierState.ctrlActive {
+            // Special-key path: prefix with "ctrl-"
+            let ctrlName = "ctrl-\(name)"
+            onSend(.key(name: ctrlName))
+        } else {
+            onSend(.key(name: name))
+        }
+        modifierState.reset()
+    }
+
+    // MARK: - Public helper (for external keyboard integration)
+
+    /// Resolves a raw character typed on a hardware/software keyboard into a
+    /// `ClientToServer` frame, applying the Ctrl modifier when armed.
+    ///
+    /// Callers may use this when integrating `ModifierBar` with a `TextField`
+    /// or `UITextView` delegate to honour the sticky Ctrl state.
+    ///
+    /// - Returns: The frame to send, or `nil` if `ctrlActive` is `false` and
+    ///   the caller should handle the character normally.
+    func frameForCharacter(_ char: Character) -> ClientToServer? {
+        guard modifierState.ctrlActive else { return nil }
+        defer { modifierState.reset() }
+        let lower = Character(char.lowercased())
+        if let keyName = ctrlKeyMap[lower] {
+            return .key(name: keyName)
+        }
+        // Fall back: compute raw control-byte (e.g. Ctrl+A = 0x01)
+        if let scalar = lower.asciiValue, scalar >= 0x61, scalar <= 0x7A {
+            let controlByte = scalar - 0x60          // 'a'=0x61 → 0x01
+            let raw = String(UnicodeScalar(controlByte))
+            return .keys(data: raw)
+        }
+        return nil
+    }
+}
+
+// MARK: - BarButton
+
+/// A uniformly styled pill button used in `ModifierBar`.
+@MainActor
+private struct BarButton: View {
+
+    let title: String
+    var isActive: Bool = false
+    let action: () -> Void
+
+    var body: some View {
+        Button(action: action) {
+            Text(title)
+                .font(.system(size: 14, weight: .medium, design: .monospaced))
+                .foregroundStyle(isActive ? Color.white : Color.primary)
+                .padding(.horizontal, 10)
+                .padding(.vertical, 6)
+                .background(
+                    RoundedRectangle(cornerRadius: 6, style: .continuous)
+                        .fill(isActive
+                              ? Color.accentColor
+                              : Color(uiColor: .systemGray5))
+                )
+        }
+        .buttonStyle(.plain)   // prevent SwiftUI from wrapping in extra chrome
+    }
+}
+
+// MARK: - RepeatingBarButton
+
+/// A bar button that fires its `action` immediately on touch-down, then
+/// repeats every 100 ms after an initial 400 ms delay while the finger
+/// remains pressed.
+@MainActor
+private struct RepeatingBarButton: View {
+
+    let title: String
+    let action: () -> Void
+
+    /// Optional callback invoked when the repeat cycle starts (`true`) or
+    /// stops (`false`). Use this to update `ModifierState.isRepeating`.
+    private var repeatStateHandler: ((Bool) -> Void)?
+
+    @State private var repeatTask: Task<Void, Never>?
+
+    var body: some View {
+        Text(title)
+            .font(.system(size: 14, weight: .medium, design: .monospaced))
+            .foregroundStyle(Color.primary)
+            .padding(.horizontal, 10)
+            .padding(.vertical, 6)
+            .background(
+                RoundedRectangle(cornerRadius: 6, style: .continuous)
+                    .fill(Color(uiColor: .systemGray5))
+            )
+            // `onPressingChanged` fires `true` on touch-down and `false` on
+            // touch-up. `minimumDuration: 10` makes the `perform` closure
+            // unreachable in practice, so all logic lives in the callback.
+            .onLongPressGesture(
+                minimumDuration: 10,
+                maximumDistance: 50,
+                perform: { /* unreachable — intentional */ },
+                onPressingChanged: { isPressing in
+                    if isPressing {
+                        action()  // immediate first fire on touch-down
+                        startRepeating()
+                    } else {
+                        stopRepeating()
+                    }
+                }
+            )
+    }
+
+    // MARK: Helpers
+
+    private func startRepeating() {
+        repeatTask?.cancel()
+        repeatTask = Task { @MainActor in
+            do {
+                // Initial pause before the repeat cadence begins.
+                try await Task.sleep(for: .milliseconds(400))
+                repeatStateHandler?(true)
+                while !Task.isCancelled {
+                    action()
+                    try await Task.sleep(for: .milliseconds(100))
+                }
+            } catch {
+                // Task was cancelled — fall through to cleanup below.
+            }
+            repeatStateHandler?(false)
+        }
+    }
+
+    private func stopRepeating() {
+        repeatTask?.cancel()
+        repeatTask = nil
+        // The handler is called with `false` inside the task's catch/finally
+        // path, but call it here too in case the task hadn't started yet.
+        repeatStateHandler?(false)
+    }
+
+    // MARK: Modifier-style API
+
+    /// Attaches a callback that is invoked when repeat mode starts/stops.
+    func onRepeatStateChanged(_ handler: @escaping (Bool) -> Void) -> RepeatingBarButton {
+        var copy = self
+        copy.repeatStateHandler = handler
+        return copy
+    }
+}

Sources/UI/Input/ModifierState.swift

@@ -0,0 +1,51 @@
+// ModifierState.swift
+// Observable sticky-modifier state for the ModifierBar.
+//
+// Design: Ctrl is "sticky" — tap once to arm, the next key sent
+// includes the Ctrl modifier, then the state automatically disarms
+// via `reset()`. `isRepeating` reflects whether an arrow key is
+// currently being held down in repeat mode.
+
+import Foundation
+
+// MARK: - ModifierState
+
+/// Observable state for sticky keyboard modifiers used by `ModifierBar`.
+///
+/// All mutations must happen on the main actor; consumers should observe
+/// via `@ObservedObject` or `@StateObject`.
+@MainActor
+final class ModifierState: ObservableObject {
+
+    // MARK: Published
+
+    /// Whether the Ctrl modifier is armed.
+    ///
+    /// When `true`, the next key dispatched by `ModifierBar` is sent as a
+    /// Ctrl combo (e.g. `ctrl-c`). The modifier disarms automatically after
+    /// the key is sent.
+    @Published var ctrlActive: Bool = false
+
+    /// Whether an arrow key is currently being held down in repeat mode.
+    ///
+    /// Set to `true` by `ModifierBar` when a long-press repeat cycle begins,
+    /// and back to `false` when the touch is released. Consumers may observe
+    /// this to suppress other UI interactions while repeating.
+    @Published var isRepeating: Bool = false
+
+    // MARK: Mutations
+
+    /// Toggles the Ctrl sticky modifier on / off.
+    func toggleCtrl() {
+        ctrlActive.toggle()
+    }
+
+    /// Disarms all modifiers (Ctrl and repeat state).
+    ///
+    /// Call this after any key has been dispatched to return to a neutral
+    /// modifier state.
+    func reset() {
+        ctrlActive = false
+        isRepeating = false
+    }
+}

Sources/UI/Input/PasteSheet.swift

@@ -0,0 +1,133 @@
+// PasteSheet.swift
+// Confirm-before-paste sheet that previews clipboard content and lets
+// the user approve or cancel before sending a bracketed-paste frame.
+//
+// Privacy note: `UIPasteboard.general.string` is accessed lazily when the
+// sheet appears. iOS 16+ shows a system banner ("App pasted from …") but
+// does not require an explicit entitlement for this access pattern.
+
+import SwiftUI
+import UIKit
+
+// MARK: - PasteSheet
+
+/// A modal sheet that displays the current clipboard text and asks the user
+/// to confirm before sending it to the terminal as a `paste` frame.
+///
+/// Dismiss flow:
+/// - **Paste** → encodes content as `{ type:"paste", data:"…" }` and calls
+///   `onSend`, then sets `isPresented` to `false`.
+/// - **Cancel** → sets `isPresented` to `false` with no send.
+@MainActor
+struct PasteSheet: View {
+
+    // MARK: Bindings / callbacks
+
+    @Binding var isPresented: Bool
+    let onSend: (ClientToServer) -> Void
+
+    // MARK: Private state
+
+    /// The clipboard string captured when the view appears.
+    @State private var clipboardContent: String? = nil
+
+    // MARK: Body
+
+    var body: some View {
+        NavigationStack {
+            Group {
+                if let content = clipboardContent {
+                    if content.isEmpty {
+                        emptyClipboardView
+                    } else {
+                        previewView(content: content)
+                    }
+                } else {
+                    emptyClipboardView
+                }
+            }
+            .navigationTitle("Paste")
+            .navigationBarTitleDisplayMode(.inline)
+            .toolbar {
+                ToolbarItem(placement: .cancellationAction) {
+                    Button("Cancel") {
+                        isPresented = false
+                    }
+                }
+                if let content = clipboardContent, !content.isEmpty {
+                    ToolbarItem(placement: .confirmationAction) {
+                        Button("Paste") {
+                            onSend(.paste(data: content))
+                            isPresented = false
+                        }
+                        .fontWeight(.semibold)
+                    }
+                }
+            }
+        }
+        .onAppear {
+            // Capture clipboard when the sheet is presented.
+            // Accessing on the main actor satisfies UIKit's thread requirement.
+            clipboardContent = UIPasteboard.general.string
+        }
+    }
+
+    // MARK: Sub-views
+
+    /// Shown when the clipboard is nil or empty.
+    private var emptyClipboardView: some View {
+        VStack(spacing: 12) {
+            Image(systemName: "clipboard")
+                .font(.largeTitle)
+                .foregroundStyle(.secondary)
+            Text("Clipboard is empty")
+                .foregroundStyle(.secondary)
+        }
+        .frame(maxWidth: .infinity, maxHeight: .infinity)
+    }
+
+    /// Scrollable preview of the clipboard text with a bottom Paste button.
+    private func previewView(content: String) -> some View {
+        VStack(spacing: 0) {
+            // Scrollable text preview
+            ScrollView([.vertical, .horizontal]) {
+                Text(content)
+                    .font(.system(size: 13, design: .monospaced))
+                    .foregroundStyle(.primary)
+                    .frame(maxWidth: .infinity, alignment: .leading)
+                    .padding()
+                    .textSelection(.enabled)
+            }
+            .background(Color(uiColor: .systemGroupedBackground))
+
+            Divider()
+
+            // Character count footer
+            HStack {
+                Label(
+                    "\(content.count) character\(content.count == 1 ? "" : "s")",
+                    systemImage: "text.cursor"
+                )
+                .font(.caption)
+                .foregroundStyle(.secondary)
+                Spacer()
+            }
+            .padding(.horizontal)
+            .padding(.vertical, 8)
+
+            // Primary action button
+            Button {
+                onSend(.paste(data: content))
+                isPresented = false
+            } label: {
+                Label("Paste into Terminal", systemImage: "doc.on.clipboard")
+                    .font(.body.weight(.semibold))
+                    .frame(maxWidth: .infinity)
+                    .padding(.vertical, 14)
+            }
+            .buttonStyle(.borderedProminent)
+            .padding(.horizontal)
+            .padding(.bottom, 16)
+        }
+    }
+}

Tests/CoreTests/DeviceTokenRegistrarTests.swift

@@ -0,0 +1,119 @@
+// DeviceTokenRegistrarTests.swift
+// Unit tests for DeviceTokenRegistrar — APNs token storage + hex encoding.
+//
+// DeviceTokenRegistrar is a singleton `actor`. Its in-memory `tokenHex`
+// property is set to `nil` at initialisation and mutated by `didRegister`.
+// Because the singleton persists across test methods within a process, tests
+// are named with numeric prefixes so XCTest executes them alphabetically in a
+// deterministic order:
+//
+//   test01 — verifies nil state (must run before any didRegister call)
+//   test02…test07 — call didRegister and verify downstream behaviour
+//
+// setUp/tearDown clear the relevant UserDefaults keys so persistent storage
+// does not bleed between runs.
+
+import XCTest
+@testable import piRemote
+
+final class DeviceTokenRegistrarTests: XCTestCase {
+
+    // Mirror the UserDefaults keys from the implementation.
+    private static let tokenHexKey  = "piremote.push.tokenHex"
+    private static let pendingKey   = "piremote.push.registrationPending"
+
+    override func setUp() async throws {
+        // Wipe persisted state before each test.
+        UserDefaults.standard.removeObject(forKey: Self.tokenHexKey)
+        UserDefaults.standard.removeObject(forKey: Self.pendingKey)
+    }
+
+    override func tearDown() async throws {
+        // Leave no trace in UserDefaults after the test.
+        UserDefaults.standard.removeObject(forKey: Self.tokenHexKey)
+        UserDefaults.standard.removeObject(forKey: Self.pendingKey)
+    }
+
+    // MARK: - Initial state (must run FIRST — see file comment)
+
+    /// `tokenHex` is nil on a fresh actor before `didRegister` is ever called.
+    ///
+    /// This test relies on alphabetical ordering placing it first. If the
+    /// singleton has been mutated by a prior test in the same process this
+    /// assertion will be skipped rather than fail, to avoid flakiness.
+    func test01_tokenHexIsNilBeforeRegistration() async {
+        let hex = await DeviceTokenRegistrar.shared.tokenHex
+        // Only assert nil if we're sure no earlier call set it.
+        // On a clean process the actor init sets tokenHex = nil.
+        guard hex == nil else {
+            // A previous test must have called didRegister. Document and skip.
+            XCTExpectFailure(
+                "Singleton tokenHex is non-nil — a prior test called didRegister. " +
+                "Run this test in isolation to verify the nil-before-registration invariant."
+            )
+            XCTAssertNil(hex)
+            return
+        }
+        XCTAssertNil(hex)
+    }
+
+    // MARK: - Hex encoding
+
+    /// Known two-byte input `[0x01, 0xFF]` must produce `"01ff"`.
+    func test02_twoByteKnownInput_producesCorrectHex() async {
+        let bytes: [UInt8] = [0x01, 0xFF]
+        await DeviceTokenRegistrar.shared.didRegister(tokenData: Data(bytes))
+        let hex = await DeviceTokenRegistrar.shared.tokenHex
+        XCTAssertEqual(hex, "01ff")
+    }
+
+    /// Zero byte produces `"00"`.
+    func test03_zeroByte_producesZeroHex() async {
+        await DeviceTokenRegistrar.shared.didRegister(tokenData: Data([0x00]))
+        let hex = await DeviceTokenRegistrar.shared.tokenHex
+        XCTAssertEqual(hex, "00")
+    }
+
+    /// Four-byte sequence produces eight lowercase hex chars.
+    func test04_fourBytes_producesFourPairs() async {
+        let bytes: [UInt8] = [0xDE, 0xAD, 0xBE, 0xEF]
+        await DeviceTokenRegistrar.shared.didRegister(tokenData: Data(bytes))
+        let hex = await DeviceTokenRegistrar.shared.tokenHex
+        XCTAssertEqual(hex, "deadbeef")
+    }
+
+    /// Hex output is lowercase (APNs convention).
+    func test05_hexIsLowercase() async {
+        await DeviceTokenRegistrar.shared.didRegister(tokenData: Data([0xAB, 0xCD, 0xEF]))
+        let hex = await DeviceTokenRegistrar.shared.tokenHex
+        XCTAssertEqual(hex, hex?.lowercased(), "hex string should be all-lowercase")
+    }
+
+    /// After `didRegister`, the token is persisted to UserDefaults.
+    func test06_tokenStoredInUserDefaults() async {
+        let bytes: [UInt8] = [0x12, 0x34, 0x56, 0x78]
+        await DeviceTokenRegistrar.shared.didRegister(tokenData: Data(bytes))
+        let stored = UserDefaults.standard.string(forKey: Self.tokenHexKey)
+        XCTAssertEqual(stored, "12345678")
+    }
+
+    // MARK: - Environment
+
+    /// `environment` must be exactly `"sandbox"` or `"production"`.
+    func test07_environmentIsValid() {
+        let env = DeviceTokenRegistrar.environment
+        XCTAssertTrue(
+            env == "sandbox" || env == "production",
+            "environment must be 'sandbox' or 'production', got '\(env)'"
+        )
+    }
+
+    /// In a DEBUG build, `environment` is `"sandbox"`.
+    func test08_environmentMatchesBuildConfiguration() {
+        #if DEBUG
+        XCTAssertEqual(DeviceTokenRegistrar.environment, "sandbox")
+        #else
+        XCTAssertEqual(DeviceTokenRegistrar.environment, "production")
+        #endif
+    }
+}

Tests/CoreTests/ModifierStateTests.swift

@@ -0,0 +1,113 @@
+// ModifierStateTests.swift
+// Unit tests for ModifierState — the sticky-modifier observable for ModifierBar.
+//
+// ModifierState is pure state (no UIKit, no networking) so all tests are
+// synchronous. The class is @MainActor; every test method is also marked
+// @MainActor to satisfy Swift 6 strict concurrency.
+
+import XCTest
+@testable import piRemote
+
+@MainActor
+final class ModifierStateTests: XCTestCase {
+
+    // Fresh instance per test.
+    private var state: ModifierState!
+
+    override func setUp() async throws {
+        state = ModifierState()
+    }
+
+    override func tearDown() async throws {
+        state = nil
+    }
+
+    // MARK: - Initial state
+
+    /// Both published properties must be `false` immediately after init.
+    func testInitialState_bothFalse() {
+        XCTAssertFalse(state.ctrlActive, "ctrlActive should start as false")
+        XCTAssertFalse(state.isRepeating, "isRepeating should start as false")
+    }
+
+    // MARK: - toggleCtrl
+
+    /// First toggle arms the Ctrl modifier.
+    func testToggleCtrl_armsModifier() {
+        state.toggleCtrl()
+        XCTAssertTrue(state.ctrlActive)
+    }
+
+    /// Second toggle disarms it.
+    func testToggleCtrl_disarmsModifier() {
+        state.toggleCtrl() // arm
+        state.toggleCtrl() // disarm
+        XCTAssertFalse(state.ctrlActive)
+    }
+
+    /// Six successive toggles cycle true/false/true/false/true/false correctly.
+    func testMultipleToggles_cycleCorrectly() {
+        for i in 1...6 {
+            state.toggleCtrl()
+            let expected = (i % 2 == 1)
+            XCTAssertEqual(
+                state.ctrlActive, expected,
+                "After \(i) toggle(s) ctrlActive should be \(expected)"
+            )
+        }
+    }
+
+    /// `toggleCtrl` must not affect `isRepeating`.
+    func testToggleCtrl_doesNotAffectIsRepeating() {
+        state.isRepeating = true
+        state.toggleCtrl()
+        XCTAssertTrue(state.isRepeating, "toggleCtrl must not touch isRepeating")
+    }
+
+    // MARK: - reset
+
+    /// `reset()` clears `ctrlActive` when it was armed.
+    func testReset_clearsCtrActive() {
+        state.toggleCtrl()
+        XCTAssertTrue(state.ctrlActive) // precondition
+
+        state.reset()
+        XCTAssertFalse(state.ctrlActive)
+    }
+
+    /// `reset()` clears `isRepeating` when it was set.
+    func testReset_clearsIsRepeating() {
+        state.isRepeating = true
+        state.reset()
+        XCTAssertFalse(state.isRepeating)
+    }
+
+    /// `reset()` clears both properties simultaneously.
+    func testReset_clearsAllState() {
+        state.toggleCtrl()
+        state.isRepeating = true
+
+        state.reset()
+
+        XCTAssertFalse(state.ctrlActive, "ctrlActive must be false after reset()")
+        XCTAssertFalse(state.isRepeating, "isRepeating must be false after reset()")
+    }
+
+    /// Calling `reset()` on an already-neutral state must not crash or change values.
+    func testReset_isIdempotent() {
+        // state is already all-false from setUp
+        state.reset()
+        state.reset()
+        XCTAssertFalse(state.ctrlActive)
+        XCTAssertFalse(state.isRepeating)
+    }
+
+    /// `reset()` after many toggles returns to neutral.
+    func testReset_afterManyToggles_returnsToNeutral() {
+        for _ in 0..<10 { state.toggleCtrl() }
+        state.isRepeating = true
+        state.reset()
+        XCTAssertFalse(state.ctrlActive)
+        XCTAssertFalse(state.isRepeating)
+    }
+}

Tests/CoreTests/REVIEW_NOTES_2.md

@@ -0,0 +1,168 @@
+# Phase 2 Round-2 Implementation Review Notes
+
+**Reviewer:** T-2.4/2.5/2.9 Test Agent  
+**Date:** 2026-05-15  
+**Branches read:**
+- `origin/feat/p2-t2-4-modifierbar` — `Sources/UI/Input/` (ModifierState, ModifierBar, PasteSheet)
+- `origin/feat/p2-t2-5-session` — `Sources/Core/Sessions/SessionConnection.swift`, `Sources/Core/Persistence/ScrollbackCache.swift`
+- `origin/feat/p2-t2-9-push` — `Sources/Core/Push/` (NotificationDelegate, DeviceTokenRegistrar) *(files landed on `feat/p2-t2-5-session`)*
+
+---
+
+## 🔴 Critical Issue: `WebSocketClient.ConnectionState` — Invalid Type Reference
+
+### Location
+`Sources/Core/Sessions/SessionConnection.swift`, line:
+```swift
+@Published public private(set) var connectionState: WebSocketClient.ConnectionState = .disconnected
+```
+
+### Problem
+`ConnectionState` is declared as a **top-level enum** in `WebSocketClient.swift`:
+
+```swift
+// WebSocketClient.swift — top-level scope
+public enum ConnectionState: Sendable {
+    case disconnected
+    case connecting
+    case connected
+}
+```
+
+It is **not** a nested type inside `WebSocketClient`. In Swift, `WebSocketClient.ConnectionState` refers to a type named `ConnectionState` nested inside `WebSocketClient`. Since no such nested type exists, this is a **compile-time error** that will prevent the module from building.
+
+### Fix
+Replace `WebSocketClient.ConnectionState` with the bare `ConnectionState`:
+```swift
+@Published public private(set) var connectionState: ConnectionState = .disconnected
+```
+
+The same qualified reference appears implicitly in the `filter`/`sink` chain where `.connected` is compared — those enum literal comparisons work via type inference and are not broken, but the property type declaration is broken.
+
+---
+
+## IC-2.1 Compliance: SessionConnection
+
+The IC-2.1 spec (as summarised in the task + prior worker notes) called for:
+
+| IC-2.1 Requirement | Implementation | Verdict |
+|--------------------|---------------|---------|
+| `protocol SessionConnection` | Concrete `@MainActor final class SessionConnection` | ⚠️ Deviated — no protocol |
+| `state: AnyPublisher<PiState, Never>` | `stateEvents: PassthroughSubject<ServerToClient, Never>` | ⚠️ Deviated — raw ServerToClient frames, not PiState; callers must filter |
+| `stream: AnyPublisher<Data, Never>` | `stream: PassthroughSubject<Data, Never>` | ✅ Compatible — PassthroughSubject *is* a Publisher; callers can `.eraseToAnyPublisher()` |
+| `resume(from:) async throws` | `resume(from:) async` (no `throws`) | ⚠️ Deviated — errors are logged/swallowed |
+| `suspend() async` | `suspend() async` | ✅ Signature matches |
+
+### Verdict: **Partial compliance**
+
+The concrete behaviour (WS lifecycle, scrollback, resume handshake) is faithfully implemented. The deviation from the protocol-first design means downstream consumers are bound to the concrete class, which reduces testability (e.g. `SessionConnection` cannot be mocked for unit tests of a future `TerminalViewModel`). The `stateEvents` deviation is particularly impactful: callers who expected `AnyPublisher<PiState, Never>` must now write a `.compactMap` step to extract `PiState` values from `ServerToClient` frames.
+
+**Recommendation:** Extract a `SessionConnectionProtocol` with the IC-2.1 signatures in a follow-up, or at minimum add a computed `var state: AnyPublisher<PiState, Never>` getter to `SessionConnection` that wraps `stateEvents`.
+
+---
+
+## Swift 6 Issues Found
+
+### 🔴 `WebSocketClient.ConnectionState` — compile error (see above)
+
+### ⚠️ `suspend()` marked `async` without any `await`
+
+```swift
+public func suspend() async {
+    client?.disconnect()
+    client = nil
+    cancellables.removeAll()
+    connectionState = .disconnected
+}
+```
+
+All four lines are synchronous. Marking the method `async` does not cause a compile error but:
+1. Forces callers to `await suspend()` unnecessarily.
+2. Causes a Swift warning: *"No 'async' operations occur within 'async' function"* under `-strict-concurrency=complete` (though as of Swift 5.10/6.0 this is a warning, not an error).
+
+**Fix:** Remove `async` from `suspend()`. Callers on `main` can call it directly; callers off-actor can `Task { @MainActor in … }`.
+
+### ⚠️ Resume-frame send captures `self` in a `Task` after `cancellables` might be cleared
+
+```swift
+ws.connectionState
+    .filter { $0 == .connected }
+    .first()
+    .sink { [weak self, weak ws, lastSeq] _ in
+        guard let self, let ws else { return }
+        Task { @MainActor [self, ws, lastSeq] in
+            try? await ws.send(.resume(lastSeq: lastSeq))
+            _ = self // silence unused-capture warning
+        }
+    }
+    .store(in: &cancellables)
+```
+
+If `suspend()` is called before the `Task` body runs (possible on a context switch), `cancellables.removeAll()` cancels the `AnyCancellable` but the already-spawned `Task` is **not** cancelled — it will still call `ws.send(…)` on the now-detached `ws`. In practice this is harmless (the WS will be disconnected), but it is a subtle race. A `Task` handle stored in the class and cancelled in `suspend()` would be cleaner.
+
+### ✅ `ScrollbackCache: @unchecked Sendable` — Correct
+
+The `DispatchQueue`-based serial queue provides the required mutual exclusion. `@unchecked Sendable` is the right annotation here since `DispatchQueue` itself is `Sendable` but the guarded mutable state is not automatically known to the compiler.
+
+### ✅ `NotificationDelegate` — `nonisolated` bridge callbacks
+
+`UNUserNotificationCenterDelegate` callbacks are correctly declared `nonisolated` with a `Task { @MainActor in … }` hop to access `visibleSessionId`. This is the correct pattern for Swift 6 under strict concurrency.
+
+### ✅ `DeviceTokenRegistrar` — `actor` isolation correct
+
+All mutable state is actor-isolated. `nonisolated static var environment` is a pure value derived at compile time via `#if DEBUG` and is safe from any isolation context.
+
+---
+
+## Naming / API Inconsistencies with T-2.1 Types
+
+### `WebSocketClient.connectionState` — `CurrentValueSubject` vs assumed `AnyPublisher`
+
+`WebSocketClient.connectionState` (T-2.1) is a `CurrentValueSubject<ConnectionState, Never>`, not an `AnyPublisher`. This is correct and fine — `CurrentValueSubject` is a `Publisher`. However, `SessionConnection` subscribes to it via `.filter { $0 == .connected }` which works; just note the type is not erased at the boundary.
+
+### `BinaryFrame.data` — correct usage
+
+`SessionConnection` calls `frame.data` on `BinaryFrame` values emitted by `ws.incomingBinary`. `BinaryFrame.data: Data` exists (T-2.1 definition). ✅
+
+### `WebSocketClientError.notConnected` — correct usage
+
+`SessionConnection.send(_:)` throws `WebSocketClientError.notConnected` — matches the T-2.1 enum case. ✅
+
+### `ClientToServer.resume(lastSeq:)` — correct usage
+
+Used in the resume handshake. T-2.1 defines `case resume(lastSeq: UInt64?)`. ✅
+
+---
+
+## T-2.9 Branch Location
+
+**Note:** The push files (`NotificationDelegate.swift`, `DeviceTokenRegistrar.swift`) were committed on `feat/p2-t2-5-session` (commit `a5c937a`), **not** on a standalone `feat/p2-t2-9-push` branch as expected. When checking out `feat/p2-t2-9-push`, those files are absent. This means:
+
+- PRs for T-2.5 and T-2.9 will contain code from both tasks.
+- The `feat/p2-t2-9-push` branch tip contains only earlier work (the test commit `89c27c0`).
+- **Action required:** either re-push T-2.9 files to a dedicated branch, or document in the PR that both T-2.5 and T-2.9 are delivered together.
+
+---
+
+## Merge Order Recommendation
+
+| Order | Branch | Reason |
+|-------|--------|--------|
+| 1 | `feat/p2-t2-1-websocket` *(already on main)* | Defines `ConnectionState`, `BinaryFrame`, `FrameCodec`, `WebSocketClient`, `ServerToClient` — foundation for all other branches |
+| 2 | `feat/p2-t2-4-modifierbar` | Pure UI, no dependencies on Sessions or Push layers |
+| 3 | `feat/p2-t2-5-session` | Depends on T-2.1 types (`WebSocketClient`, `BinaryFrame`, etc.); also contains T-2.9 files |
+| 4 | `feat/p2-tests-2` *(this branch)* | Can merge any time after T-2.1; imports `piRemote` which will contain all sources once T-2.4 and T-2.5 are merged |
+
+**Block on:** the `WebSocketClient.ConnectionState` compile error in `SessionConnection.swift` — this **must** be fixed before `feat/p2-t2-5-session` can be merged without breaking the build.
+
+---
+
+## Summary of Test Files Created
+
+| File | Tests | Coverage |
+|------|-------|----------|
+| `ModifierStateTests.swift` | 9 | Initial state, toggleCtrl arm/disarm/cycle/isolation, reset clear-all/idempotent/after-many-toggles |
+| `ScrollbackCacheTests.swift` | 11 | Append+read round-trip, ordered appends, sizeBytes, cap size/tail/small-chunks, clear zero/empty/then-append, isolation, empty-append no-op |
+| `DeviceTokenRegistrarTests.swift` | 8 | nil-before-registration, 2-byte / 1-byte / 4-byte hex, lowercase, UserDefaults persistence, environment valid, environment matches build config |
+
+**Total: 28 unit tests across 3 files.**

Tests/CoreTests/ScrollbackCacheTests.swift

@@ -0,0 +1,181 @@
+// ScrollbackCacheTests.swift
+// Unit tests for ScrollbackCache — the rolling 5 MB on-disk ANSI cache.
+//
+// Each test creates its own ScrollbackCache with a unique UUID session ID so
+// instances never share the same file. All test files are deleted in tearDown.
+//
+// Tests are synchronous because ScrollbackCache's public API is
+// synchronous (guarded internally by a serial DispatchQueue).
+
+import XCTest
+@testable import piRemote
+
+final class ScrollbackCacheTests: XCTestCase {
+
+    // Track session IDs created so tearDown can clean every one up.
+    private var sessionIds: [String] = []
+
+    override func setUp() {
+        super.setUp()
+        sessionIds = []
+    }
+
+    override func tearDown() {
+        // Remove every test cache file, regardless of test outcome.
+        for id in sessionIds {
+            ScrollbackCache(sessionId: id).clear()
+        }
+        sessionIds.removeAll()
+        super.tearDown()
+    }
+
+    // Convenience: create a cache for a fresh UUID session and track it.
+    private func makeCache() -> ScrollbackCache {
+        let id = UUID().uuidString
+        sessionIds.append(id)
+        return ScrollbackCache(sessionId: id)
+    }
+
+    // MARK: - Append + Read round-trip
+
+    /// A single `append` followed by `read` returns exactly the same bytes.
+    func testAppendAndRead_roundTrip() {
+        let cache = makeCache()
+        let data = Data("hello world".utf8)
+        cache.append(data)
+        XCTAssertEqual(cache.read(), data)
+    }
+
+    /// Multiple successive appends are concatenated in order.
+    func testMultipleAppends_areOrdered() {
+        let cache = makeCache()
+        cache.append(Data("foo".utf8))
+        cache.append(Data("bar".utf8))
+        XCTAssertEqual(cache.read(), Data("foobar".utf8))
+    }
+
+    /// `sizeBytes` reflects the total bytes written.
+    func testSizeBytes_reflectsAppend() {
+        let cache = makeCache()
+        let data = Data(repeating: 0xAB, count: 1_024)
+        cache.append(data)
+        XCTAssertEqual(cache.sizeBytes, 1_024)
+    }
+
+    // MARK: - Cap enforcement
+
+    private let fiveMB = 5 * 1024 * 1024
+
+    /// Writing > 5 MB of data keeps the on-disk size at or below 5 MB.
+    func testCap_sizeStaysWithinFiveMB() {
+        let cache = makeCache()
+        // Write three 2 MB chunks (6 MB total) — must trim.
+        let chunk = Data(repeating: 0xCC, count: 2 * 1_024 * 1_024)
+        cache.append(chunk)
+        cache.append(chunk)
+        cache.append(chunk)
+
+        XCTAssertLessThanOrEqual(
+            cache.sizeBytes, fiveMB,
+            "sizeBytes (\(cache.sizeBytes)) should be ≤ 5 MB after overflow"
+        )
+    }
+
+    /// After overflow, the last bytes in the file come from the newest chunk (oldest dropped).
+    func testCap_dropsOldestBytes() {
+        let cache = makeCache()
+        // 3 MB of 'A' then 3 MB of 'B' = 6 MB total; trim must keep the tail.
+        let chunkA = Data(repeating: 0x41, count: 3 * 1_024 * 1_024) // 'A'
+        let chunkB = Data(repeating: 0x42, count: 3 * 1_024 * 1_024) // 'B'
+        cache.append(chunkA)
+        cache.append(chunkB)
+
+        let result = cache.read()
+        XCTAssertFalse(result.isEmpty, "result must not be empty after overflow")
+        XCTAssertLessThanOrEqual(result.count, fiveMB)
+
+        // The tail of the retained data must be the newest bytes ('B').
+        XCTAssertEqual(result.last, 0x42, "last byte should be from the newest chunk")
+        // The very first byte of 'A'-only content should have been trimmed.
+        XCTAssertEqual(result.first, 0x41,
+            "some 'A' bytes may remain at the head, but 'A'-only prefix was trimmed")
+    }
+
+    /// `read()` count stays ≤ 5 MB even after many small appends that accumulate.
+    func testCap_manySmallAppends() {
+        let cache = makeCache()
+        // 1 024 appends × 6 KB = ~6 MB
+        let chunk = Data(repeating: 0x55, count: 6 * 1_024)
+        for _ in 0..<1_024 {
+            cache.append(chunk)
+        }
+        XCTAssertLessThanOrEqual(cache.read().count, fiveMB)
+    }
+
+    // MARK: - clear
+
+    /// After `clear()`, `sizeBytes` is 0.
+    func testClear_zeroesSizeBytes() {
+        let cache = makeCache()
+        cache.append(Data(repeating: 0x00, count: 1_024))
+        XCTAssertGreaterThan(cache.sizeBytes, 0) // precondition
+
+        cache.clear()
+        XCTAssertEqual(cache.sizeBytes, 0)
+    }
+
+    /// After `clear()`, `read()` returns empty Data.
+    func testClear_emptiesData() {
+        let cache = makeCache()
+        cache.append(Data("test".utf8))
+        cache.clear()
+        XCTAssertTrue(cache.read().isEmpty)
+    }
+
+    /// Appending after `clear()` works as if the cache were freshly created.
+    func testClear_thenAppend_worksCorrectly() {
+        let cache = makeCache()
+        cache.append(Data("old".utf8))
+        cache.clear()
+        cache.append(Data("new".utf8))
+        XCTAssertEqual(cache.read(), Data("new".utf8))
+        XCTAssertEqual(cache.sizeBytes, 3)
+    }
+
+    // MARK: - Isolation between instances
+
+    /// Two caches with different session IDs do not share state.
+    func testTwoInstances_doNotInterfere() {
+        let cacheA = makeCache()
+        let cacheB = makeCache()
+
+        cacheA.append(Data("alpha".utf8))
+        cacheB.append(Data("beta".utf8))
+
+        XCTAssertEqual(cacheA.read(), Data("alpha".utf8))
+        XCTAssertEqual(cacheB.read(), Data("beta".utf8))
+    }
+
+    /// Clearing one cache does not affect another.
+    func testClearOneCache_doesNotAffectOther() {
+        let cacheA = makeCache()
+        let cacheB = makeCache()
+
+        cacheA.append(Data("kept".utf8))
+        cacheB.append(Data("cleared".utf8))
+        cacheB.clear()
+
+        XCTAssertEqual(cacheA.read(), Data("kept".utf8))
+        XCTAssertTrue(cacheB.read().isEmpty)
+    }
+
+    // MARK: - Edge cases
+
+    /// Appending empty Data is a no-op (no file created, size stays 0).
+    func testAppendEmptyData_isNoOp() {
+        let cache = makeCache()
+        cache.append(Data())
+        XCTAssertEqual(cache.sizeBytes, 0)
+        XCTAssertTrue(cache.read().isEmpty)
+    }
+}