From bb399e6d35cddaa1f4c19ee62bb15edf7d085f1a Mon Sep 17 00:00:00 2001 From: Alan Wizemann Date: Fri, 24 Apr 2026 13:55:31 +0200 Subject: [PATCH] M9 #2+#4: ServerListView root + ServerID-aware onboarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScarfGo now boots into a list of configured servers instead of the single-server Dashboard. Each row renders nickname + user@host:port, taps to connect, swipes to forget. A "+" toolbar button re-enters onboarding for a new server. Fresh install → straight to onboarding. RootModel state machine redesigned around the multi-server world: - `.loading` → `.serverList` when listAll() returns 1+ servers. - `.loading` → `.onboarding(forNewServer:)` on fresh install. - `.serverList` → `.onboarding(newID)` via "+" button. - `.serverList` → `.connected(id, config, key)` via row tap. - `.connected(id)` → `.serverList` via soft Disconnect (keeps creds). - `.connected(id)` → `.serverList|.onboarding` via Forget (wipes id). - `.onboarding` → `.connected(newID, …)` on completion. Published `servers: [ServerID: IOSServerConfig]` on the RootModel so ServerListView renders reactively without re-querying stores on every re-render. `refreshServers()` is the `.task` hook; `forget()` wipes a single id + refreshes. OnboardingViewModel gains an optional `targetServerID` so its final save lands in `keyStore.save(_:for:)` / `configStore.save(_:id:)` instead of the singleton shims. Nil falls back to the old singleton path for any remaining callers (tests, previews). OnboardingRootView accepts `targetServerID` + a new `onCancel` closure. The toolbar now shows Cancel so users can back out without leaving half-written credentials; Cancel hides on the final .connected step so you can't race-cancel a just-saved server. ScarfGoTabRoot takes the server's ServerID as the context id so the CitadelServerTransport pool caches per-server (two active servers → two connection holders, no SSH channel contention). Splits the v1 onDisconnect into two callbacks: - onSoftDisconnect: close transport, return to server list, keep creds. - onForget: wipe this server's creds + return to server list (or onboarding if empty). MoreTab renders both Disconnect and Forget rows in distinct sections with explicit footers. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Security/OnboardingViewModel.swift | 22 ++- scarf/Scarf iOS/App/ScarfGoTabRoot.swift | 58 ++++-- scarf/Scarf iOS/App/ScarfIOSApp.swift | 154 +++++++++++++--- .../Onboarding/OnboardingRootView.swift | 52 +++++- scarf/Scarf iOS/Servers/ServerListView.swift | 167 ++++++++++++++++++ 5 files changed, 404 insertions(+), 49 deletions(-) create mode 100644 scarf/Scarf iOS/Servers/ServerListView.swift diff --git a/scarf/Packages/ScarfCore/Sources/ScarfCore/Security/OnboardingViewModel.swift b/scarf/Packages/ScarfCore/Sources/ScarfCore/Security/OnboardingViewModel.swift index e5aa053..e18df97 100644 --- a/scarf/Packages/ScarfCore/Sources/ScarfCore/Security/OnboardingViewModel.swift +++ b/scarf/Packages/ScarfCore/Sources/ScarfCore/Security/OnboardingViewModel.swift @@ -53,17 +53,25 @@ public final class OnboardingViewModel { private let configStore: any IOSServerConfigStore private let tester: any SSHConnectionTester private let keyGenerator: KeyGenerator + /// ServerID under which to save the key + config on completion. + /// Single-server v1 left this nil and the stores fell back to the + /// singleton APIs. M9 multi-server passes in a fresh ID from the + /// caller (or an existing ID when re-onboarding an existing row), + /// so the save lands in the right slot. + public let targetServerID: ServerID? public init( keyStore: any SSHKeyStore, configStore: any IOSServerConfigStore, tester: any SSHConnectionTester, - keyGenerator: @escaping KeyGenerator + keyGenerator: @escaping KeyGenerator, + targetServerID: ServerID? = nil ) { self.keyStore = keyStore self.configStore = configStore self.tester = tester self.keyGenerator = keyGenerator + self.targetServerID = targetServerID } // MARK: - Derived @@ -142,7 +150,11 @@ public final class OnboardingViewModel { defer { isWorking = false } do { - try await keyStore.save(bundle) + if let id = targetServerID { + try await keyStore.save(bundle, for: id) + } else { + try await keyStore.save(bundle) + } } catch { lastTestError = .other("Couldn't save key to Keychain: \(error.localizedDescription)") step = .testFailed(reason: lastTestError?.errorDescription ?? "Keychain save failed") @@ -190,7 +202,11 @@ public final class OnboardingViewModel { do { try await tester.testConnection(config: config, key: bundle) - try await configStore.save(config) + if let id = targetServerID { + try await configStore.save(config, id: id) + } else { + try await configStore.save(config) + } step = .connected } catch let err as SSHConnectionTestError { lastTestError = err diff --git a/scarf/Scarf iOS/App/ScarfGoTabRoot.swift b/scarf/Scarf iOS/App/ScarfGoTabRoot.swift index 0b0c648..16df113 100644 --- a/scarf/Scarf iOS/App/ScarfGoTabRoot.swift +++ b/scarf/Scarf iOS/App/ScarfGoTabRoot.swift @@ -19,19 +19,20 @@ import ScarfIOS /// push navigation (Cron editor, Memory detail, etc.) stays scoped /// to the tab instead of bleeding across. struct ScarfGoTabRoot: View { + let serverID: ServerID let config: IOSServerConfig let key: SSHKeyBundle - let onDisconnect: @MainActor () async -> Void - - /// Stable context UUID shared with DashboardView + ChatView. - /// Matches the prior convention so the CitadelServerTransport - /// connection pool reuses the same SSH client across tabs. - private static let sharedContextID: ServerID = ServerID( - uuidString: "00000000-0000-0000-0000-0000000000A1" - )! + let onSoftDisconnect: @MainActor () async -> Void + let onForget: @MainActor () async -> Void var body: some View { - let ctx = config.toServerContext(id: Self.sharedContextID) + // The transport factory is keyed by ServerID, so the correct + // Keychain slot + config is picked automatically. Reuses the + // server's own id as the context id so the CitadelServerTransport + // pool caches per-server (instead of the singleton we had + // pre-M9). Two active servers → two connection holders, no + // SSH channel contention. + let ctx = config.toServerContext(id: serverID) TabView { // 1 — Chat: the reason the app is on your phone. Primary // tab; opens straight into the chat surface. @@ -66,7 +67,11 @@ struct ScarfGoTabRoot: View { // label automatically; choosing the same word keeps our // More tab visually consistent with the system default. NavigationStack { - MoreTab(config: config, onDisconnect: onDisconnect) + MoreTab( + config: config, + onSoftDisconnect: onSoftDisconnect, + onForget: onForget + ) } .tabItem { Label("More", systemImage: "ellipsis.circle") @@ -88,10 +93,12 @@ struct ScarfGoTabRoot: View { /// deliberate design decision. private struct MoreTab: View { let config: IOSServerConfig - let onDisconnect: @MainActor () async -> Void + let onSoftDisconnect: @MainActor () async -> Void + let onForget: @MainActor () async -> Void @State private var showForgetConfirmation = false @State private var isForgetting = false + @State private var isDisconnecting = false var body: some View { List { @@ -126,6 +133,29 @@ private struct MoreTab: View { .scarfGoCompactListRow() } + Section { + Button { + Task { + isDisconnecting = true + await onSoftDisconnect() + } + } label: { + HStack { + Spacer() + if isDisconnecting { + ProgressView() + } else { + Text("Disconnect") + } + Spacer() + } + } + .disabled(isDisconnecting || isForgetting) + } footer: { + Text("Closes the live connection. Your key and host details stay on this device; tapping the server from the list reconnects with no re-onboarding.") + .font(.caption) + } + Section { Button(role: .destructive) { showForgetConfirmation = true @@ -140,7 +170,7 @@ private struct MoreTab: View { Spacer() } } - .disabled(isForgetting) + .disabled(isForgetting || isDisconnecting) } footer: { Text("Removes this server's SSH key and host info from the device. You'll need to add the public key back to `~/.ssh/authorized_keys` to reconnect.") .font(.caption) @@ -157,12 +187,12 @@ private struct MoreTab: View { Button("Forget \(config.displayName)", role: .destructive) { Task { isForgetting = true - await onDisconnect() + await onForget() } } Button("Cancel", role: .cancel) {} } message: { - Text("Your SSH key and host settings will be removed from this device. This cannot be undone.") + Text("Your SSH key and host settings for \(config.displayName) will be removed. Other servers stay configured. This cannot be undone.") } } } diff --git a/scarf/Scarf iOS/App/ScarfIOSApp.swift b/scarf/Scarf iOS/App/ScarfIOSApp.swift index bfb335e..12ddcee 100644 --- a/scarf/Scarf iOS/App/ScarfIOSApp.swift +++ b/scarf/Scarf iOS/App/ScarfIOSApp.swift @@ -66,17 +66,44 @@ struct ScarfIOSApp: App { } } -/// Decides whether the user needs to onboard or can see the Dashboard. +/// Decides what screen ScarfGo shows. M9 added the `.serverList` +/// state so users can manage multiple servers instead of being +/// stuck with a single-server app. Transitions: +/// +/// - `.loading` → `.serverList` when `load()` finds 1+ servers. +/// - `.loading` → `.onboarding(newID)` on fresh install. +/// - `.serverList` → `.onboarding(newID)` via the "+" button. +/// - `.serverList` → `.connected(id)` when the user taps a row. +/// - `.connected(id)` → `.serverList` via the "Disconnect" button +/// (soft — credentials kept). +/// - `.connected(id)` → `.serverList` via "Forget" (hard — wipes that +/// server's row from both stores). +/// - `.onboarding` → `.connected(newID)` on completion. @Observable @MainActor final class RootModel { - enum State { + enum State: Equatable { case loading - case onboarding - case connected(IOSServerConfig, SSHKeyBundle) + case serverList + case onboarding(forNewServer: ServerID) + case connected(ServerID, IOSServerConfig, SSHKeyBundle) + + static func == (lhs: State, rhs: State) -> Bool { + switch (lhs, rhs) { + case (.loading, .loading): return true + case (.serverList, .serverList): return true + case (.onboarding(let a), .onboarding(let b)): return a == b + case (.connected(let a, _, _), .connected(let b, _, _)): return a == b + default: return false + } + } } private(set) var state: State = .loading + /// Cached snapshot of all configured servers, keyed by ServerID. + /// Published so ServerListView can render reactively without + /// having to re-query stores on every re-render. + private(set) var servers: [ServerID: IOSServerConfig] = [:] private let keyStore: any SSHKeyStore private let configStore: any IOSServerConfigStore @@ -86,32 +113,103 @@ final class RootModel { self.configStore = configStore } + /// Load configured servers from disk and pick an initial state. func load() async { do { - let key = try await keyStore.load() - let cfg = try await configStore.load() - if let key, let cfg { - state = .connected(cfg, key) + let all = try await configStore.listAll() + servers = all + if all.isEmpty { + // Fresh install or user forgot every server → go + // straight to onboarding with a new ID reserved so + // completion writes under the right slot. + state = .onboarding(forNewServer: ServerID()) } else { - state = .onboarding + state = .serverList } } catch { - // Corrupted state → re-onboard. Logging would go here. - state = .onboarding + servers = [:] + state = .onboarding(forNewServer: ServerID()) } } - /// Called from OnboardingView when the flow reaches `.connected`. - /// Re-reads the stores and flips the root state. - func onboardingFinished() async { - await load() + /// Refresh the server list without disturbing `state`. Call from + /// ServerListView `.task` on appear so just-added servers show up + /// immediately. + func refreshServers() async { + servers = (try? await configStore.listAll()) ?? [:] } - /// Called from Dashboard "Disconnect" to wipe state and restart onboarding. + /// Start onboarding for a new server. The UI passes us the + /// ServerID we reserved at that moment so the completion handler + /// writes to the right slot. + func beginAddServer() { + state = .onboarding(forNewServer: ServerID()) + } + + /// Cancel an in-progress onboarding and return to the list. + /// Called by the sheet's Cancel affordance. + func cancelOnboarding() { + state = servers.isEmpty ? .onboarding(forNewServer: ServerID()) : .serverList + } + + /// Called from OnboardingView when the flow finishes. Reload the + /// list and transition to `.connected` for the just-added server, + /// or back to `.serverList` if we can't find it (defensive). + func onboardingFinished(serverID: ServerID) async { + servers = (try? await configStore.listAll()) ?? [:] + if let config = servers[serverID], + let key = try? await keyStore.load(for: serverID) { + state = .connected(serverID, config, key) + } else { + state = .serverList + } + } + + /// Tap a server row → connect. Loads fresh from disk to catch any + /// edits made through the Mac app (or future multi-device scenarios). + func connect(to id: ServerID) async { + var diskConfig: IOSServerConfig? = servers[id] + if diskConfig == nil { + diskConfig = try? await configStore.load(id: id) + } + let diskKey: SSHKeyBundle? = try? await keyStore.load(for: id) + guard let config = diskConfig, let key = diskKey else { + // Missing key → force re-onboarding under this ID so the + // user can regenerate without losing host/user/port. + state = .onboarding(forNewServer: id) + return + } + state = .connected(id, config, key) + } + + /// Soft disconnect: close any live transport but keep stored + /// credentials. Returns to the server list so the user can tap + /// another server (or the same one again). + func softDisconnect() async { + // Transport teardown is owned by ConnectedServerRegistry + // (added in 3.3); for now the per-view controllers own their + // own lifecycles via .onDisappear, so this is mostly a state + // change. The registry commit will thread through here. + state = .serverList + } + + /// Hard forget: wipe the specified server's key + config, refresh + /// the list, transition to serverList (or onboarding if empty). + func forget(id: ServerID) async { + try? await keyStore.delete(for: id) + try? await configStore.delete(id: id) + servers = (try? await configStore.listAll()) ?? [:] + state = servers.isEmpty ? .onboarding(forNewServer: ServerID()) : .serverList + } + + /// Legacy v1 "Disconnect" that wipes EVERYTHING. Kept for back-compat + /// with any caller that still hits the no-arg path (there shouldn't + /// be any after 3.5 lands, but the protocol still supports it). func disconnect() async { try? await keyStore.delete() try? await configStore.delete() - state = .onboarding + servers = [:] + state = .onboarding(forNewServer: ServerID()) } } @@ -122,16 +220,24 @@ struct RootView: View { switch model.state { case .loading: ProgressView("Loading…") - case .onboarding: - OnboardingRootView(onFinished: { - await model.onboardingFinished() - }) - case .connected(let config, let key): + case .serverList: + ServerListView(model: model) + case .onboarding(let forNewServer): + OnboardingRootView(targetServerID: forNewServer) { + await model.onboardingFinished(serverID: forNewServer) + } onCancel: { + model.cancelOnboarding() + } + case .connected(let id, let config, let key): ScarfGoTabRoot( + serverID: id, config: config, key: key, - onDisconnect: { - await model.disconnect() + onSoftDisconnect: { + await model.softDisconnect() + }, + onForget: { + await model.forget(id: id) } ) } diff --git a/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift b/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift index d5e5cef..73e008a 100644 --- a/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift +++ b/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift @@ -6,18 +6,37 @@ import ScarfIOS /// Each step gets its own small view; the view switch is driven by /// `vm.step`. struct OnboardingRootView: View { + /// ServerID under which this onboarding run writes the key + + /// config. M9: the ServerListView reserves a fresh ID when the + /// user taps "+"; the RootModel passes it through to us; we pass + /// it into OnboardingViewModel which uses the ID-keyed store APIs. + let targetServerID: ServerID let onFinished: @MainActor () async -> Void + /// Invoked when the user cancels before completing. M9: pops us + /// back to the server list instead of leaving the user stuck on + /// step 1 with nowhere to go. Optional for callers that don't + /// need cancel (shouldn't be any, but keeps the API forgiving). + let onCancel: @MainActor () -> Void - @State private var vm: OnboardingViewModel = { - let tester = CitadelSSHService() - let service = tester // reuse the same instance for key generation - return OnboardingViewModel( + @State private var vm: OnboardingViewModel + + init( + targetServerID: ServerID, + onFinished: @escaping @MainActor () async -> Void, + onCancel: @escaping @MainActor () -> Void = {} + ) { + self.targetServerID = targetServerID + self.onFinished = onFinished + self.onCancel = onCancel + let service = CitadelSSHService() + _vm = State(initialValue: OnboardingViewModel( keyStore: KeychainSSHKeyStore(), configStore: UserDefaultsIOSServerConfigStore(), - tester: tester, - keyGenerator: { try service.generateEd25519Key() } - ) - }() + tester: service, + keyGenerator: { try service.generateEd25519Key() }, + targetServerID: targetServerID + )) + } var body: some View { NavigationStack { @@ -35,6 +54,23 @@ struct OnboardingRootView: View { } .navigationTitle("Connect to Hermes") .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .topBarLeading) { + // Cancel only makes sense while we haven't yet + // completed — once the connection-test passes we + // auto-forward to onFinished so there's nothing + // to cancel. Hiding the button then also keeps + // users from accidentally wiping a just-saved + // server mid-race. + if case .connected = vm.step { + EmptyView() + } else { + Button("Cancel") { + onCancel() + } + } + } + } } .onChange(of: vm.step) { _, new in if case .connected = new { diff --git a/scarf/Scarf iOS/Servers/ServerListView.swift b/scarf/Scarf iOS/Servers/ServerListView.swift new file mode 100644 index 0000000..851ccaa --- /dev/null +++ b/scarf/Scarf iOS/Servers/ServerListView.swift @@ -0,0 +1,167 @@ +import SwiftUI +import ScarfCore +import ScarfIOS + +/// ScarfGo's root surface when the user has at least one server +/// configured. Replaces the pre-M9 "boot straight into Dashboard" +/// flow — which worked while the app was single-server, but had +/// nowhere to put a second host once multi-server landed. +/// +/// Each row shows nickname + host and navigates to ScarfGoTabRoot +/// on tap. The "+" toolbar button re-enters onboarding for a new +/// server. Swipe → Forget (destructive, with confirmation) so users +/// can prune without going into the More tab. +struct ServerListView: View { + let model: RootModel + + @State private var serverPendingForget: ServerRow? + + var body: some View { + NavigationStack { + List { + Section { + ForEach(sortedServers, id: \.id) { row in + ServerListRow(row: row) { + Task { await model.connect(to: row.id) } + } + .scarfGoCompactListRow() + .swipeActions(edge: .trailing, allowsFullSwipe: false) { + Button(role: .destructive) { + serverPendingForget = row + } label: { + Label("Forget", systemImage: "trash") + } + } + } + } footer: { + Text("Tap a server to connect. Swipe for more actions.") + .font(.caption) + } + } + .scarfGoListDensity() + .navigationTitle("Servers") + .navigationBarTitleDisplayMode(.inline) + .toolbar { + ToolbarItem(placement: .topBarTrailing) { + Button { + model.beginAddServer() + } label: { + Label("Add server", systemImage: "plus.circle.fill") + } + } + } + .task { await model.refreshServers() } + .confirmationDialog( + "Forget this server?", + isPresented: forgetBinding, + titleVisibility: .visible, + presenting: serverPendingForget + ) { row in + Button("Forget \(row.displayName)", role: .destructive) { + Task { + await model.forget(id: row.id) + serverPendingForget = nil + } + } + Button("Cancel", role: .cancel) { + serverPendingForget = nil + } + } message: { row in + Text("Removes \(row.displayName)'s SSH key and host details from this device. Other servers stay configured.") + } + } + } + + /// View-model carrier — Identifiable + stable sort key. Fileprivate + /// so the ServerListRow subview in this same file can reference + /// it; the rest of the module doesn't need it. + fileprivate struct ServerRow: Identifiable, Hashable { + let id: ServerID + let displayName: String + let host: String + let port: Int? + let user: String? + } + + /// Project the model's `servers` dict into a sortable list. + /// Alphabetical by display name so the ordering is deterministic + /// and matches what users see in the picker. + private var sortedServers: [ServerRow] { + model.servers + .map { id, config in + ServerRow( + id: id, + displayName: config.displayName, + host: config.host, + port: config.port, + user: config.user + ) + } + .sorted { $0.displayName.localizedCaseInsensitiveCompare($1.displayName) == .orderedAscending } + } + + /// `.confirmationDialog(isPresented:)` wants a Bool binding; map + /// `serverPendingForget` to one so the dialog dismisses when we + /// clear the optional. + private var forgetBinding: Binding { + Binding( + get: { serverPendingForget != nil }, + set: { newValue in + if !newValue { serverPendingForget = nil } + } + ) + } +} + +private struct ServerListRow: View { + let row: ServerListView.ServerRow + let onTap: () -> Void + + var body: some View { + Button(action: onTap) { + HStack(alignment: .center, spacing: 12) { + Image(systemName: "server.rack") + .font(.title3) + .foregroundStyle(.tint) + .frame(width: 28) + VStack(alignment: .leading, spacing: 2) { + Text(row.displayName) + .font(.body) + .fontWeight(.medium) + .foregroundStyle(.primary) + Text(hostLine) + .font(.caption) + .foregroundStyle(.secondary) + } + Spacer() + Image(systemName: "chevron.right") + .font(.caption2) + .foregroundStyle(.tertiary) + } + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + } + + /// Second-row subtitle: `user@host:port` when fully specified, + /// else whichever pieces are known. + private var hostLine: String { + var parts: [String] = [] + if let user = row.user, !user.isEmpty { + parts.append("\(user)@\(row.host)") + } else { + parts.append(row.host) + } + if let port = row.port, port != 22 { + parts[parts.count - 1] += ":\(port)" + } + return parts.joined(separator: " ") + } +} + +// ServerRow needs to live outside the private struct for the +// confirmationDialog(presenting:) closure to reference it. Swift's +// type scoping won't let us put Identifiable conformance on a nested +// struct that's used from the view's top level; we accept a small +// scope leak. +extension ServerListView.ServerRow: @unchecked Sendable {}