diff --git a/.gitignore b/.gitignore index 7587cd7..fdc4e65 100644 --- a/.gitignore +++ b/.gitignore @@ -181,5 +181,8 @@ apps/native-macos/.build/ apps/native-macos/.swiftpm/ apps/native-macos/*.xcodeproj +# Instruments trace captures (local performance investigation artifacts) +apps/native-macos/*.trace/ + .agents/** .claude/skills/** diff --git a/apps/native-macos/Sources/App/Main.swift b/apps/native-macos/Sources/App/Main.swift index dabb16f..f4edaf5 100644 --- a/apps/native-macos/Sources/App/Main.swift +++ b/apps/native-macos/Sources/App/Main.swift @@ -5,7 +5,7 @@ import Foundation struct Main { static func main() async { guard let spike = ProcessInfo.processInfo.environment["MCV_SPIKE"] else { - print("MC-Vector Native starting…") + MCVectorApp.main() return } diff --git a/apps/native-macos/Sources/Core/ActivityDrawerView.swift b/apps/native-macos/Sources/Core/ActivityDrawerView.swift new file mode 100644 index 0000000..686c434 --- /dev/null +++ b/apps/native-macos/Sources/Core/ActivityDrawerView.swift @@ -0,0 +1,46 @@ +import SwiftUI + +/// Content of the Activity Drawer -- a global (all-servers) view of +/// `ServerListViewModel.activityLog`, presented via `.inspector` from +/// `RootView`. +/// +/// Deliberately a plain `List`/`ForEach`, not the `ScrollView` + `LazyVStack` +/// pattern task 3-8's console output needed: that pattern exists for a +/// high-frequency, potentially-thousands-of-lines stream where `List`'s +/// diffing overhead matters. `activityLog` is bounded (see +/// `ServerListViewModel.activityLogCap`) and updates far less often (one +/// entry per process start/stop/crash, not per log line), so `List`'s +/// automatic diffing is the right, standard, simple tool here -- and it +/// (like `.inspector` and `.toolbar`) gets Liquid Glass styling automatically +/// just by being used, with no manual `.glassEffect` needed. +struct ActivityDrawerView: View { + let entries: [ActivityEntry] + + var body: some View { + Group { + if self.entries.isEmpty { + ContentUnavailableView( + "No Activity Yet", + systemImage: "clock.arrow.circlepath", + description: Text("Server start, stop, and crash events will appear here."), + ) + } else { + List(self.entries) { entry in + ActivityRow(entry: entry) + } + } + } + .navigationTitle("Activity") + } +} + +#Preview { + ActivityDrawerView(entries: [ + ActivityEntry(serverId: "srv-1", serverName: "Survival", kind: .serverStatusChange(.offline)), + ActivityEntry(serverId: "srv-2", serverName: "Creative", kind: .serverStatusChange(.crashed)) + ]) +} + +#Preview("Empty") { + ActivityDrawerView(entries: []) +} diff --git a/apps/native-macos/Sources/Core/ActivityEntry.swift b/apps/native-macos/Sources/Core/ActivityEntry.swift new file mode 100644 index 0000000..349c350 --- /dev/null +++ b/apps/native-macos/Sources/Core/ActivityEntry.swift @@ -0,0 +1,55 @@ +import Foundation + +/// A single entry in `ServerListViewModel.activityLog` -- a lightweight, +/// display-only record of something that happened to a tracked server. +/// +/// Unlike `Server` (which mirrors a TypeScript domain type over a JSON wire +/// format, and so constrains dates to ISO8601 `String`s -- see +/// `Server.createdDate`), `ActivityEntry` has no cross-language contract and +/// is never persisted (see `ServerListViewModel.activityLog`'s doc comment +/// for why): a plain `Date` is the right, simple choice for `timestamp`. +public struct ActivityEntry: Sendable, Identifiable, Equatable { + /// What kind of thing happened. Currently only tracks process status + /// changes -- both those observed via `ServerProcessService.events` + /// (`.offline`/`.crashed`, applied by `ServerListViewModel.apply(_:)`) + /// and the synchronous `.online` transition `ServerListViewModel + /// .startSelectedServer()` logs directly on a successful start. Both + /// paths construct entries through `ServerListViewModel + /// .appendActivity(forServerId:status:)`. + /// + /// Deliberately an enum rather than folding `ServerStatus` directly into + /// `ActivityEntry` -- this app has no backup feature/service implemented + /// anywhere yet (`Server.autoBackup*` fields exist on the domain model, + /// but nothing schedules or runs a backup), so there is nothing to log + /// for it today. `Kind` exists so a future case such as + /// `.backupCompleted` can be added later without reshaping + /// `ActivityEntry` itself or this file's callers. + public enum Kind: Sendable, Equatable { + case serverStatusChange(ServerStatus) + } + + public let id: UUID + public let serverId: String + /// Resolved from `ServerListViewModel.servers` at append time (not + /// looked up lazily when the drawer renders) so a server later removed + /// from `servers` can't leave this entry with a dangling reference -- + /// the drawer can always render a name for a historical entry, even for + /// a server that no longer exists. + public let serverName: String + public let kind: Kind + public let timestamp: Date + + public init( + id: UUID = UUID(), + serverId: String, + serverName: String, + kind: Kind, + timestamp: Date = Date(), + ) { + self.id = id + self.serverId = serverId + self.serverName = serverName + self.kind = kind + self.timestamp = timestamp + } +} diff --git a/apps/native-macos/Sources/Core/ActivityRow.swift b/apps/native-macos/Sources/Core/ActivityRow.swift new file mode 100644 index 0000000..2d09723 --- /dev/null +++ b/apps/native-macos/Sources/Core/ActivityRow.swift @@ -0,0 +1,83 @@ +import SwiftUI + +/// A single row in `ActivityDrawerView`'s `List`, rendering one +/// `ActivityEntry`. +/// +/// Split into its own file (task 3-12 code-review fix) -- previously +/// declared alongside `ActivityDrawerView` in `ActivityDrawerView.swift`, +/// violating this codebase's one-type-per-file convention that every other +/// View in this phase already follows. Left at the default `internal` +/// visibility (not `private`, since `private` would no longer be usable from +/// `ActivityDrawerView.swift` once split out; not `public`, since nothing +/// outside this module constructs a row directly -- `ActivityDrawerView` is +/// the sole caller) -- an implementation detail of the drawer, not part of +/// this package's public API. +struct ActivityRow: View { + let entry: ActivityEntry + + var body: some View { + HStack(spacing: 10) { + Image(systemName: self.systemImage) + .foregroundStyle(self.tint) + .frame(width: 20) + + VStack(alignment: .leading, spacing: 2) { + Text(self.entry.serverName) + .font(.body) + Text(self.statusLabel) + .font(.caption) + .foregroundStyle(.secondary) + } + + Spacer() + + // Standard SwiftUI relative-date `Text` -- auto-updates ("2m + // ago" -> "3m ago") without any manual timer/formatter code. + Text(self.entry.timestamp, style: .relative) + .font(.caption) + .foregroundStyle(.secondary) + .monospacedDigit() + } + .accessibilityElement(children: .combine) + } + + private var status: ServerStatus { + switch self.entry.kind { + case let .serverStatusChange(status): status + } + } + + private var statusLabel: String { + self.status.rawValue.capitalized + } + + private var systemImage: String { + switch self.status { + case .online: "play.circle.fill" + case .offline: "stop.circle" + case .starting, .restarting: "arrow.triangle.2.circlepath.circle" + case .stopping: "stop.circle.fill" + case .crashed: "exclamationmark.triangle.fill" + } + } + + private var tint: Color { + switch self.status { + case .online: .green + case .offline: .secondary + case .starting, .restarting, .stopping: .orange + case .crashed: .red + } + } +} + +#Preview { + List { + ActivityRow( + entry: ActivityEntry(serverId: "srv-1", serverName: "Survival", kind: .serverStatusChange(.online)), + ) + ActivityRow( + entry: ActivityEntry(serverId: "srv-2", serverName: "Creative", kind: .serverStatusChange(.crashed)), + ) + } +} diff --git a/apps/native-macos/Sources/Core/Domain/AutoBackupScheduleType.swift b/apps/native-macos/Sources/Core/Domain/AutoBackupScheduleType.swift new file mode 100644 index 0000000..a45bd39 --- /dev/null +++ b/apps/native-macos/Sources/Core/Domain/AutoBackupScheduleType.swift @@ -0,0 +1,11 @@ +/// How an automatic backup schedule is expressed for a server. +/// +/// Mirrors the TypeScript `autoBackupScheduleType` union +/// (`'interval' | 'daily' | 'weekly'`) shared by `MinecraftServer` and +/// `ServerTemplate` in `src/renderer/shared/server declaration.ts` / +/// `src/lib/server-commands.ts`. +public enum AutoBackupScheduleType: String, Codable, Sendable, Equatable, CaseIterable { + case interval + case daily + case weekly +} diff --git a/apps/native-macos/Sources/Core/Domain/Server.swift b/apps/native-macos/Sources/Core/Domain/Server.swift new file mode 100644 index 0000000..37942b3 --- /dev/null +++ b/apps/native-macos/Sources/Core/Domain/Server.swift @@ -0,0 +1,98 @@ +/// A running (or runnable) Minecraft server instance. +/// +/// Mirrors the TypeScript `MinecraftServer` interface in +/// `src/renderer/shared/server declaration.ts` on the Tauri Classic side. +/// JSON keys match the TS property names exactly (camelCase, no +/// snake_case conversion), so `Codable` synthesis lines up automatically +/// without custom `CodingKeys`. +/// +/// `createdDate` is kept as a plain ISO-8601 `String` rather than decoded +/// into `Date` — the JSON on disk stores it as a string, and introducing a +/// custom `JSONDecoder.dateDecodingStrategy` is out of scope for this task. +public struct Server: Codable, Sendable, Equatable, Identifiable { + public var id: String + public var name: String + public var profileName: String? + public var groupName: String? + public var version: String + public var software: String + public var port: Int + public var memory: Int + public var path: String + public var status: ServerStatus + public var javaPath: String? + public var autoRestartOnCrash: Bool? + public var maxAutoRestarts: Int? + public var autoRestartDelaySec: Int? + public var autoBackupEnabled: Bool? + public var autoBackupIntervalMin: Int? + public var autoBackupScheduleType: AutoBackupScheduleType? + public var autoBackupTime: String? + public var autoBackupWeekday: Int? + public var autoBackupRetainCount: Int? + public var autoBackupRetainDays: Int? + public var createdDate: String? + public var jvmArgs: String? + public var notifyOnCrash: Bool? + public var notifyOnStart: Bool? + public var notifyOnHighCpu: Bool? + public var notifyHighCpuThreshold: Int? + + public init( + id: String, + name: String, + profileName: String? = nil, + groupName: String? = nil, + version: String, + software: String, + port: Int, + memory: Int, + path: String, + status: ServerStatus, + javaPath: String? = nil, + autoRestartOnCrash: Bool? = nil, + maxAutoRestarts: Int? = nil, + autoRestartDelaySec: Int? = nil, + autoBackupEnabled: Bool? = nil, + autoBackupIntervalMin: Int? = nil, + autoBackupScheduleType: AutoBackupScheduleType? = nil, + autoBackupTime: String? = nil, + autoBackupWeekday: Int? = nil, + autoBackupRetainCount: Int? = nil, + autoBackupRetainDays: Int? = nil, + createdDate: String? = nil, + jvmArgs: String? = nil, + notifyOnCrash: Bool? = nil, + notifyOnStart: Bool? = nil, + notifyOnHighCpu: Bool? = nil, + notifyHighCpuThreshold: Int? = nil + ) { + self.id = id + self.name = name + self.profileName = profileName + self.groupName = groupName + self.version = version + self.software = software + self.port = port + self.memory = memory + self.path = path + self.status = status + self.javaPath = javaPath + self.autoRestartOnCrash = autoRestartOnCrash + self.maxAutoRestarts = maxAutoRestarts + self.autoRestartDelaySec = autoRestartDelaySec + self.autoBackupEnabled = autoBackupEnabled + self.autoBackupIntervalMin = autoBackupIntervalMin + self.autoBackupScheduleType = autoBackupScheduleType + self.autoBackupTime = autoBackupTime + self.autoBackupWeekday = autoBackupWeekday + self.autoBackupRetainCount = autoBackupRetainCount + self.autoBackupRetainDays = autoBackupRetainDays + self.createdDate = createdDate + self.jvmArgs = jvmArgs + self.notifyOnCrash = notifyOnCrash + self.notifyOnStart = notifyOnStart + self.notifyOnHighCpu = notifyOnHighCpu + self.notifyHighCpuThreshold = notifyHighCpuThreshold + } +} diff --git a/apps/native-macos/Sources/Core/Domain/ServerStatus.swift b/apps/native-macos/Sources/Core/Domain/ServerStatus.swift new file mode 100644 index 0000000..6b8b2ab --- /dev/null +++ b/apps/native-macos/Sources/Core/Domain/ServerStatus.swift @@ -0,0 +1,14 @@ +/// Lifecycle state of a Minecraft server instance. +/// +/// Mirrors the TypeScript `ServerStatus` union in +/// `src/renderer/shared/server declaration.ts` on the Tauri Classic side. +/// The raw string values are the JSON wire format and must stay in sync +/// with that union. +public enum ServerStatus: String, Codable, Sendable, Equatable, CaseIterable { + case online + case offline + case starting + case stopping + case restarting + case crashed +} diff --git a/apps/native-macos/Sources/Core/Domain/ServerStore.swift b/apps/native-macos/Sources/Core/Domain/ServerStore.swift new file mode 100644 index 0000000..0aa649a --- /dev/null +++ b/apps/native-macos/Sources/Core/Domain/ServerStore.swift @@ -0,0 +1,67 @@ +import Foundation + +/// Minimal, independent reader/writer for the Native app's own +/// `servers.json`. +/// +/// This does not interoperate with the Tauri Classic app's on-disk store — +/// the Native app manages its own file, whose location is injected as a +/// `URL` so tests can point it at a temp directory. This is intentionally +/// thin: just enough to prove `Server`/`ServerTemplate` round-trip through +/// JSON and that a real file can be read and written. CRUD/UI wiring is +/// out of scope for this task. +public actor ServerStore { + private let fileURL: URL + + public init(fileURL: URL) { + self.fileURL = fileURL + } + + public func load() throws -> ServersFile { + let data = try Data(contentsOf: self.fileURL) + return try JSONDecoder().decode(ServersFile.self, from: data) + } + + public func save(_ file: ServersFile) throws { + let encoder = JSONEncoder() + encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + let data = try encoder.encode(file) + try data.write(to: self.fileURL, options: .atomic) + } +} + +extension ServerStore { + /// Subdirectory name under Application Support that holds this app's + /// own on-disk data. Distinct from the Tauri Classic app's storage. + private static let applicationSupportSubdirectoryName = "MC-Vector Native" + + /// Resolves the production `servers.json` location under the current + /// user's Application Support directory, creating the parent directory + /// first if it doesn't exist yet. + /// + /// Non-throwing by design so it can back a `View`'s `@State` default + /// initializer directly: directory resolution/creation failures here + /// are exceedingly rare (a missing/unwritable Application Support + /// directory would already be breaking most of macOS), and any real + /// failure still surfaces later as a `ServerStore.load()`/`save()` + /// error, which callers already handle. + /// + /// Tests must not call this -- it touches the real Application Support + /// directory on the machine running the tests. Point a `ServerStore` at + /// a temp file instead (see `ServerStoreTests`). + public static func defaultFileURL(fileManager: FileManager = .default) -> URL { + let supportDirectory = (try? fileManager.url( + for: .applicationSupportDirectory, + in: .userDomainMask, + appropriateFor: nil, + create: true, + )) ?? fileManager.temporaryDirectory + + let appDirectory = supportDirectory.appendingPathComponent( + self.applicationSupportSubdirectoryName, + isDirectory: true, + ) + try? fileManager.createDirectory(at: appDirectory, withIntermediateDirectories: true) + + return appDirectory.appendingPathComponent("servers.json", isDirectory: false) + } +} diff --git a/apps/native-macos/Sources/Core/Domain/ServerTemplate.swift b/apps/native-macos/Sources/Core/Domain/ServerTemplate.swift new file mode 100644 index 0000000..de8d6ce --- /dev/null +++ b/apps/native-macos/Sources/Core/Domain/ServerTemplate.swift @@ -0,0 +1,63 @@ +/// A reusable server configuration that has not been instantiated yet. +/// +/// Mirrors the TypeScript `ServerTemplate` interface in +/// `src/lib/server-commands.ts` — the same shape as `Server`/ +/// `MinecraftServer` minus `path` and `status`, since a template isn't a +/// running (or even provisioned) instance. +public struct ServerTemplate: Codable, Sendable, Equatable { + public var id: String + public var name: String + public var profileName: String? + public var groupName: String? + public var version: String + public var software: String + public var port: Int + public var memory: Int + public var javaPath: String? + public var autoRestartOnCrash: Bool? + public var maxAutoRestarts: Int? + public var autoRestartDelaySec: Int? + public var autoBackupEnabled: Bool? + public var autoBackupIntervalMin: Int? + public var autoBackupScheduleType: AutoBackupScheduleType? + public var autoBackupTime: String? + public var autoBackupWeekday: Int? + + public init( + id: String, + name: String, + profileName: String? = nil, + groupName: String? = nil, + version: String, + software: String, + port: Int, + memory: Int, + javaPath: String? = nil, + autoRestartOnCrash: Bool? = nil, + maxAutoRestarts: Int? = nil, + autoRestartDelaySec: Int? = nil, + autoBackupEnabled: Bool? = nil, + autoBackupIntervalMin: Int? = nil, + autoBackupScheduleType: AutoBackupScheduleType? = nil, + autoBackupTime: String? = nil, + autoBackupWeekday: Int? = nil + ) { + self.id = id + self.name = name + self.profileName = profileName + self.groupName = groupName + self.version = version + self.software = software + self.port = port + self.memory = memory + self.javaPath = javaPath + self.autoRestartOnCrash = autoRestartOnCrash + self.maxAutoRestarts = maxAutoRestarts + self.autoRestartDelaySec = autoRestartDelaySec + self.autoBackupEnabled = autoBackupEnabled + self.autoBackupIntervalMin = autoBackupIntervalMin + self.autoBackupScheduleType = autoBackupScheduleType + self.autoBackupTime = autoBackupTime + self.autoBackupWeekday = autoBackupWeekday + } +} diff --git a/apps/native-macos/Sources/Core/Domain/ServersFile.swift b/apps/native-macos/Sources/Core/Domain/ServersFile.swift new file mode 100644 index 0000000..a83e3e0 --- /dev/null +++ b/apps/native-macos/Sources/Core/Domain/ServersFile.swift @@ -0,0 +1,13 @@ +/// Top-level shape of the on-disk `servers.json` document. +/// +/// Matches the object the Tauri Classic app persists via +/// `@tauri-apps/plugin-store`: `{ "servers": [...], "serverTemplates": [...] }`. +public struct ServersFile: Codable, Sendable, Equatable { + public var servers: [Server] + public var serverTemplates: [ServerTemplate] + + public init(servers: [Server] = [], serverTemplates: [ServerTemplate] = []) { + self.servers = servers + self.serverTemplates = serverTemplates + } +} diff --git a/apps/native-macos/Sources/Core/FloatingConsoleContentView.swift b/apps/native-macos/Sources/Core/FloatingConsoleContentView.swift new file mode 100644 index 0000000..1609cef --- /dev/null +++ b/apps/native-macos/Sources/Core/FloatingConsoleContentView.swift @@ -0,0 +1,54 @@ +import SwiftUI + +/// Content hosted inside `FloatingConsolePanel`: a small header bar +/// identifying the server, above the shared `ServerLogView`. +/// +/// **Liquid Glass usage**: per `spec/native-macos-requirements.md` §5.4's +/// policy -- "機能レイヤー(toolbar/ナビ/コントロール)限定、コンテンツ本体には使わない" +/// (glass is for the functional/toolbar layer only, never the content body) +/// -- `.glassEffect` is applied *only* to `header` below, not to the log +/// content area. This is a deliberate departure from the Phase 3-A spike's +/// `GlassSpikeContent`, which applied `.glassEffect` to its entire content +/// view for demo simplicity; that shape was never the intended production +/// policy. The panel's own window background is left at the AppKit default +/// (opaque), matching the same policy. +/// +/// This also means the known `.nonactivatingPanel` + `.glassEffect` +/// degrade-to-blur-when-inactive bug (`spec/native-macos-requirements.md` +/// §5.4, `spec/phase3a-spike-results.md` §3-1) has minimal surface here: it +/// can only visibly affect this one small header bar, never the log +/// content most of the panel's area is spent on. +struct FloatingConsoleContentView: View { + let serverName: String + let viewModel: ServerLogViewModel + + var body: some View { + VStack(alignment: .leading, spacing: 0) { + self.header + Divider() + ServerLogView(viewModel: self.viewModel) + } + .frame(minWidth: 420, minHeight: 240) + } + + private var header: some View { + HStack(spacing: 8) { + Image(systemName: "terminal") + .foregroundStyle(.secondary) + Text(self.serverName) + .font(.headline) + Spacer() + } + .padding(.horizontal, 12) + .padding(.vertical, 8) + .glassEffect(.regular, in: .rect(cornerRadius: 10)) + .padding(8) + } +} + +#Preview { + FloatingConsoleContentView( + serverName: "Survival", + viewModel: ServerLogViewModel(serverId: "srv-1", processService: ServerProcessService()), + ) +} diff --git a/apps/native-macos/Sources/Core/FloatingConsolePanel.swift b/apps/native-macos/Sources/Core/FloatingConsolePanel.swift new file mode 100644 index 0000000..281ef17 --- /dev/null +++ b/apps/native-macos/Sources/Core/FloatingConsolePanel.swift @@ -0,0 +1,43 @@ +import AppKit +import SwiftUI + +/// Task 3-9's real floating console panel, built on the NSPanel bridge +/// confirmed by the Phase 3-A spike (`NonactivatingGlassPanel`; see +/// `Sources/Core/Spikes/PanelSpike/NonactivatingGlassPanel.swift` and +/// `spec/phase3a-spike-results.md` §3-1): `.nonactivatingPanel` + +/// `isFloatingPanel` + `NSHostingView`. On real hardware this reliably +/// detects app-inactive state, which is why it was chosen over the +/// rejected pure-SwiftUI `Window` + `WindowLevel` approach (that approach +/// couldn't detect a Dock-click deactivation, only Cmd+Tab-style ones; see +/// `spec/native-macos-requirements.md` §5.4). +/// +/// Style mask, `isFloatingPanel`, `level`, `collectionBehavior`, and +/// `titlebarAppearsTransparent` all match the spike's confirmed-winning +/// configuration exactly. Two deliberate differences from the spike: +/// +/// 1. **Generic over `Content`** (the spike's `NonactivatingGlassPanel` +/// hardcoded `GlassSpikeContent`) so this type can host the real, +/// per-server `FloatingConsoleContentView` rather than being reused +/// as-is for a fixed demo view. +/// 2. **Title bar shows a title** (`titleVisibility` left at its `.visible` +/// default; the spike set `.hidden` since its demo content had its own +/// title text). A real floating console needs to identify which +/// server it belongs to even before the panel's own header renders. +@MainActor +final class FloatingConsolePanel: NSPanel { + init(title: String, content: Content) { + super.init( + contentRect: NSRect(x: 0, y: 0, width: 480, height: 320), + styleMask: [.nonactivatingPanel, .titled, .resizable, .closable], + backing: .buffered, + defer: false, + ) + + self.isFloatingPanel = true + self.level = .floating + self.collectionBehavior = [.canJoinAllSpaces, .fullScreenAuxiliary] + self.titlebarAppearsTransparent = true + self.title = title + self.contentView = NSHostingView(rootView: content) + } +} diff --git a/apps/native-macos/Sources/Core/FloatingConsolePanelController.swift b/apps/native-macos/Sources/Core/FloatingConsolePanelController.swift new file mode 100644 index 0000000..cfe32d5 --- /dev/null +++ b/apps/native-macos/Sources/Core/FloatingConsolePanelController.swift @@ -0,0 +1,100 @@ +import AppKit +import Observation +import SwiftUI + +/// Owns a lazily-created `FloatingConsolePanel` for one server's console and +/// drives its show/hide/dismiss lifecycle from SwiftUI. +/// +/// Held as `@State` in `ServerDetailView`, so it is recreated whenever that +/// view's own `@State` is -- i.e. whenever `RootView`'s `.id(server.id)` +/// selects a different server (see `RootView`'s doc comment on that +/// modifier). `ServerDetailView` calls `dismiss()` from `.onDisappear` so a +/// panel for a previously-selected server can never outlive it. +/// +/// **Why this is safe against the single-consumer stdout stream +/// constraint**: this controller never calls `ServerLogViewModel +/// .streamLogs()` itself, and the `ServerLogView` inside the panel it +/// creates has no `.task` of its own (see that view's doc comment). Only +/// `ServerDetailView`'s own `.task(id: server.status)` calls `streamLogs()`, +/// exactly once, regardless of whether this panel is open, closed, or was +/// never shown at all. This controller and the inline "Console Output" +/// section both only *read* the same `ServerLogViewModel.lines`, which is +/// safe from any number of SwiftUI views -- unlike calling `streamLogs()` +/// (and transitively `ServerProcessService.stdoutLines(serverId:)`) more +/// than once for the same running server, which is documented there as +/// unsafe. +@MainActor +@Observable +final class FloatingConsolePanelController: NSObject { + private(set) var isVisible = false + + private var panel: FloatingConsolePanel? + private let serverName: String + private let viewModel: ServerLogViewModel + + init(serverName: String, viewModel: ServerLogViewModel) { + self.serverName = serverName + self.viewModel = viewModel + } + + /// Shows the panel, creating it on first call. `orderFront(nil)` (not + /// `makeKeyAndOrderFront`) matches this panel's `.nonactivatingPanel` + /// design: it should float into view without taking key window status + /// or activating the app, per the confirmed spike design. + func show() { + let panel = self.panel ?? self.makePanel() + self.panel = panel + panel.orderFront(nil) + self.isVisible = true + } + + /// Hides the panel without discarding it -- a subsequent `show()` + /// reuses the same `NSPanel`/`NSHostingView`/`ServerLogView`, cheaper + /// than recreating the panel on every toggle and preserving its + /// on-screen frame across hides. + func hide() { + self.panel?.orderOut(nil) + self.isVisible = false + } + + func toggle() { + if self.isVisible { + self.hide() + } else { + self.show() + } + } + + /// Fully closes and releases the panel. Called from + /// `ServerDetailView.onDisappear` so a panel never outlives the server + /// it was showing -- `hide()` alone would leave it orphaned on screen, + /// still retaining this now-stale `viewModel`, once `ServerDetailView` + /// itself is torn down (e.g. a different server is selected). + func dismiss() { + self.panel?.close() + self.panel = nil + self.isVisible = false + } + + private func makePanel() -> FloatingConsolePanel { + let content = FloatingConsoleContentView(serverName: self.serverName, viewModel: self.viewModel) + let panel = FloatingConsolePanel(title: "Console — \(self.serverName)", content: content) + panel.delegate = self + return panel + } +} + +extension FloatingConsolePanelController: NSWindowDelegate { + /// Handles the panel's native close button (present because + /// `.closable` is in its `styleMask`), which calls `NSWindow.close()` + /// directly and bypasses `dismiss()`. Without this, `isVisible` would + /// stay stuck at `true` after the user closes the panel by hand, and a + /// later `show()` would call `orderFront(nil)` on an already-closed + /// window, which does not reliably reopen it -- so this resets to the + /// same "not shown" state `dismiss()` produces, ready for `show()` to + /// lazily recreate the panel next time. + func windowWillClose(_: Notification) { + self.panel = nil + self.isVisible = false + } +} diff --git a/apps/native-macos/Sources/Core/MCVectorApp.swift b/apps/native-macos/Sources/Core/MCVectorApp.swift new file mode 100644 index 0000000..9de86d1 --- /dev/null +++ b/apps/native-macos/Sources/Core/MCVectorApp.swift @@ -0,0 +1,19 @@ +import SwiftUI + +/// The Native app's real, persistent window. +/// +/// `Main.swift` invokes `MCVectorApp.main()` for normal (non-spike) +/// launches -- mirroring how spike windows are launched via their own +/// `App`-conforming types (see `PanelSpikeRunner.runSwiftUIWindowLevel()`). +/// This is the first task where the app becomes runnable as a real windowed +/// app, so it's kept intentionally minimal: no menu bar customization, no +/// app delegate logic, just `RootView` in a `WindowGroup`. +public struct MCVectorApp: App { + public init() {} + + public var body: some Scene { + WindowGroup { + RootView() + } + } +} diff --git a/apps/native-macos/Sources/Core/RootView.swift b/apps/native-macos/Sources/Core/RootView.swift index d74db5e..7b882b0 100644 --- a/apps/native-macos/Sources/Core/RootView.swift +++ b/apps/native-macos/Sources/Core/RootView.swift @@ -1,15 +1,165 @@ import SwiftUI +/// Top-level app shell: a `NavigationSplitView` with the server list as the +/// sidebar and the selected server's detail as the detail pane, falling +/// back to a "select a server" placeholder when nothing is selected or the +/// selected id no longer matches any loaded server. public struct RootView: View { - public init() {} + @State private var viewModel: ServerListViewModel + /// Toggles the Activity Drawer (task 3-10). Owned here, not by + /// `ServerListViewModel` -- it's pure view-presentation state with no + /// bearing on the view model's data, matching how `ServerDetailView` + /// keeps its own console-panel visibility state locally rather than + /// hoisting it. + @State private var isActivityDrawerPresented = false + + public init() { + self.init(viewModel: ServerListViewModel()) + } + + /// Test/preview-only injection point: lets a caller supply a + /// `ServerListViewModel` backed by a temp-file `ServerStore` instead of + /// the real Application Support location. Not `public` -- production + /// callers always use `init()`. + init(viewModel: ServerListViewModel) { + self._viewModel = State(initialValue: viewModel) + } public var body: some View { - Text("MC-Vector Native") - .font(.title) - .padding() + NavigationSplitView { + ServerListView(viewModel: self.viewModel) + } detail: { + if let server = self.viewModel.selectedServer { + // `.id(server.id)` forces a fresh `ServerDetailView` + // instance (and therefore a fresh `ServerLogViewModel`, per + // that view's `@State` init) whenever the sidebar selection + // changes to a different server -- without it, SwiftUI + // would reuse the existing `@State` across selections and + // the log view would keep streaming the *previous* + // server's stdout. + ServerDetailView(server: server, processService: self.viewModel.processService) + .id(server.id) + .toolbar { + ToolbarItem(placement: .primaryAction) { + Button("Start", systemImage: "play.fill") { + Task { await self.viewModel.startSelectedServer() } + } + .disabled(!Self.canStart(server)) + } + ToolbarItem(placement: .primaryAction) { + Button("Stop", systemImage: "stop.fill") { + Task { await self.viewModel.stopSelectedServer() } + } + .disabled(!Self.canStop(server)) + } + } + } else { + ContentUnavailableView( + "Select a Server", + systemImage: "server.rack", + description: Text("Choose a server from the sidebar to see its details."), + ) + } + } + // Attached to the outer `NavigationSplitView` (not inside the + // `detail` closure) so the Activity Drawer is a global, all-servers + // panel available regardless of sidebar selection -- unlike + // Start/Stop (attached inside `detail`, since those act on + // `viewModel.selectedServer` and are meaningless with nothing + // selected). SwiftUI merges this `.toolbar` with `ServerDetailView`'s + // own `.toolbar` (the 3-9 console toggle) into one unified toolbar + // when a server is selected, the same way `ServerDetailView`'s + // toolbar already merges with the Start/Stop toolbar above. + .inspector(isPresented: self.$isActivityDrawerPresented) { + ActivityDrawerView(entries: self.viewModel.activityLog) + .inspectorColumnWidth(min: 220, ideal: 280, max: 380) + } + .toolbar { + ToolbarItem(placement: .automatic) { + Button( + self.isActivityDrawerPresented ? "Hide Activity" : "Show Activity", + systemImage: "clock.arrow.circlepath", + ) { + self.isActivityDrawerPresented.toggle() + } + } + } + // Surfaces `ServerListViewModel.error` (task 3-12 code-review fix): + // previously `load()`/`startSelectedServer()`/`stopSelectedServer()` + // all set that property on failure, but no View ever read it, so a + // failed start/stop (e.g. a missing Java path) had zero user-visible + // signal beyond the status silently reverting. + // + // Uses `.alert(_:isPresented:presenting:actions:message:)` -- the + // current, non-deprecated alert API -- rather than the older + // `alert(item:content:) -> Alert` overload: that one predates this + // API and is itself soft-deprecated (it returns the also-deprecated + // `Alert` type; see `references/soft-deprecation.md` in + // `swiftui-expert-skill`, which calls out `Alert`/`ActionSheet` by + // name). `presenting:` still takes `viewModel.error` as a snapshot, + // so `message` reads a value captured at presentation time rather + // than re-reading a since-possibly-cleared view model property. + // + // `isPresented` still needs an explicit `Binding` -- that's an + // inherent part of this API's shape, not an avoidable synthesis -- + // but unlike the anti-pattern the review flagged, it only decides + // *whether* to show the alert; the message content itself never + // flows through it, so there is no risk of the boolean and the + // string momentarily disagreeing about what happened. + .alert( + "Something Went Wrong", + isPresented: Binding( + get: { self.viewModel.error != nil }, + set: { isPresented in + if !isPresented { + self.viewModel.clearError() + } + }, + ), + presenting: self.viewModel.error, + ) { _ in + Button("OK", role: .cancel) {} + } message: { error in + Text(error.message) + } + } + + /// Start is only meaningful from a fully-stopped state. `.stopping` and + /// `.restarting` are deliberately excluded too (not just the task's + /// explicitly called-out `.online`/`.starting`) -- starting a server + /// that's already mid-transition would race the in-flight operation. + private static func canStart(_ server: Server) -> Bool { + switch server.status { + case .offline, .crashed: + true + case .online, .starting, .stopping, .restarting: + false + } + } + + /// Stop is meaningful whenever a process might plausibly be running or + /// coming up. Matches the task's explicit disabled set + /// (`.offline`/`.crashed`/`.stopping`) exactly. + private static func canStop(_ server: Server) -> Bool { + switch server.status { + case .online, .starting, .restarting: + true + case .offline, .crashed, .stopping: + false + } } } #Preview { - RootView() + // Avoid `RootView()`'s production default here -- it would touch the + // real Application Support directory on whatever machine renders this + // preview. Point the view model's store at a scratch temp file instead. + RootView( + viewModel: ServerListViewModel( + store: ServerStore( + fileURL: FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-preview-servers-\(UUID().uuidString).json"), + ), + ), + ) } diff --git a/apps/native-macos/Sources/Core/Security/AuditEntry.swift b/apps/native-macos/Sources/Core/Security/AuditEntry.swift new file mode 100644 index 0000000..9d280e6 --- /dev/null +++ b/apps/native-macos/Sources/Core/Security/AuditEntry.swift @@ -0,0 +1,50 @@ +import Foundation +import os + +/// A single audit-log record, ported from `security.rs`'s +/// `build_audit_entry` JSON shape (`{"user", "action", "timestamp"}`). +/// `timestamp` is whole seconds since the Unix epoch, matching Rust's +/// `SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs()`. +public struct AuditEntry: Sendable, Equatable { + public let user: String + public let action: String + public let timestamp: UInt64 +} + +/// Builds (and logs) an audit entry for `user` performing `action`. +/// +/// `now` defaults to the real wall clock but is an explicit parameter -- +/// unlike `RateLimiter`'s elapsed-time comparisons, this doesn't need +/// `ContinuousClock` (which has no fixed epoch); it needs a `Date` to +/// convert to seconds-since-epoch, so tests can assert on a specific +/// `timestamp` without depending on wall-clock timing. +public func buildAuditEntry(user: String, action: String, now: Date = Date()) throws -> AuditEntry { + let normalizedUser = user.trimmingCharacters(in: .whitespacesAndNewlines) + let normalizedAction = action.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedUser.isEmpty, !normalizedAction.isEmpty else { + throw SecurityError.emptyAuditFields + } + + let secondsSinceEpoch = now.timeIntervalSince1970 + guard secondsSinceEpoch >= 0 else { + throw SecurityError.auditTimestampCreationFailed + } + let timestamp = UInt64(secondsSinceEpoch) + + AuditLogger.logEntry(user: normalizedUser, action: normalizedAction, timestamp: timestamp) + + return AuditEntry(user: normalizedUser, action: normalizedAction, timestamp: timestamp) +} + +/// Minimal `os.Logger` side effect mirroring Rust's +/// `log::info!(target: "security.audit", ...)` call. Kept to a single +/// log line with no additional behavior -- this isn't a logging +/// subsystem, just parity with the one line Rust emits. +private enum AuditLogger { + private static let logger = Logger(subsystem: "com.mc-vector.native", category: "security.audit") + + static func logEntry(user: String, action: String, timestamp: UInt64) { + let message = "[AUDIT] user=\(user) action=\(action) timestamp=\(timestamp)" + self.logger.info("\(message, privacy: .public)") + } +} diff --git a/apps/native-macos/Sources/Core/Security/Authorization.swift b/apps/native-macos/Sources/Core/Security/Authorization.swift new file mode 100644 index 0000000..5f423f8 --- /dev/null +++ b/apps/native-macos/Sources/Core/Security/Authorization.swift @@ -0,0 +1,54 @@ +import Foundation + +/// Role-based authorization, ported from `security.rs`'s `authorize` and +/// `is_mutating_action`. Deliberately a free function, not a type -- the +/// Rust original is pure and stateless (no lock, no actor, no I/O), so a +/// Swift `actor` here would add isolation overhead with nothing to +/// protect. Compare with `RateLimiter`, which genuinely needs an actor +/// because it owns mutable shared state. +/// +/// Not wired into any command or view model: this app has no +/// user/role/authentication concept yet. A future task connects this once +/// "current role" exists somewhere real. +public func authorize(role: Role, action: String) throws { + let normalizedAction = action.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedAction.isEmpty else { + throw SecurityError.emptyAction + } + + switch role { + case .admin: + return + case .user: + guard normalizedAction == "start_server" || normalizedAction == "stop_server" else { + throw SecurityError.forbidden(role: role.rawName, action: normalizedAction) + } + case .viewer: + guard !isMutatingAction(normalizedAction) else { + throw SecurityError.forbidden(role: role.rawName, action: normalizedAction) + } + } +} + +/// Whether `action` is treated as mutating (state-changing) rather than +/// read-only, matching Rust's `is_mutating_action`. An empty action is +/// conservatively treated as mutating -- `authorize` never reaches this +/// with an empty action itself (it rejects that earlier), but the +/// function is ported standalone for fidelity with the Rust source, which +/// exposes the same conservative default. +public func isMutatingAction(_ action: String) -> Bool { + let normalizedAction = action.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedAction.isEmpty else { + return true + } + + let isReadOnly = normalizedAction.hasPrefix("get_") + || normalizedAction.hasPrefix("list_") + || normalizedAction.hasPrefix("read_") + || normalizedAction.hasPrefix("fetch_") + || normalizedAction.hasPrefix("sanitize_") + || normalizedAction == "authorize_action" + || normalizedAction == "rate_limit_check" + + return !isReadOnly +} diff --git a/apps/native-macos/Sources/Core/Security/RateLimiter.swift b/apps/native-macos/Sources/Core/Security/RateLimiter.swift new file mode 100644 index 0000000..0de4668 --- /dev/null +++ b/apps/native-macos/Sources/Core/Security/RateLimiter.swift @@ -0,0 +1,84 @@ +import Foundation + +/// Per-user call throttling, ported from `security.rs`'s +/// `RATE_LIMITER`/`check_rate_limit`/`check_rate_limit_with_state`. +/// +/// Rust protects its `HashMap` with a `std::sync::Mutex` +/// behind a `OnceLock`. The `swift-concurrency` skill's guidance for +/// "shared mutable state" is to prefer an `actor` over locks/queues and +/// keep isolated sections small -- that's exactly the shape here: this +/// actor's single method body *is* the entire critical section (prune, +/// saturation check, per-user check, insert), with no `await` in the +/// middle, so there's no reentrancy window where another call could +/// observe a half-updated map. That gives the same atomicity Rust's +/// `Mutex::lock()` gives around `check_rate_limit_with_state`, without a +/// manual lock. +/// +/// Rust splits a real entry point (`check_rate_limit`, which reads +/// `Instant::now()`) from a testable core (`check_rate_limit_with_state`, +/// which takes `now` as a parameter) so its unit tests can assert on +/// specific instants instead of racing the real clock. This actor mirrors +/// that split with a single method that takes `now` as a parameter +/// defaulting to the real clock: real callers get `RateLimiter.checkRateLimit(userId:)` +/// for free, and tests pass explicit `ContinuousClock.Instant` values +/// (typically offset from a shared `start` via `+ .milliseconds(...)`) to +/// deterministically land on either side of the rate-limit window. This +/// avoids `Task.sleep`-based timing tests, which is what every one of the +/// Rust rate-limit tests does with injected `Instant`s. +public actor RateLimiter { + /// Matches Rust's `RATE_LIMIT_WINDOW`: calls from the same user inside + /// this window are rejected. + public static let rateLimitWindow: Duration = .seconds(1) + /// Matches Rust's `RATE_LIMIT_MAX_ENTRIES`: once the state map holds + /// this many distinct (non-expired) users, calls from *new* users are + /// rejected until entries expire and get pruned. + public static let rateLimitMaxEntries = 4096 + + private var lastCallByUserId: [String: ContinuousClock.Instant] = [:] + + public init() {} + + /// Checks (and, on success, records) a call for `userId`. + /// + /// Mirrors Rust's `check_rate_limit_with_state` step for step: + /// 1. Reject empty/whitespace-only user ids. + /// 2. Prune every entry whose last call falls outside the window -- + /// this runs on every call, exactly like Rust, so the map never + /// grows unbounded from one-off callers. + /// 3. If this is a *new* user and the (now-pruned) map is already at + /// capacity, reject as saturated. + /// 4. If this user already has a call inside the window, reject as + /// rate-limited. + /// 5. Otherwise record `now` for this user and succeed. + public func checkRateLimit( + userId: String, + now: ContinuousClock.Instant = ContinuousClock.now, + ) throws { + let normalizedUserId = userId.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedUserId.isEmpty else { + throw SecurityError.emptyUserId + } + + self.lastCallByUserId = self.lastCallByUserId.filter { _, lastCall in + now - lastCall < Self.rateLimitWindow + } + + let isNewUser = self.lastCallByUserId[normalizedUserId] == nil + if isNewUser, self.lastCallByUserId.count >= Self.rateLimitMaxEntries { + throw SecurityError.rateLimiterSaturated + } + + if let lastCall = self.lastCallByUserId[normalizedUserId], now - lastCall < Self.rateLimitWindow { + throw SecurityError.rateLimitExceeded(userId: normalizedUserId) + } + + self.lastCallByUserId[normalizedUserId] = now + } + + /// Test-only introspection of which user ids are currently tracked, + /// used to assert pruning removed stale entries without exposing the + /// backing dictionary (or its `Instant` values) as public API. + func trackedUserIds() -> Set { + Set(self.lastCallByUserId.keys) + } +} diff --git a/apps/native-macos/Sources/Core/Security/Role.swift b/apps/native-macos/Sources/Core/Security/Role.swift new file mode 100644 index 0000000..a470e0a --- /dev/null +++ b/apps/native-macos/Sources/Core/Security/Role.swift @@ -0,0 +1,37 @@ +import Foundation + +/// The three authorization roles from `security.rs`'s `Role` enum. +/// +/// This app has no login screen, session, or "current user" concept yet +/// (see the Phase 3-B task note for this file's introduction) -- `Role` is +/// deliberately not wired into any view model or command today. It exists +/// so a future task can gate real actions once the app knows who's asking. +public enum Role: Sendable, Equatable { + case admin + case user + case viewer + + /// The wire/log-facing name for this role, matching Rust's + /// `Role::as_str`. Used inside `SecurityError.forbidden` messages. + var rawName: String { + switch self { + case .admin: "admin" + case .user: "user" + case .viewer: "viewer" + } + } + + /// Parses a free-form role string the same way Rust's `Role::parse` + /// does: trim whitespace, lowercase, then exact-match against the + /// three known role names. Deliberately not `init?(rawValue:)` -- + /// that would require the caller to already have normalized input, + /// whereas Rust's `authorize_action` payload is untrusted raw text. + public static func parse(_ value: String) throws -> Role { + switch value.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() { + case "admin": .admin + case "user": .user + case "viewer": .viewer + default: throw SecurityError.invalidRole(value) + } + } +} diff --git a/apps/native-macos/Sources/Core/Security/SafePathResolver.swift b/apps/native-macos/Sources/Core/Security/SafePathResolver.swift new file mode 100644 index 0000000..97701cd --- /dev/null +++ b/apps/native-macos/Sources/Core/Security/SafePathResolver.swift @@ -0,0 +1,72 @@ +import Foundation + +/// Joins an untrusted relative path onto a trusted base directory while +/// rejecting any attempt to escape it, ported from `security.rs`'s +/// `resolve_safe_path`. +/// +/// Deliberately does **not** use `URL` or `NSString.standardizingPath`: +/// both *silently normalize away* `..` components (turning +/// `base/../etc/passwd` into `/etc/passwd` without complaint) rather than +/// rejecting the input, which is the opposite of what a path-safety gate +/// needs. This walks `input`'s `/`-separated components by hand, exactly +/// like Rust's `Path::components()` + `Component::ParentDir`/`CurDir` +/// check, so `..`/`.` anywhere in the input is caught and rejected +/// instead of resolved. +/// +/// The Windows drive-letter-prefix check (`C:...`) is ported unconditionally, +/// even though this app only ships on macOS -- it's defensive logic in the +/// Rust original that doesn't depend on the host OS (a drive-relative path +/// like `C:windows\temp` has no leading `/` and no `..` component, so +/// without this explicit check it would otherwise slip past every other +/// guard here). +public func resolveSafePath(base: String, input: String) throws -> String { + let normalizedBase = base.trimmingCharacters(in: .whitespacesAndNewlines) + let normalizedInput = input.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedBase.isEmpty, !normalizedInput.isEmpty else { + throw SecurityError.emptyPathInputs + } + + guard normalizedBase.hasPrefix("/") else { + throw SecurityError.baseMustBeAbsolute + } + + if hasWindowsDriveLetterPrefix(normalizedInput) { + throw SecurityError.pathTraversalDetected + } + + guard !normalizedInput.hasPrefix("/") else { + throw SecurityError.pathTraversalDetected + } + + // Deliberately stricter than the Rust original here: `Path::components()` + // only ever produces a `Component::CurDir` for a *leading* `.` segment, + // since its parser silently collapses interior `.` segments away during + // normalization (e.g. `a/./b` never yields a `CurDir` component at all). + // This check instead rejects `.` in ANY position, not just leading. That + // divergence is intentional, not a transcription bug: an interior `.` + // segment has no legitimate meaning in a safe-path input, and rejecting + // it fails safe (over-strict) rather than silently accepting something + // Rust would also silently accept -- it's a strictly *safer* superset of + // the original behavior, never a looser one. + let components = normalizedInput.split(separator: "/", omittingEmptySubsequences: false) + guard !components.contains(where: { $0 == "." || $0 == ".." }) else { + throw SecurityError.pathTraversalDetected + } + + let needsSeparator = !normalizedBase.hasSuffix("/") + return normalizedBase + (needsSeparator ? "/" : "") + normalizedInput +} + +/// Mirrors Rust's raw byte check: the first two bytes of `input` are an +/// ASCII letter followed by `:` (e.g. `C:windows\temp`). Checked on the +/// trimmed input's UTF-8 bytes, matching Rust's `bytes[0]`/`bytes[1]` +/// indexing on `normalized_input.as_bytes()`. +private func hasWindowsDriveLetterPrefix(_ input: String) -> Bool { + let bytes = Array(input.utf8) + guard bytes.count >= 2 else { + return false + } + let isASCIIAlphabetic = (UInt8(ascii: "a") ... UInt8(ascii: "z")).contains(bytes[0]) + || (UInt8(ascii: "A") ... UInt8(ascii: "Z")).contains(bytes[0]) + return isASCIIAlphabetic && bytes[1] == UInt8(ascii: ":") +} diff --git a/apps/native-macos/Sources/Core/Security/SecurityError.swift b/apps/native-macos/Sources/Core/Security/SecurityError.swift new file mode 100644 index 0000000..8ccd161 --- /dev/null +++ b/apps/native-macos/Sources/Core/Security/SecurityError.swift @@ -0,0 +1,72 @@ +import Foundation + +/// Every failure mode of the `security.rs`-equivalent authorization/audit +/// layer, ported one case per Rust `Err(String)` value. +/// +/// The Rust original returns a bare `String` for every error; this app's +/// standing "share nothing, port everything" rule means the *logic* is +/// re-implemented, not the Rust type. To keep the "仕様の一致率" (spec-match +/// fidelity) this port is reviewed against, `errorDescription` reproduces +/// the exact Rust wording for each case rather than a Swift-idiomatic +/// rephrasing -- callers (and this module's tests) can compare against the +/// Rust source string-for-string. +public enum SecurityError: Error, LocalizedError, Equatable, Sendable { + /// `authorize`: `payload.action` was empty/whitespace-only. + case emptyAction + /// `Role::parse`: the raw string didn't match `admin`/`user`/`viewer` + /// after trim + lowercase. + case invalidRole(String) + /// `authorize`: the role/action combination isn't permitted. + case forbidden(role: String, action: String) + /// `check_rate_limit`: `payload.userId` was empty/whitespace-only. + case emptyUserId + /// `check_rate_limit`: the state map is at `RATE_LIMIT_MAX_ENTRIES` + /// and the caller isn't already a tracked user. + case rateLimiterSaturated + /// `check_rate_limit`: this user already has a call inside the + /// current rate-limit window. + case rateLimitExceeded(userId: String) + /// `resolve_safe_path`: `payload.base` and/or `payload.input` was + /// empty/whitespace-only. + case emptyPathInputs + /// `resolve_safe_path`: `payload.base` was not an absolute path. + case baseMustBeAbsolute + /// `resolve_safe_path`: `payload.input` attempted to escape `base` + /// (absolute input, Windows drive-letter prefix, or a `.`/`..` + /// component). + case pathTraversalDetected + /// `build_audit_entry`: `payload.user` and/or `payload.action` was + /// empty/whitespace-only. + case emptyAuditFields + /// `build_audit_entry`: the system clock reported a time before the + /// Unix epoch. Practically unreachable, ported only for parity with + /// Rust's `SystemTime::now().duration_since(UNIX_EPOCH)` failure path. + case auditTimestampCreationFailed + + public var errorDescription: String? { + switch self { + case .emptyAction: + "security_gateway authorize_action requires non-empty payload.action" + case .invalidRole: + "security_gateway authorize_action requires payload.role as \"admin\"|\"user\"|\"viewer\"" + case let .forbidden(role, action): + "Forbidden: role \(role) is not allowed to perform action \(action)" + case .emptyUserId: + "security_gateway rate_limit_check requires non-empty payload.userId" + case .rateLimiterSaturated: + "Forbidden: rate limit state is saturated" + case let .rateLimitExceeded(userId): + "Forbidden: rate limit exceeded for user \(userId)" + case .emptyPathInputs: + "security_gateway resolve_safe_path requires non-empty payload.base and payload.input" + case .baseMustBeAbsolute: + "security_gateway resolve_safe_path payload.base must be absolute" + case .pathTraversalDetected: + "Path traversal detected" + case .emptyAuditFields: + "security_gateway audit_log requires non-empty payload.user and payload.action" + case .auditTimestampCreationFailed: + "Failed to create audit timestamp" + } + } +} diff --git a/apps/native-macos/Sources/Core/ServerDetailView.swift b/apps/native-macos/Sources/Core/ServerDetailView.swift new file mode 100644 index 0000000..cea4d5c --- /dev/null +++ b/apps/native-macos/Sources/Core/ServerDetailView.swift @@ -0,0 +1,134 @@ +import SwiftUI + +/// Read-only detail pane for a single `Server`, shown in `RootView`'s +/// `NavigationSplitView` detail column once a sidebar row is selected. +/// +/// Takes a resolved `Server` value rather than the whole +/// `ServerListViewModel` -- this view has no dependency on selection state +/// or the underlying store, only the data it renders, which keeps it simple +/// and independently previewable/testable in isolation. +/// +/// Shows the identifying/operational fields that are always meaningful for +/// a hand-authored server entry today (name, status, version, software, +/// port, memory, path, Java path). Auto-restart, auto-backup, and +/// notification settings are left for a later settings-focused task -- this +/// app has no server-creation flow yet, so surfacing 19 more optional +/// fields here would mostly be clutter with no data ever populated. +/// +/// Below the configuration `Form`, a "Console Output" section (task 3-8) +/// streams the server's live stdout via `ServerLogView`/`ServerLogViewModel` +/// -- added additively alongside the existing fields and the 3-7 toolbar +/// (`RootView` still owns Start/Stop), not folded into the `Form` itself, +/// since log lines have nothing in common with the labeled-field rows above +/// them. +/// +/// This view now also owns the single `.task(id: server.status)` that +/// drives `logViewModel.streamLogs()` (task 3-9's "Floating Console Panel" +/// moved this up from `ServerLogView` itself). That's what makes it safe to +/// show the same log stream in two places at once -- inline here *and* in +/// `FloatingConsolePanel` -- without ever calling `streamLogs()` (and +/// transitively `ServerProcessService.stdoutLines(serverId:)`) more than +/// once for the same running server; see `ServerLogView`'s and +/// `FloatingConsolePanelController`'s doc comments for the full rationale. +struct ServerDetailView: View { + let server: Server + @State private var logViewModel: ServerLogViewModel + @State private var consolePanelController: FloatingConsolePanelController? + + /// `processService` is threaded in from `RootView`'s + /// `ServerListViewModel` (see that class's `processService` doc + /// comment) rather than this view creating its own, so the log view + /// reads from the same tracked process that Start/Stop act on. + init(server: Server, processService: ServerProcessService) { + self.server = server + self._logViewModel = State( + initialValue: ServerLogViewModel(serverId: server.id, processService: processService), + ) + } + + var body: some View { + VStack(spacing: 0) { + Form { + Section { + LabeledContent("Status") { + Text(self.server.status.rawValue.capitalized) + } + } + + Section("Configuration") { + LabeledContent("Version", value: self.server.version) + LabeledContent("Software", value: self.server.software) + LabeledContent("Port", value: String(self.server.port)) + LabeledContent("Memory", value: "\(self.server.memory) MB") + LabeledContent("Path", value: self.server.path) + if let javaPath = self.server.javaPath { + LabeledContent("Java Path", value: javaPath) + } + } + } + .formStyle(.grouped) + .fixedSize(horizontal: false, vertical: true) + + Divider() + + VStack(alignment: .leading, spacing: 0) { + Text("Console Output") + .font(.headline) + .padding([.horizontal, .top], 12) + .padding(.bottom, 4) + ServerLogView(viewModel: self.logViewModel) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + .navigationTitle(self.server.name) + .task(id: self.server.status) { + await self.logViewModel.streamLogs() + } + .toolbar { + ToolbarItem(placement: .automatic) { + Button( + self.consolePanelController?.isVisible == true ? "Hide Console Panel" : "Show Console Panel", + systemImage: "terminal", + ) { + self.consolePanel().toggle() + } + } + } + .onDisappear { + self.consolePanelController?.dismiss() + } + } + + /// Lazily creates `consolePanelController` on first use (first toolbar + /// tap), passing it the same `logViewModel` instance driving the inline + /// "Console Output" section above -- never a second + /// `ServerLogViewModel`. Subsequent calls return the already-created + /// controller so repeated toggling reuses the same panel. + private func consolePanel() -> FloatingConsolePanelController { + if let controller = self.consolePanelController { + return controller + } + let controller = FloatingConsolePanelController(serverName: self.server.name, viewModel: self.logViewModel) + self.consolePanelController = controller + return controller + } +} + +#Preview { + NavigationStack { + ServerDetailView( + server: Server( + id: "srv-1", + name: "Survival", + version: "1.21.1", + software: "paper", + port: 25565, + memory: 4096, + path: "/servers/srv-1", + status: .online, + javaPath: "/usr/bin/java", + ), + processService: ServerProcessService(), + ) + } +} diff --git a/apps/native-macos/Sources/Core/ServerListView.swift b/apps/native-macos/Sources/Core/ServerListView.swift new file mode 100644 index 0000000..3f09a66 --- /dev/null +++ b/apps/native-macos/Sources/Core/ServerListView.swift @@ -0,0 +1,38 @@ +import SwiftUI + +/// Sidebar content for `RootView`'s `NavigationSplitView`: a `List` of the +/// user's configured servers, driven by `ServerListViewModel`. +/// +/// Selection and list identity both come from `Server.id` (via +/// `Identifiable`), so rows keep stable identity across reloads. +struct ServerListView: View { + @Bindable var viewModel: ServerListViewModel + + var body: some View { + List(self.viewModel.servers, selection: self.$viewModel.selection) { server in + Text(server.name) + } + .navigationTitle("Servers") + .task { + await self.viewModel.load() + } + } +} + +#Preview { + // No servers.json exists at this path, so `.task { load() }` resolves + // to the deterministic "missing file -> empty list" path instantly -- + // no live service, no flakiness, nothing to hang on. + NavigationSplitView { + ServerListView( + viewModel: ServerListViewModel( + store: ServerStore( + fileURL: FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-preview-servers-\(UUID().uuidString).json"), + ), + ), + ) + } detail: { + Text("Detail") + } +} diff --git a/apps/native-macos/Sources/Core/ServerListViewModel.swift b/apps/native-macos/Sources/Core/ServerListViewModel.swift new file mode 100644 index 0000000..badafc9 --- /dev/null +++ b/apps/native-macos/Sources/Core/ServerListViewModel.swift @@ -0,0 +1,280 @@ +import Foundation +import Observation + +/// Drives the sidebar's server list. +/// +/// Loads `Server` records from a `ServerStore`-backed JSON file and exposes +/// them for `List`/`ForEach`, along with the sidebar's current selection. +/// +/// A first-run environment where `servers.json` doesn't exist on disk yet +/// is not an error from this view model's perspective: `load()` treats a +/// missing file as an empty list. Any other failure (corrupt JSON, +/// permission error, etc.) is surfaced via `error`, which `RootView` +/// presents through a real `.alert` (task 3-12 code-review fix -- see +/// `ServerListViewModelError`'s doc comment for why that's an `Identifiable` +/// wrapper rather than a plain `String?`). +@MainActor +@Observable +public final class ServerListViewModel { + private let store: ServerStore + /// Not `private` -- `RootView` needs the same `ServerProcessService` + /// instance to build a `ServerLogViewModel` for the detail screen's log + /// view (task 3-8), so log streaming reads from the same tracked + /// process this view model started/stopped. `ServerListViewModel` + /// itself has no log-related responsibility beyond exposing this; + /// owning log-streaming state is `ServerLogViewModel`'s job, not this + /// class's. + public let processService: ServerProcessService + /// Housekeeping handle for the background event-subscription `Task`, not + /// UI-relevant state -- no view reads `processEventTask`, so there's no + /// reason for it to participate in `@Observable`'s change tracking. + /// `@ObservationIgnored` opts it out of `@ObservationTracked`'s macro + /// expansion, which also sidesteps that macro's separate restriction on + /// `nonisolated` mutable stored properties (irrelevant here anyway, + /// since this property stays `@MainActor`-isolated like the rest of the + /// class). + /// + /// Cancelling it from `deinit` is handled via `isolated deinit` (Swift + /// 6.2, SE-0371) rather than making the property itself `nonisolated`: + /// `deinit` is normally nonisolated even on a `@MainActor` class, but + /// `isolated deinit` runs on the class's actor, so it can touch + /// actor-isolated state (like this property) directly and safely, with + /// no unsafe escape hatch and no cross-isolation gymnastics on the + /// property declaration. Available here because this package's + /// deployment target (`platforms: [.macOS(.v26)]` in `Package.swift`) + /// is far above the feature's macOS 15.4+ minimum. + @ObservationIgnored + private var processEventTask: Task? + + public private(set) var servers: [Server] = [] + public var selection: Server.ID? + /// The most recent failure from `load()`, `startSelectedServer()`, or + /// `stopSelectedServer()`, or `nil` if none is currently outstanding. + /// `RootView` presents this via `.alert(_:isPresented:presenting: + /// actions:message:)`, passing the snapshotted value through + /// `presenting:` -- see `ServerListViewModelError`'s doc comment. + public private(set) var error: ServerListViewModelError? + + /// Global, cross-server activity log for the Activity Drawer (task + /// 3-10), newest-first (index 0 is the most recent entry). Session-only: + /// unlike `servers` (backed by `ServerStore`/`servers.json`), this array + /// is never persisted to disk and starts empty on every launch -- + /// activity history has a different, much simpler lifecycle than server + /// definitions, so it doesn't need a store of its own. + /// + /// Populated from two places: `apply(_:)` (events delivered over + /// `processService.events` -- a tracked process exiting cleanly + /// (`.offline`) or crashing (`.crashed`), see `ServerProcessEvent`'s doc + /// comment) and `startSelectedServer()`'s synchronous `.online` success + /// path. Both funnel through the shared `appendActivity(forServerId:status:)` + /// helper. + /// + /// `startSelectedServer()` logs its own `.online` entry directly rather + /// than going through `processService.events` -- code review on task + /// 3-10 (see git history) found that a successful start never produced + /// an `ActivityEntry` at all, which contradicted the task spec's literal + /// "起動/停止/バックアップ等のアクティビティ履歴を表示する" requirement. + /// The original justification for omitting it (avoiding a second + /// subscriber on `processService.events`) doesn't actually apply here: + /// `startSelectedServer()` never touches that stream, so calling + /// `appendActivity` directly from its success path adds no risk of a + /// second consumer on the single-consumer `AsyncStream`. + /// + /// `stopSelectedServer()` deliberately does NOT get a matching + /// "stop requested" entry at the point it issues the stop -- only the + /// later `.offline`/`.crashed` entry, logged via `apply(_:)` once the + /// process actually exits. A stop's outcome is inherently uncertain + /// until the process really terminates (see that method's doc comment: + /// the actor's termination monitor is the single source of truth), so + /// logging an entry the moment the request is merely issued would be a + /// weaker, and arguably misleading, signal than a start's -- unlike + /// `startSelectedServer()`, where `.online` is already known for certain + /// by the time this code runs. Double-logging "stop requested" + + /// "stop completed" would also just clutter the drawer with two entries + /// for one user action; the spec asks for stop activity to be visible, + /// and the existing single offline/crashed entry already satisfies that. + /// + /// Bounded to `activityLogCap` entries (oldest dropped first) so a long + /// session doesn't grow this array unboundedly -- same trim-on-overflow + /// principle as `LogLineBuffer`, simplified to a plain array trim since + /// process events arrive far less frequently than log lines. + public private(set) var activityLog: [ActivityEntry] = [] + + /// Maximum number of entries retained in `activityLog`. Not `static` -- + /// overridable per-instance (see `init`) so tests can exercise the + /// trim-on-overflow path with a small cap instead of needing hundreds of + /// real process launches. + private let activityLogCap: Int + + /// The currently selected `Server`, resolved from `selection` against + /// `servers`. `nil` when nothing is selected, and also `nil` when + /// `selection` no longer matches any loaded server (e.g. it was + /// deleted, or a stale id survived a reload) -- callers such as + /// `RootView`'s detail pane fall back to a placeholder in both cases + /// without needing to distinguish them. + public var selectedServer: Server? { + self.servers.first(where: { $0.id == self.selection }) + } + + /// Injectable initializer. Tests should use this with a `ServerStore` + /// pointed at a temp file (see `ServerStoreTests` for the pattern) so + /// they never touch the real Application Support directory, and + /// (optionally) a dedicated `ServerProcessService` so process-related + /// tests don't share state with other tests' server instances. + public init( + store: ServerStore, + processService: ServerProcessService = ServerProcessService(), + activityLogCap: Int = 200, + ) { + self.store = store + self.processService = processService + self.activityLogCap = activityLogCap + // @MainActor is this class's inherited isolation, but nothing in + // this task's synchronous prefix needs it -- fetching `events` is + // itself a cross-actor call, and the loop body only touches `self` + // after each `await`, at which point it's back on the main actor to + // update `servers`. Per the swift-concurrency skill: nothing before + // the first `await` needs `@MainActor`, so this task doesn't need + // to inherit it either; it just hops back via actor-isolated `self` + // access after each event. + self.processEventTask = Task { @concurrent [weak self, processService] in + for await event in processService.events { + await self?.apply(event) + } + } + } + + /// Production default: points the underlying `ServerStore` at the + /// app's real on-disk location under Application Support. + public convenience init() { + self.init(store: ServerStore(fileURL: ServerStore.defaultFileURL())) + } + + isolated deinit { + self.processEventTask?.cancel() + } + + /// Loads servers from disk, updating `servers`. Treats a missing file + /// (first run, nothing saved yet) as an empty list rather than an + /// error; any other failure is recorded in `error`. + public func load() async { + do { + let file = try await self.store.load() + self.servers = file.servers + self.error = nil + } catch let error as CocoaError where error.code == .fileReadNoSuchFile { + self.servers = [] + self.error = nil + } catch { + self.error = ServerListViewModelError(message: error.localizedDescription) + } + } + + /// Starts the currently selected server's Java process via + /// `ServerProcessService`. + /// + /// Sets `.starting` optimistically before the call so the toolbar + /// reflects the in-flight request immediately, then `.online` once + /// `start(server:)` returns successfully -- that's known synchronously + /// at that point, so it doesn't need to round-trip through + /// `processService.events`. On failure, reverts to the server's prior + /// status and surfaces the error via `error` (e.g. a missing Java path), + /// which `RootView` presents as a real alert. + public func startSelectedServer() async { + guard let server = self.selectedServer else { return } + + self.setStatus(.starting, forServerId: server.id) + self.error = nil + + do { + try await self.processService.start(server: server) + self.setStatus(.online, forServerId: server.id) + // Logged directly here, not via `processService.events` -- see + // `activityLog`'s doc comment for why this is safe (no stream + // contact) and why the task spec requires it (a successful + // start is an activity, same as a stop/crash). + self.appendActivity(forServerId: server.id, status: .online) + } catch { + self.setStatus(server.status, forServerId: server.id) + self.error = ServerListViewModelError(message: error.localizedDescription) + } + } + + /// Stops the currently selected server's Java process via + /// `ServerProcessService`. + /// + /// Sets `.stopping` optimistically before the call. The definitive + /// outcome (`.offline` on clean exit, `.crashed` otherwise) is left to + /// `processService.events` -- the actor's termination monitor is the + /// single source of truth for whether the process actually exited, so + /// this method doesn't guess at a final state itself. Only a failure to + /// even issue the stop (e.g. it wasn't running) reverts the optimistic + /// status here. + public func stopSelectedServer() async { + guard let server = self.selectedServer else { return } + + self.setStatus(.stopping, forServerId: server.id) + self.error = nil + + do { + try await self.processService.stop(serverId: server.id) + } catch { + self.setStatus(server.status, forServerId: server.id) + self.error = ServerListViewModelError(message: error.localizedDescription) + } + } + + /// Clears the currently displayed `error`, if any. `RootView`'s alert + /// calls this from its dismiss/OK action -- `error` is `private(set)`, + /// so the view can't `nil` it out directly, matching how `servers` and + /// `activityLog` are only ever mutated through this class's own methods. + public func clearError() { + self.error = nil + } + + /// The sole subscriber to `processService.events` -- handles both jobs + /// (status update and activity logging) for a single event delivery, + /// per this task's constraint against adding a second + /// `for await event in processService.events` loop: `processService.events` + /// is single-consumer, and this `apply(_:)` method is already the one + /// and only subscriber (see `init`'s `processEventTask`). Not the only + /// place activity entries are appended, though -- see + /// `appendActivity(forServerId:status:)`'s doc comment; the other caller + /// is `startSelectedServer()`'s synchronous success path, which never + /// touches this stream. + private func apply(_ event: ServerProcessEvent) { + self.setStatus(event.status, forServerId: event.serverId) + self.appendActivity(forServerId: event.serverId, status: event.status) + } + + /// Constructs and inserts a single `ActivityEntry`, shared by both + /// `apply(_:)` (event-driven: `.offline`/`.crashed`) and + /// `startSelectedServer()`'s success path (direct/synchronous: + /// `.online`) -- see `activityLog`'s doc comment for why the latter + /// bypasses the event stream entirely rather than routing through it. + /// + /// Resolves `serverId` against `servers` *before* appending, so the + /// stored `ActivityEntry.serverName` is a snapshot rather than a live + /// lookup -- see `ActivityEntry.serverName`'s doc comment. Falls back to + /// the raw id on a lookup miss (should not happen in practice, since + /// `setStatus` runs against the same `servers` array moments earlier in + /// both callers, but avoids ever losing an entry over a resolution + /// failure). + private func appendActivity(forServerId serverId: String, status: ServerStatus) { + let serverName = self.servers.first(where: { $0.id == serverId })?.name ?? serverId + let entry = ActivityEntry( + serverId: serverId, + serverName: serverName, + kind: .serverStatusChange(status), + ) + self.activityLog.insert(entry, at: 0) + if self.activityLog.count > self.activityLogCap { + self.activityLog.removeLast(self.activityLog.count - self.activityLogCap) + } + } + + private func setStatus(_ status: ServerStatus, forServerId serverId: String) { + guard let index = self.servers.firstIndex(where: { $0.id == serverId }) else { return } + self.servers[index].status = status + } +} diff --git a/apps/native-macos/Sources/Core/ServerListViewModelError.swift b/apps/native-macos/Sources/Core/ServerListViewModelError.swift new file mode 100644 index 0000000..050eda9 --- /dev/null +++ b/apps/native-macos/Sources/Core/ServerListViewModelError.swift @@ -0,0 +1,30 @@ +import Foundation + +/// Wraps a failure surfaced by `ServerListViewModel` (`load()`, +/// `startSelectedServer()`, `stopSelectedServer()`) so it can drive a real +/// SwiftUI alert. +/// +/// Task 3-12's swiftui-pro review flagged the original `errorMessage: +/// String?` for having no View that ever read it -- a failed start/stop had +/// zero user-visible signal beyond the status reverting. The straightforward +/// fix of presenting an alert off a raw `String?` invites a `Binding(get:set:)` +/// synthesized from "is this optional non-nil", which is exactly the +/// anti-pattern the review called out: it reconstructs a throwaway `Bool` +/// from the optional's presence on every access, rather than letting the +/// optional itself carry a stable, typed identity through the alert's +/// lifecycle. +/// +/// Wrapping the message in this `Identifiable` struct instead lets +/// `RootView` pass a snapshot of the failure into `.alert(_:isPresented: +/// presenting:actions:message:)`'s `presenting:` parameter -- the value used +/// to render the alert's message is captured once, at presentation time, +/// rather than re-read from the (possibly-already-cleared) view model on +/// every body evaluation. +public struct ServerListViewModelError: Identifiable, Equatable, Sendable { + public let id = UUID() + public let message: String + + public init(message: String) { + self.message = message + } +} diff --git a/apps/native-macos/Sources/Core/ServerLogView.swift b/apps/native-macos/Sources/Core/ServerLogView.swift new file mode 100644 index 0000000..06140bb --- /dev/null +++ b/apps/native-macos/Sources/Core/ServerLogView.swift @@ -0,0 +1,61 @@ +import SwiftUI + +/// Live console output for a running server, driven by `ServerLogViewModel`. +/// +/// Structural sibling of the 3-3 spike's `LogStreamScrollView` +/// (`ServerProcessService.stdoutLines`-fed lines rendered via +/// `ScrollView` + `LazyVStack` + `ForEach`) -- the spike measured this +/// combination as clearly faster than `List` at high line-arrival rates +/// (9 hitches/517ms vs. 78/2017ms; see `spec/phase3a-spike-results.md` +/// §3-3), so this view keeps that shape rather than switching to `List`. +/// +/// Auto-scrolls to the newest line as they arrive -- the spike view didn't +/// do this (it had no notion of "newest" worth chasing, being a synthetic +/// firehose), but it's expected behavior for any real log viewer. Scrolls +/// to a fixed sentinel row (`Self.bottomAnchorID`) rather than the last +/// `LogLine`'s own id: `LogLineBuffer`'s hysteresis trim removes a bulk +/// range from the *front* of `lines` once `retainedLineCount + +/// trimOvershoot` is exceeded, so "the last id" is still always present and +/// valid, but anchoring on a dedicated always-present sentinel avoids ever +/// depending on that being true. +/// +/// Purely a display component -- unlike its original (task 3-8) shape, it no +/// longer owns a `.task` that calls `viewModel.streamLogs()`. As of task 3-9 +/// (Floating Console Panel), this view is instantiated *twice* +/// simultaneously against the *same* `ServerLogViewModel`: once inline in +/// `ServerDetailView`'s "Console Output" section, once inside +/// `FloatingConsoleContentView` hosted in `FloatingConsolePanel`. If each +/// instance ran its own `streamLogs()` call, that would be two concurrent +/// calls into `ServerProcessService.stdoutLines(serverId:)` for the same +/// running server -- explicitly documented there as unsafe (the underlying +/// stream is single-consumer; two readers split stdout bytes +/// unpredictably). `ServerDetailView` now owns the single `.task(id: +/// server.status)` that drives `streamLogs()`; every `ServerLogView` merely +/// *reads* `viewModel.lines`, which is safe from any number of views. +struct ServerLogView: View { + let viewModel: ServerLogViewModel + + private static let bottomAnchorID = "server-log-bottom" + + var body: some View { + ScrollViewReader { proxy in + ScrollView { + LazyVStack(alignment: .leading, spacing: 1) { + ForEach(self.viewModel.lines) { line in + Text(line.text) + .font(.system(.caption, design: .monospaced)) + .frame(maxWidth: .infinity, alignment: .leading) + .textSelection(.enabled) + } + Color.clear + .frame(height: 1) + .id(Self.bottomAnchorID) + } + .padding(8) + } + .onChange(of: self.viewModel.lines.count) { + proxy.scrollTo(Self.bottomAnchorID, anchor: .bottom) + } + } + } +} diff --git a/apps/native-macos/Sources/Core/ServerLogViewModel.swift b/apps/native-macos/Sources/Core/ServerLogViewModel.swift new file mode 100644 index 0000000..1db1a08 --- /dev/null +++ b/apps/native-macos/Sources/Core/ServerLogViewModel.swift @@ -0,0 +1,151 @@ +import Foundation +import Observation + +/// Bridges a running server's live stdout (`ServerProcessService.stdoutLines(serverId:)`) +/// into the 3-3 spike's confirmed-fastest rendering pipeline: a +/// `LogLineBuffer` (hysteresis-trim, avoids per-line `Array.removeFirst` +/// cost) fed through a `LogBatcher` (avoids one `@Observable` state mutation +/// per incoming line, which is what made `List`'s ARC/diffing cost dominate +/// in the 3-3 trace). Neither `LogLineBuffer` nor `LogBatcher` is +/// reimplemented here -- both are reused exactly as validated by that spike; +/// see `spec/phase3a-spike-results.md` §3-3. +/// +/// `LogLine.id` is a monotonic `Int` assigned by this view model as lines +/// arrive, and `LogLine.timestamp` is a `ContinuousClock.Instant` taken at +/// arrival time. Neither needs adjustment from the spike's shape for real +/// stdout: `id` only needs to be stable per line for `Identifiable` (not +/// meaningful), and `timestamp` is only ever consumed internally by +/// `LogBatcher`'s interval-window grouping (relative elapsed time, never +/// rendered to the user as wall-clock time) -- `ContinuousClock.Instant` is +/// actually the *better* fit there than a wall-clock `Date`, since it can't +/// be skewed by a system clock adjustment mid-session. +@MainActor +@Observable +public final class ServerLogViewModel { + private var buffer: LogLineBuffer + private let batcher: LogBatcher + private let flushInterval: Duration + private let processService: ServerProcessService + private let serverId: String + + /// Lines pulled off the stdout stream since the last periodic flush. + /// Plain accumulation only -- not `@Observable`-tracked, since no view + /// reads it directly; only `flush()`'s contribution to `buffer` (via + /// `lines`) is meant to drive view updates. + @ObservationIgnored + private var pendingLines: [LogLine] = [] + @ObservationIgnored + private var nextLineId = 0 + @ObservationIgnored + private let clock = ContinuousClock() + + /// The lines currently retained for display, per `LogLineBuffer`'s + /// hysteresis-trim policy. Reading this from a view registers an + /// `@Observable` dependency on the underlying `buffer` storage. + public var lines: [LogLine] { + self.buffer.lines + } + + /// - Parameters: + /// - flushInterval: How often accumulated stdout lines are applied to + /// `buffer` as a single batch of `@Observable` mutations, rather + /// than one mutation per line. `50ms` keeps the UI feeling live + /// (well under human perception of "instant") while still + /// collapsing bursts -- matching the interval the 3-3 spike's + /// `LogBatcher` was designed to group by. + public init( + serverId: String, + processService: ServerProcessService, + retainedLineCount: Int = 5000, + trimOvershoot: Int = 500, + flushInterval: Duration = .milliseconds(50), + ) { + self.serverId = serverId + self.processService = processService + self.buffer = LogLineBuffer(retainedLineCount: retainedLineCount, trimOvershoot: trimOvershoot) + self.batcher = LogBatcher(interval: flushInterval) + self.flushInterval = flushInterval + } + + /// Streams `serverId`'s live stdout into `lines` until either the + /// stream ends (the process exited -- its write end of the pipe closes, + /// so `ServerProcessService`'s reader hits EOF and finishes the stream + /// naturally) or the calling `Task` is cancelled. + /// + /// Intended to be awaited directly from a SwiftUI `.task`/`.task(id:)` + /// modifier so cancellation is structured at the call site -- when that + /// task is cancelled (view disappears, `.task(id:)`'s id changes), + /// cancellation propagates into the `for await` loop below (per the + /// async-sequences skill: streams cancel when the enclosing task does) + /// and this method returns, with no separately-stored `Task` for a + /// caller to remember to cancel. + /// + /// Internally, a second `Task` (`flushTicker`) runs alongside the read + /// loop purely to guarantee periodic flushing even during a quiet + /// stretch with no new lines arriving -- without it, a burst of lines + /// followed by silence (the common shape of Minecraft's own startup + /// log, then nothing until a player acts) would leave the last burst + /// sitting unflushed in `pendingLines` indefinitely, since flushing + /// otherwise only happens as a side effect of a line arriving. Its + /// lifetime is scoped to this function via `defer`, not stored as + /// instance state, so it can never outlive a single `streamLogs()` + /// call. (An earlier version used `withTaskGroup` with two `@MainActor` + /// child tasks for this instead; that hit a Swift 6.2 region-based + /// isolation checker limitation -- "pattern that the region based + /// isolation checker does not understand how to check" -- so this + /// simpler `defer`-scoped `Task` is used instead, per the + /// swift-concurrency skill's smallest-safe-fix guidance.) + /// + /// If `serverId` has no tracked running process (`stdoutLines` returns + /// `nil` -- not started yet, already stopped/crashed), this returns + /// immediately without touching `buffer`. Callers key their `.task(id:)` + /// on the server's status (e.g. `.online`) so this re-runs and picks up + /// the live stream once the process actually starts. + public func streamLogs() async { + guard let stream = await self.processService.stdoutLines(serverId: self.serverId) else { return } + + // Inherits this method's @MainActor isolation (no `@concurrent`): + // its entire body -- the sleep aside -- needs `self.flush()` to run + // on the main actor, and there's no off-actor work worth hopping + // away for. + let flushTicker = Task { [weak self] in + guard let self else { return } + while !Task.isCancelled { + try? await Task.sleep(for: self.flushInterval) + self.flush() + } + } + defer { flushTicker.cancel() } + + for await text in stream { + self.nextLineId += 1 + self.pendingLines.append(LogLine(id: self.nextLineId, timestamp: self.clock.now, text: text)) + } + + // Final flush: the loop above ended (stream EOF or cancellation) + // with lines possibly still sitting in `pendingLines` from less + // than `flushInterval` ago. + self.flush() + } + + /// Applies everything accumulated in `pendingLines` to `buffer` as a + /// single synchronous burst of `LogLineBuffer.append` calls -- no + /// `await` in between, so SwiftUI observes one state change per flush + /// rather than one per line. `LogBatcher.batch` re-groups the flushed + /// slice by its own interval windows first: if a flush was ever delayed + /// past multiple windows' worth of backlog, this keeps that backlog's + /// internal structure rather than flattening it into one giant + /// undifferentiated group -- consistent with reusing `LogBatcher` + /// exactly as the 3-3 spike validated it, not reinventing its grouping. + private func flush() { + guard !self.pendingLines.isEmpty else { return } + let toFlush = self.pendingLines + self.pendingLines.removeAll(keepingCapacity: true) + + for group in self.batcher.batch(toFlush) { + for line in group { + self.buffer.append(line) + } + } + } +} diff --git a/apps/native-macos/Sources/Core/Services/ServerProcessService.swift b/apps/native-macos/Sources/Core/Services/ServerProcessService.swift new file mode 100644 index 0000000..eaa92a5 --- /dev/null +++ b/apps/native-macos/Sources/Core/Services/ServerProcessService.swift @@ -0,0 +1,400 @@ +import Foundation + +/// Errors thrown by `ServerProcessService`. +public enum ServerProcessError: Error, Sendable, Equatable { + /// `Server.javaPath` was `nil` when `start(server:)` was called. + /// + /// This app has no Java auto-detection/configuration flow yet -- that's + /// a planned but not-yet-built feature. Silently guessing a system path + /// (e.g. `/usr/bin/java`) would be surprising and wrong, so this is + /// surfaced as a typed error for the caller (the view model/UI) to show + /// the user instead. + case javaPathNotConfigured(serverId: String) + + /// `start(server:)` was called for a server this instance already has a + /// tracked running process for. Not part of the original design + /// surface, but starting twice would silently overwrite the tracked + /// `Process` reference and leak the first (now-untracked) child. + case alreadyRunning(serverId: String) + + /// `stop(serverId:)` was called for a server with no tracked running + /// process (already stopped, crashed, or never started via this + /// service instance). + case serverNotRunning(serverId: String) +} + +extension ServerProcessError: LocalizedError { + public var errorDescription: String? { + switch self { + case let .javaPathNotConfigured(serverId): + "No Java path is configured for server \(serverId). Set a Java path before starting it." + case let .alreadyRunning(serverId): + "Server \(serverId) is already running." + case let .serverNotRunning(serverId): + "Server \(serverId) is not running." + } + } +} + +/// A status-change notification emitted by `ServerProcessService` for +/// transitions it alone can observe asynchronously -- namely a tracked +/// process exiting on its own, at a time the caller isn't actively waiting. +/// +/// Transitions the caller learns synchronously (a successful `start(server:)` +/// return, or the caller's own optimistic "starting"/"stopping" UI state) +/// don't need to round-trip through this stream -- only the two outcomes +/// this actor uniquely knows about when they happen (clean exit vs. crash) +/// are published here. +public struct ServerProcessEvent: Sendable, Equatable { + public let serverId: String + public let status: ServerStatus + + public init(serverId: String, status: ServerStatus) { + self.serverId = serverId + self.status = status + } +} + +/// Actor-isolated owner of running Minecraft server child processes. +/// +/// Launches and stops `Process` instances, tracking them by `Server.id`. +/// `Process` is not `Sendable`, but per the swift-concurrency skill's +/// guidance for shared mutable state ("move it behind an actor"), that's +/// fine here: every `Process` reference lives only in this actor's private +/// `runningProcesses` dictionary, all access to it is already serialized by +/// actor isolation, and no `Process` reference ever escapes across the +/// actor boundary -- only `Sendable` value types (`ServerProcessEvent`, +/// thrown `ServerProcessError`s) cross into caller code. +/// +/// Launch args and the "write stop\n to stdin, no SIGTERM/SIGKILL as the +/// primary mechanism" shutdown approach mirror the existing Tauri/Rust +/// implementation (`src-tauri/src/commands/server.rs`) as a design +/// reference only -- no logic is shared or ported between the two apps, +/// per this project's standing Native/Classic independence rule. +public actor ServerProcessService { + /// `Server` has no explicit jar-filename field yet (unlike the Rust + /// `start_server` command, which takes an explicit `jar_file: String` + /// argument from its caller). Until a future task adds one, this + /// service assumes the convention that the server jar is named + /// `server.jar` and lives directly inside `server.path`. This + /// convention is local to this service -- `Server.swift` itself is not + /// touched. + private static let jarFileName = "server.jar" + + /// Bounded wait for graceful shutdown (writing `"stop\n"` to stdin) + /// before escalating to `Process.terminate()` (SIGTERM). The Rust + /// reference implementation has no such timeout/fallback at all -- an + /// acknowledged gap in that reference. This is a deliberate + /// improvement: 15s is long enough for a typical vanilla/Paper world + /// save-on-stop, short enough that a hung server doesn't leave the + /// caller waiting indefinitely. Overridable via `init` so tests can + /// exercise the escalation path without a real 15s wait. + private let gracefulStopTimeout: Duration + + private struct RunningProcess { + let process: Process + let stdin: FileHandle + /// The child's stdout `Pipe`, retained explicitly (rather than only + /// reachable via `process.standardOutput as? Pipe`) so + /// `stdoutLines(serverId:)` has a direct, typed handle to read from + /// -- task 3-8's job, per `start(server:)`'s doc comment below. + let stdout: Pipe + /// Set when `stdoutLines(serverId:)` claims a live reader; gates `handleTermination`'s salvage. + var claimed = false + } + + private var runningProcesses: [String: RunningProcess] = [:] + + /// Stdout `Pipe`s salvaged from `RunningProcess` entries whose process + /// already exited (`handleTermination` removed them from + /// `runningProcesses`), but which `stdoutLines(serverId:)` hasn't read + /// yet. Closes a narrow race: a process that exits very quickly can be + /// reaped by `start(server:)`'s monitoring `Task` before the caller's + /// first `stdoutLines` call -- without this cache, `stdoutLines` would + /// key "anything to read" on `runningProcesses` liveness alone and + /// wrongly return `nil`, even though the OS pipe still buffers every + /// byte written (a late-attaching reader sees EOF right after that + /// data, not nothing) -- most importantly, a crash message from a + /// server that dies immediately after launch. Entries are removed when + /// `stdoutLines` claims one, or when `start(server:)` runs again for + /// the same `serverId`. + /// + /// Only populated when never claimed live (`RunningProcess.claimed == + /// false`) at termination -- see `handleTermination`. If it *was* + /// claimed (log view open, then `.task(id: server.status)` re-fires on + /// stop), the live reader may still be mid-teardown, so salvaging would + /// create a second reader splitting bytes unpredictably. + private var terminatedStdout: [String: Pipe] = [:] + + private let eventContinuation: AsyncStream.Continuation + + /// Single actor-wide stream of status-change events, not one per + /// server -- this task's scope doesn't need per-server granularity, and + /// it establishes the same AsyncStream-based observation pattern task + /// 3-8 (log streaming) will also use, per its own task description. + public let events: AsyncStream + + public init(gracefulStopTimeout: Duration = .seconds(15)) { + self.gracefulStopTimeout = gracefulStopTimeout + var continuation: AsyncStream.Continuation! + self.events = AsyncStream { continuation = $0 } + self.eventContinuation = continuation + } + + /// Whether this instance currently has a tracked running process for + /// `serverId`. `async` (even though actor-isolated methods are + /// implicitly asynchronous to external callers regardless) to make the + /// cross-actor call site explicit at the call site. + public func isRunning(serverId: String) async -> Bool { + self.runningProcesses[serverId] != nil + } + + /// A live, line-oriented view of `serverId`'s stdout, or `nil` if this + /// instance has never started a process for that id, or has already + /// served (and discarded) its buffered stdout via a previous call to + /// this method. `nil`, not a thrown error: "no live stdout right now" is + /// an expected, steady-state outcome for a server that simply isn't + /// running -- the log view's job is to reflect that (e.g. show nothing / + /// a placeholder) rather than treat it as exceptional, so a typed error + /// would just force every caller to immediately catch-and-ignore it. + /// + /// Checks `runningProcesses` first (marking the pipe claimed -- see + /// `RunningProcess.claimed`), then falls back to `terminatedStdout` for + /// an already-exited, never-claimed process's unread buffered stdout. + /// + /// Reads incrementally via `FileHandle.bytes` (an `AsyncSequence` of + /// bytes), per the swift-concurrency skill's guidance for bridging + /// callback/handle-based APIs to `AsyncStream` -- deliberately not + /// `readToEnd()`/`readToEndCompat()` (see `JavaLaunchHarness`), which + /// blocks until the pipe's write end closes and so cannot serve a + /// *live* stream. Bytes accumulate into a line buffer, yielded as + /// `String`s on each `\n`, with any trailing partial line flushed once + /// the loop ends. + /// + /// The reading `Task` is spawned with `@concurrent` rather than + /// inheriting this actor's isolation -- per the swift-concurrency + /// skill's actor guidance, a long-lived unstructured `Task` should never + /// pin an actor's serial executor for its whole lifetime, which reading + /// a potentially-hours-long stream would otherwise do. `onTermination` + /// cancels the reading `Task` when the caller stops iterating; the + /// process exiting on its own needs no separate handling, since the + /// child's pipe end closing produces a natural EOF that falls through + /// to `continuation.finish()`. + /// + /// Single-consumer only, like any `AsyncStream`: calling this twice for + /// the same *live* `serverId` starts two independent readers on the + /// same pipe descriptor, splitting bytes unpredictably. Callers (today: + /// one `ServerLogViewModel` per server) must not do that -- the + /// `claimed` flag only protects the live-then-terminated re-entry + /// pattern (see `terminatedStdout`), not two concurrent live callers. + public func stdoutLines(serverId: String) async -> AsyncStream? { + let stdoutPipe: Pipe + if let running = self.runningProcesses[serverId] { + stdoutPipe = running.stdout + // Mark claimed before returning: a same-race exit must not salvage this pipe out from under the reader. + self.runningProcesses[serverId]?.claimed = true + } else if let terminated = self.terminatedStdout.removeValue(forKey: serverId) { + stdoutPipe = terminated + } else { + return nil + } + let handle = stdoutPipe.fileHandleForReading + + return AsyncStream { continuation in + let task = Task { @concurrent in + var lineBuffer = Data() + do { + for try await byte in handle.bytes { + if byte == UInt8(ascii: "\n") { + continuation.yield(Self.decodeLine(lineBuffer)) + lineBuffer.removeAll(keepingCapacity: true) + } else { + lineBuffer.append(byte) + } + } + } catch { + // `FileHandle.AsyncBytes` only throws for a genuine + // read error on the underlying descriptor (e.g. it was + // closed out from under us). There's nothing to retry + // or recover mid-stream, so this just falls through to + // flushing any trailing partial line and finishing -- + // the same outcome as a clean EOF. + } + if !lineBuffer.isEmpty { + continuation.yield(Self.decodeLine(lineBuffer)) + } + continuation.finish() + } + + continuation.onTermination = { _ in task.cancel() } + } + } + + private static func decodeLine(_ data: Data) -> String { + String(bytes: data, encoding: .utf8) ?? "" + } + + /// Launches `server`'s Java process. Sets `currentDirectoryURL` to + /// `server.path` so the `-jar` argument can be a bare filename rather + /// than an absolute path (matching the Rust reference's `current_dir` + /// approach). `standardOutput` is redirected to a `Pipe` this actor + /// retains (see `RunningProcess.stdout`) so `stdoutLines(serverId:)` + /// can read it live -- task 3-8's job. `standardError` is redirected to + /// its own `Pipe` too: Minecraft duplicates its console output to both + /// stdout and stderr, and the Rust reference app likewise never surfaces + /// stderr content, so nothing decodes or yields it -- but the pipe's + /// read end is still drained continuously in the background. Leaving it + /// unread would let the OS pipe buffer (~64KB on macOS) fill up under + /// enough console spam, blocking the child's next `write()` to stderr + /// and potentially hanging the Minecraft server itself. + public func start(server: Server) async throws { + guard self.runningProcesses[server.id] == nil else { + throw ServerProcessError.alreadyRunning(serverId: server.id) + } + guard let javaPath = server.javaPath else { + throw ServerProcessError.javaPathNotConfigured(serverId: server.id) + } + + // Drop any stale, never-claimed pipe from a previous run of this + // server id -- `stdoutLines` should serve the *new* process's + // stdout, not a leftover reference to the old one. + self.terminatedStdout.removeValue(forKey: server.id) + + let process = Process() + process.executableURL = URL(fileURLWithPath: javaPath) + process.currentDirectoryURL = URL(fileURLWithPath: server.path, isDirectory: true) + process.arguments = self.buildArguments(for: server) + + let stdinPipe = Pipe() + let stdoutPipe = Pipe() + let stderrPipe = Pipe() + process.standardInput = stdinPipe + process.standardOutput = stdoutPipe + process.standardError = stderrPipe + + try process.run() + + self.runningProcesses[server.id] = RunningProcess( + process: process, + stdin: stdinPipe.fileHandleForWriting, + stdout: stdoutPipe, + ) + + Self.drainAndDiscard(stderrPipe.fileHandleForReading) + + let serverId = server.id + Task { [weak self] in + let terminationStatus = await Self.awaitTermination(of: process) + let status: ServerStatus = terminationStatus == 0 ? .offline : .crashed + await self?.handleTermination(serverId: serverId, status: status) + } + } + + /// Continuously reads and discards bytes from `handle` in the + /// background, purely to keep its OS pipe buffer from filling up and + /// blocking the writer -- see the note on `standardError` above. Not + /// tied to this actor's isolation (`@concurrent`, like + /// `stdoutLines(serverId:)`'s reader) since it lives for the whole + /// process lifetime and reads nothing actor-relevant. Ends naturally + /// when the pipe's write end closes (process exit), same as + /// `stdoutLines`'s reader. + private static func drainAndDiscard(_ handle: FileHandle) { + Task { @concurrent in + do { + for try await _ in handle.bytes {} + } catch { + // Same rationale as `stdoutLines(serverId:)`: a read error + // here just means the descriptor closed out from under us, + // which is an ordinary way for this loop to end. + } + } + } + + /// Stops the tracked process for `serverId`. Primary mechanism: write + /// `"stop\n"` to the process's stdin, letting Minecraft's own console + /// command handler shut down gracefully (matching the Rust reference -- + /// no SIGTERM/SIGKILL as the first resort). If the process hasn't + /// exited within `gracefulStopTimeout`, escalates to + /// `Process.terminate()` (SIGTERM). + /// + /// Returns once either the process has exited or the escalation has + /// been issued -- the resulting `.offline`/`.crashed` status is + /// reported asynchronously via `events` once the monitoring task set up + /// in `start(server:)` observes the actual termination, since that's + /// this actor's single source of truth for "did it actually exit." + public func stop(serverId: String) async throws { + guard let running = self.runningProcesses[serverId] else { + throw ServerProcessError.serverNotRunning(serverId: serverId) + } + + // Writing can fail if the process already exited and closed its + // stdin between our lookup above and this write; that's not a hard + // failure here -- the goal ("process no longer running") may + // already be achieved, so fall through to the wait/escalation + // below rather than propagating this as an error. + try? running.stdin.write(contentsOf: Data("stop\n".utf8)) + + let deadline = ContinuousClock.now.advanced(by: self.gracefulStopTimeout) + while self.runningProcesses[serverId] != nil, ContinuousClock.now < deadline { + try? await Task.sleep(for: .milliseconds(100)) + } + + if let stillRunning = self.runningProcesses[serverId] { + stillRunning.process.terminate() + } + } + + /// Builds the JVM launch arguments: `-Xmx{memory}M -Xms{memory}M + /// [extra JVM args] -jar server.jar nogui`, matching the Rust + /// reference's argument shape. + /// + /// `Server.jvmArgs` is a single optional raw `String` (unlike Rust's + /// already-tokenized, shell-metacharacter-validated `Vec`). + /// Since `Process` never goes through a shell here (same as Rust's + /// `Command::new().args()`), classic shell-injection isn't the threat + /// model -- a light touch (split on whitespace, drop empty tokens, pass + /// each as a separate argument) is enough; a validation/sanitization + /// subsystem is out of this task's scope. + private func buildArguments(for server: Server) -> [String] { + var arguments = [ + "-Xmx\(server.memory)M", + "-Xms\(server.memory)M" + ] + if let jvmArgs = server.jvmArgs { + arguments.append(contentsOf: jvmArgs.split(whereSeparator: \.isWhitespace).map(String.init)) + } + arguments.append(contentsOf: ["-jar", Self.jarFileName, "nogui"]) + return arguments + } + + /// Bridges `Process.terminationHandler` (a system-driven callback fired + /// on an arbitrary queue, not a blocking wait) to `async`/`await`. + /// + /// Deliberately not `waitUntilExit()`: that call blocks the calling + /// thread for as long as the child runs, which -- if driven from + /// within this actor's isolation -- would hold this actor's executor + /// hostage for the server's entire runtime, making `stop`/`isRunning` + /// uncallable until it exits. Using a continuation resumed by + /// `terminationHandler` is a true suspension instead: the `Task` in + /// `start(server:)` that awaits this can inherit the actor's isolation + /// safely, because suspending here releases the actor's executor for + /// other work rather than blocking it. + private static func awaitTermination(of process: Process) async -> Int32 { + await withCheckedContinuation { continuation in + process.terminationHandler = { terminatedProcess in + continuation.resume(returning: terminatedProcess.terminationStatus) + } + } + } + + private func handleTermination(serverId: String, status: ServerStatus) { + if let running = self.runningProcesses.removeValue(forKey: serverId) { + try? running.stdin.close() + // Only salvage an unclaimed pipe -- see `terminatedStdout`'s doc. + if !running.claimed { + self.terminatedStdout[serverId] = running.stdout + } + } + self.eventContinuation.yield(ServerProcessEvent(serverId: serverId, status: status)) + } +} diff --git a/apps/native-macos/Tests/CoreTests/FloatingConsolePanelTests.swift b/apps/native-macos/Tests/CoreTests/FloatingConsolePanelTests.swift new file mode 100644 index 0000000..fe69392 --- /dev/null +++ b/apps/native-macos/Tests/CoreTests/FloatingConsolePanelTests.swift @@ -0,0 +1,62 @@ +import SwiftUI +import Testing +@testable import Core + +/// Mirrors `PanelSpikeTests.swift`'s style: assert the panel's static +/// AppKit configuration matches the Phase 3-A spike's confirmed-winning +/// `NonactivatingGlassPanel` settings exactly. A `Text` view stands in for +/// the real `FloatingConsoleContentView` here -- this suite is about the +/// panel's window configuration, not its hosted content. +@MainActor +@Test("FloatingConsolePanel matches the confirmed NSPanel bridge configuration") +func floatingConsolePanelConfiguration() { + let panel = FloatingConsolePanel(title: "Console — Test Server", content: Text("log content")) + + #expect(panel.styleMask.contains(.nonactivatingPanel)) + #expect(panel.styleMask.contains(.titled)) + #expect(panel.styleMask.contains(.resizable)) + #expect(panel.styleMask.contains(.closable)) + #expect(panel.isFloatingPanel) + #expect(panel.level == .floating) + #expect(panel.collectionBehavior.contains(.canJoinAllSpaces)) + #expect(panel.collectionBehavior.contains(.fullScreenAuxiliary)) + #expect(panel.titlebarAppearsTransparent) + #expect(panel.title == "Console — Test Server") +} + +/// `FloatingConsolePanelController` is the riskiest piece of new state in +/// task 3-9 (it must never call `ServerLogViewModel.streamLogs()` itself -- +/// see its doc comment), so its `isVisible` state machine is exercised +/// directly rather than only checking static panel configuration. +@MainActor +@Test("FloatingConsolePanelController tracks isVisible through show/hide/toggle/dismiss") +func floatingConsolePanelControllerLifecycle() { + let service = ServerProcessService() + let viewModel = ServerLogViewModel(serverId: "srv-1", processService: service) + let controller = FloatingConsolePanelController(serverName: "Test Server", viewModel: viewModel) + + #expect(!controller.isVisible) + + controller.show() + #expect(controller.isVisible) + + controller.hide() + #expect(!controller.isVisible) + + controller.toggle() + #expect(controller.isVisible) + + controller.toggle() + #expect(!controller.isVisible) + + controller.show() + #expect(controller.isVisible) + + controller.dismiss() + #expect(!controller.isVisible) + + // show() after dismiss() must lazily recreate the panel rather than + // reusing (or failing to reuse) a closed one. + controller.show() + #expect(controller.isVisible) +} diff --git a/apps/native-macos/Tests/CoreTests/SecurityTests.swift b/apps/native-macos/Tests/CoreTests/SecurityTests.swift new file mode 100644 index 0000000..986ef2d --- /dev/null +++ b/apps/native-macos/Tests/CoreTests/SecurityTests.swift @@ -0,0 +1,288 @@ +import Foundation +import Testing +@testable import Core + +// Faithful, one-test-per-Rust-test port of `security.rs`'s unit tests (see +// Task 3-11). Each Swift test below names, in its doc comment, the exact +// Rust test it corresponds to. The four `ipc_contract_*` tests per Rust +// group tested an IPC dispatcher envelope that doesn't exist in this +// Swift app (there is no IPC layer here) -- those are ported as direct +// tests of the underlying function they exercised in Rust, per this +// task's scope note. +// +// Error-message assertions compare against `SecurityError.errorDescription` +// verbatim against the Rust source's string literals -- this is the +// "仕様の一致率" (spec-match fidelity) this port is reviewed against. + +// MARK: - authorize + +/// Ports: `admin_can_execute_any_action`. +@Test("admin can execute any action") +func adminCanExecuteAnyAction() throws { + try authorize(role: .admin, action: "delete_world") +} + +/// Ports: `user_can_start_and_stop_only`. +@Test("user can only start_server and stop_server") +func userCanStartAndStopOnly() throws { + try authorize(role: .user, action: "start_server") + try authorize(role: .user, action: "stop_server") + + do { + try authorize(role: .user, action: "delete_world") + Issue.record("expected authorize to throw for role=user action=delete_world") + } catch let error as SecurityError { + #expect(error.errorDescription == "Forbidden: role user is not allowed to perform action delete_world") + } +} + +/// Ports: `viewer_can_read_non_mutating_action`. +@Test("viewer can perform non-mutating (read) actions") +func viewerCanReadNonMutatingAction() throws { + try authorize(role: .viewer, action: "get_server_status") +} + +/// Ports: `viewer_is_forbidden_for_mutating_action`. +@Test("viewer is forbidden from mutating actions") +func viewerIsForbiddenForMutatingAction() throws { + do { + try authorize(role: .viewer, action: "start_server") + Issue.record("expected authorize to throw for role=viewer action=start_server") + } catch let error as SecurityError { + #expect(error.errorDescription == "Forbidden: role viewer is not allowed to perform action start_server") + } +} + +/// Ports: `ipc_contract_authorize_action_response_shape`, adapted to test +/// `authorize` directly (no IPC dispatcher exists in this app) -- success +/// here *is* "allowed", there's no separate envelope to shape-check. +@Test("authorize succeeds (represents \"allowed\") for a role/action the role permits") +func authorizeSucceedsRepresentsAllowed() throws { + try authorize(role: .user, action: "start_server") +} + +/// Ports: `ipc_contract_authorize_action_missing_fields_error`, adapted to +/// test that an empty/whitespace-only `action` throws the exact Rust +/// error message (there's no payload/missing-field envelope in Swift -- +/// the equivalent "missing field" here is an empty `action` string). +@Test("authorize throws the exact empty-action error for empty and whitespace-only action") +func authorizeEmptyActionThrowsExactMessage() throws { + for emptyAction in ["", " "] { + do { + try authorize(role: .admin, action: emptyAction) + Issue.record("expected authorize to throw for action=\"\(emptyAction)\"") + } catch let error as SecurityError { + #expect(error.errorDescription == "security_gateway authorize_action requires non-empty payload.action") + } + } +} + +// MARK: - check_rate_limit (RateLimiter) + +/// Ports: `rate_limit_blocks_rapid_repeated_calls`. +@Test("rate limit blocks a second call from the same user inside the window") +func rateLimitBlocksRapidRepeatedCalls() async throws { + let limiter = RateLimiter() + let start = ContinuousClock.now + + try await limiter.checkRateLimit(userId: "user-1", now: start) + + do { + try await limiter.checkRateLimit(userId: "user-1", now: start + RateLimiter.rateLimitWindow - .milliseconds(1)) + Issue.record("expected checkRateLimit to throw for a repeated call inside the window") + } catch let error as SecurityError { + #expect(error.errorDescription == "Forbidden: rate limit exceeded for user user-1") + } +} + +/// Ports: `rate_limit_allows_after_window`. +@Test("rate limit allows a second call from the same user once the window elapses") +func rateLimitAllowsAfterWindow() async throws { + let limiter = RateLimiter() + let start = ContinuousClock.now + + try await limiter.checkRateLimit(userId: "user-1", now: start) + try await limiter.checkRateLimit(userId: "user-1", now: start + RateLimiter.rateLimitWindow) +} + +/// Ports: `rate_limit_prunes_expired_entries`. +@Test("rate limit prunes expired entries and still allows a fresh user") +func rateLimitPrunesExpiredEntries() async throws { + let limiter = RateLimiter() + let start = ContinuousClock.now + let staleInstant = start - (RateLimiter.rateLimitWindow + .milliseconds(10)) + + try await limiter.checkRateLimit(userId: "stale-user", now: staleInstant) + try await limiter.checkRateLimit(userId: "fresh-user", now: start) + + let trackedUserIds = await limiter.trackedUserIds() + #expect(trackedUserIds == ["fresh-user"]) +} + +/// Ports: `ipc_contract_rate_limit_check_response_shape`, adapted to test +/// `RateLimiter.checkRateLimit` directly for a fresh, unique user id. +@Test("rate limit check succeeds for a fresh, unique user id") +func rateLimitCheckSucceedsForFreshUser() async throws { + let limiter = RateLimiter() + try await limiter.checkRateLimit(userId: "unique-user-\(UUID().uuidString)") +} + +/// Ports: `ipc_contract_rate_limit_check_missing_field_error`, adapted to +/// test that an empty/whitespace-only `userId` throws the exact Rust +/// error message. +@Test("rate limit check throws the exact empty-userId error for empty and whitespace-only userId") +func rateLimitCheckEmptyUserIdThrowsExactMessage() async throws { + let limiter = RateLimiter() + for emptyUserId in ["", " "] { + do { + try await limiter.checkRateLimit(userId: emptyUserId) + Issue.record("expected checkRateLimit to throw for userId=\"\(emptyUserId)\"") + } catch let error as SecurityError { + #expect(error.errorDescription == "security_gateway rate_limit_check requires non-empty payload.userId") + } + } +} + +// MARK: - resolve_safe_path + +/// Base directory shared by the `resolveSafePath` tests, mirroring the +/// Rust tests' `/mc-vector-security/app-data`. Computed once per +/// call (not written to disk -- `resolveSafePath` is a pure string +/// operation and never touches the filesystem) with any trailing slash +/// stripped so assertions can predictably concatenate `"/"` themselves. +private func safePathTestBase() -> String { + var base = FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-security", isDirectory: true) + .appendingPathComponent("app-data", isDirectory: true) + .path + if base.hasSuffix("/") { + base.removeLast() + } + return base +} + +/// Ports: `resolve_safe_path_rejects_traversal`. +@Test("resolve safe path rejects a traversal attempt") +func resolveSafePathRejectsTraversal() throws { + let base = safePathTestBase() + + do { + _ = try resolveSafePath(base: base, input: "../etc/passwd") + Issue.record("expected resolveSafePath to throw for a traversal input") + } catch let error as SecurityError { + #expect(error.errorDescription == "Path traversal detected") + } +} + +/// Ports: `resolve_safe_path_builds_absolute_path`. +@Test("resolve safe path builds base joined with input") +func resolveSafePathBuildsAbsolutePath() throws { + let base = safePathTestBase() + + let resolved = try resolveSafePath(base: base, input: "servers/a") + + #expect(resolved == base + "/servers/a") +} + +/// Ports: `resolve_safe_path_rejects_windows_drive_relative_prefix`. This +/// must reject regardless of host OS -- it's the Rust original's +/// defensive byte-level check, exercised here on macOS deliberately. +@Test("resolve safe path rejects a Windows drive-relative prefix even on non-Windows hosts") +func resolveSafePathRejectsWindowsDriveRelativePrefix() throws { + let base = safePathTestBase() + + do { + _ = try resolveSafePath(base: base, input: "C:windows\\temp") + Issue.record("expected resolveSafePath to throw for a drive-relative prefix input") + } catch let error as SecurityError { + #expect(error.errorDescription == "Path traversal detected") + } +} + +/// Ports: `ipc_contract_resolve_safe_path_response_shape`, adapted to +/// test `resolveSafePath` directly: the resolved path equals +/// `base/servers/a`, with no IPC envelope to shape-check. +@Test("resolve safe path succeeds and the resolved path matches base/input") +func resolveSafePathSucceedsAndMatchesJoin() throws { + let base = safePathTestBase() + + let resolved = try resolveSafePath(base: base, input: "servers/a") + + #expect(resolved == base + "/servers/a") +} + +/// Ports: `ipc_contract_resolve_safe_path_missing_fields_error`, adapted +/// to test that an empty/whitespace-only `base` or `input` throws the +/// exact Rust error message, in both directions. +@Test("resolve safe path throws the exact empty-fields error for empty/whitespace base or input") +func resolveSafePathEmptyFieldsThrowsExactMessage() throws { + let base = safePathTestBase() + let expectedMessage = "security_gateway resolve_safe_path requires non-empty payload.base and payload.input" + + for emptyBase in ["", " "] { + do { + _ = try resolveSafePath(base: emptyBase, input: "servers/a") + Issue.record("expected resolveSafePath to throw for base=\"\(emptyBase)\"") + } catch let error as SecurityError { + #expect(error.errorDescription == expectedMessage) + } + } + + for emptyInput in ["", " "] { + do { + _ = try resolveSafePath(base: base, input: emptyInput) + Issue.record("expected resolveSafePath to throw for input=\"\(emptyInput)\"") + } catch let error as SecurityError { + #expect(error.errorDescription == expectedMessage) + } + } +} + +// MARK: - build_audit_entry + +/// Ports: `build_audit_entry_contains_required_fields`. +@Test("build audit entry contains user, action, and a retrievable timestamp") +func buildAuditEntryContainsRequiredFields() throws { + let entry = try buildAuditEntry(user: "user-1", action: "start_server") + + #expect(entry.user == "user-1") + #expect(entry.action == "start_server") + #expect(entry.timestamp > 0) +} + +/// Ports: `ipc_contract_audit_log_response_shape`, adapted to test +/// `buildAuditEntry` directly: the returned fields match the inputs, +/// with no IPC envelope to shape-check. +@Test("build audit entry succeeds with fields matching the inputs") +func auditLogSucceedsWithMatchingFields() throws { + let entry = try buildAuditEntry(user: "audit-user", action: "read_logs") + + #expect(entry.user == "audit-user") + #expect(entry.action == "read_logs") +} + +/// Ports: `ipc_contract_audit_log_missing_fields_error`, adapted to test +/// that an empty/whitespace-only `user` or `action` throws the exact +/// Rust error message, in both directions. +@Test("build audit entry throws the exact empty-fields error for empty/whitespace user or action") +func auditLogEmptyFieldsThrowsExactMessage() throws { + let expectedMessage = "security_gateway audit_log requires non-empty payload.user and payload.action" + + for emptyUser in ["", " "] { + do { + _ = try buildAuditEntry(user: emptyUser, action: "read_logs") + Issue.record("expected buildAuditEntry to throw for user=\"\(emptyUser)\"") + } catch let error as SecurityError { + #expect(error.errorDescription == expectedMessage) + } + } + + for emptyAction in ["", " "] { + do { + _ = try buildAuditEntry(user: "audit-user", action: emptyAction) + Issue.record("expected buildAuditEntry to throw for action=\"\(emptyAction)\"") + } catch let error as SecurityError { + #expect(error.errorDescription == expectedMessage) + } + } +} diff --git a/apps/native-macos/Tests/CoreTests/ServerListViewModelProcessTests.swift b/apps/native-macos/Tests/CoreTests/ServerListViewModelProcessTests.swift new file mode 100644 index 0000000..cd62be7 --- /dev/null +++ b/apps/native-macos/Tests/CoreTests/ServerListViewModelProcessTests.swift @@ -0,0 +1,396 @@ +import Foundation +import Testing +@testable import Core + +/// Writes an executable shell script fixture to a temp file and returns its +/// URL. Mirrors `ServerProcessServiceTests`' fixture-script pattern (real, +/// tiny, purpose-built shell scripts standing in for a long-running +/// Minecraft server process, rather than mocking `Process`) -- duplicated +/// here rather than shared because the originals are file-private to +/// `ServerProcessServiceTests.swift`. +/// +/// Unlike `ServerProcessServiceTests`, these tests go through +/// `ServerListViewModel` -- a real `ServerProcessService` is injected via +/// `ServerListViewModel`'s injectable initializer, so both the actor's own +/// logic (already covered by `ServerProcessServiceTests`) *and* the +/// ViewModel's `@concurrent` event-subscription `Task` that wraps it get +/// exercised end-to-end. +private func makeScriptFixture(_ contents: String) throws -> URL { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-vm-process-fixture-\(UUID().uuidString)", isDirectory: false) + .appendingPathExtension("sh") + try Data(contents.utf8).write(to: url) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: url.path) + return url +} + +/// Reads stdin lines forever, exiting 0 the moment it reads a line equal to +/// "stop" -- see `ServerProcessServiceTests.stopOnStdinScript` for the +/// original. +private let stopOnStdinScript = """ +#!/bin/sh +while IFS= read -r line; do + if [ "$line" = "stop" ]; then + exit 0 + fi +done +exit 0 +""" + +/// Exits non-zero shortly after launch, simulating a server crashing on its +/// own without ever being asked to stop -- see +/// `ServerProcessServiceTests.crashesShortlyScript` for the original. +private let crashesShortlyScript = """ +#!/bin/sh +sleep 0.2 +exit 7 +""" + +private func makeTempFileURL() -> URL { + FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-servers-vm-process-test-\(UUID().uuidString)", isDirectory: false) + .appendingPathExtension("json") +} + +private func makeServer(id: String = "srv-1", javaPath: String?) -> Server { + Server( + id: id, + name: "Test Server", + version: "1.21.1", + software: "paper", + port: 25565, + memory: 512, + path: FileManager.default.temporaryDirectory.path, + status: .offline, + javaPath: javaPath, + ) +} + +/// Polls `condition` until it returns `true` or `timeout` elapses, sleeping +/// `pollInterval` between checks. +/// +/// Used instead of a single fixed-duration `Task.sleep` guess to wait on +/// asynchronous status propagation from `ServerListViewModel`'s background +/// event-subscription `Task` (see that class's `init`): a fixed sleep would +/// either race a slow CI machine (flaky) or pad every run with dead time +/// long enough to always be safe (slow). `@MainActor` because every caller +/// inspects `viewModel` state, which is `@MainActor`-isolated. +/// +/// Default bumped from 2s to 8s (task 3-10): with this file's crash-based +/// tests now outnumbering the original one (3 vs. 1 -- see the Activity +/// Drawer tests below), Swift Testing's parallel test execution puts enough +/// concurrent real `Process` launches under contention that 2s was +/// occasionally too tight purely from CPU scheduling delay, not any actual +/// functional problem -- `waitUntil` still returns as soon as `condition` +/// is true, so this only affects how long a genuinely-broken test takes to +/// fail, not how long a passing run takes. +@MainActor +private func waitUntil( + timeout: Duration = .seconds(8), + pollInterval: Duration = .milliseconds(20), + _ condition: () -> Bool, +) async { + let deadline = ContinuousClock.now.advanced(by: timeout) + while !condition(), ContinuousClock.now < deadline { + try? await Task.sleep(for: pollInterval) + } +} + +@MainActor +@Test("startSelectedServer() sets status to .online synchronously on success") +func startSelectedServerSetsStatusToOnlineOnSuccess() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(stopOnStdinScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + + await viewModel.startSelectedServer() + + // This is the synchronous-success path: `startSelectedServer()` sets + // `.online` itself once `processService.start(server:)` returns, with + // no need to round-trip through the event stream. See + // `crashPropagatesToViewModelStatusViaEventStream` below for the + // complementary case that *does* require the event stream. + #expect(viewModel.selectedServer?.status == .online) + #expect(viewModel.error == nil) + + // Clean up: stop the real child process so the test doesn't leak it. + await viewModel.stopSelectedServer() +} + +// MARK: - Error alert (task 3-12 code-review fix) + +// +// `ServerListViewModel.error` was set on failure but never read by any View +// -- a failed start/stop had zero user-visible signal. These tests exercise +// the ViewModel's state directly (the `Identifiable` `ServerListViewModelError` +// wrapper populating and clearing correctly), not the rendered `.alert` -- +// this codebase has no UI-level testing infrastructure, matching this file's +// existing convention of asserting on `viewModel` state rather than View +// output. + +@MainActor +@Test("a failed startSelectedServer() populates error with the failure's message") +func startSelectedServerFailurePopulatesError() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + let server = makeServer(javaPath: nil) + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [server])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + + await viewModel.startSelectedServer() + + // A missing `javaPath` throws `ServerProcessError.javaPathNotConfigured` + // (see `ServerProcessService.start(server:)`) before any process is + // ever launched, so this is a deterministic, real failure path -- not a + // simulated/mocked one. + let expectedMessage = try #require( + ServerProcessError.javaPathNotConfigured(serverId: server.id).errorDescription, + ) + let error = try #require(viewModel.error) + #expect(error.message == expectedMessage) + + // The optimistic `.starting` status is reverted back to the server's + // prior status (`.offline`) on failure -- same assertion + // `startSelectedServerSetsStatusToOnlineOnSuccess` makes for the + // success path's `.online`. + #expect(viewModel.selectedServer?.status == .offline) + + // `clearError()` is the ViewModel-side hook `RootView`'s alert calls + // from its dismiss/OK action. + viewModel.clearError() + #expect(viewModel.error == nil) +} + +@MainActor +@Test( + "a process that crashes on its own transitions the ViewModel's status to .crashed via the event-subscription Task", +) +func crashPropagatesToViewModelStatusViaEventStream() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(crashesShortlyScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + + await viewModel.startSelectedServer() + // `startSelectedServer()`'s synchronous-success path (exercised above + // in `startSelectedServerSetsStatusToOnlineOnSuccess`) has no code path + // that ever sets `.crashed` -- only the actor's termination monitor + // (via `processService.events`) and this ViewModel's `@concurrent` + // event-subscription `Task` set up in `init` can observe and apply + // that. The fixture script exits non-zero ~0.2s after launch, well + // after `start(server:)` (and therefore this line) returns, so this is + // still the synchronous `.online` state. + #expect(viewModel.selectedServer?.status == .online) + + // Deliberately does NOT call `stopSelectedServer()` -- the crash must + // be observed purely through the event stream / `@concurrent` + // subscription Task, not any code path reachable from + // `stopSelectedServer()`. If that subscription Task were dead (e.g. the + // `@concurrent` regression this test guards against, where a missing + // `@concurrent` silently made the `await` inside it a no-op), this poll + // would time out and the final `#expect` below would fail on `.online` + // rather than observing `.crashed`. + await waitUntil { viewModel.selectedServer?.status == .crashed } + + #expect(viewModel.selectedServer?.status == .crashed) +} + +@MainActor +@Test("stopSelectedServer() eventually sets status to .offline via the event-subscription Task") +func stopSelectedServerEventuallySetsStatusToOfflineViaEventStream() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(stopOnStdinScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + + await viewModel.startSelectedServer() + #expect(viewModel.selectedServer?.status == .online) + + // `stopSelectedServer()` sets `.stopping` optimistically and returns + // once the actor's `stop(serverId:)` returns, but the definitive + // `.offline` (vs. `.crashed`) outcome is -- by design (see + // `ServerProcessService.stop`'s doc comment) -- only ever reported via + // `processService.events`, so it still requires the ViewModel's + // subscription Task to apply it. + await viewModel.stopSelectedServer() + + await waitUntil { viewModel.selectedServer?.status == .offline } + + #expect(viewModel.selectedServer?.status == .offline) +} + +// MARK: - Activity Drawer (task 3-10) + +// +// These tests exercise `ServerListViewModel.activityLog` exclusively through +// `startSelectedServer()`/`stopSelectedServer()` -- same pattern as the tests +// above. A successful start's `.online` entry is logged directly and +// synchronously (no stream involved); `.offline`/`.crashed` entries still +// arrive via real event propagation from `processService.events`, and +// deliberately not via any second `for await event in processService.events` +// loop: `apply(_:)` (invoked by the ViewModel's own single subscription +// `Task` set up in `init`) is the only place those two entries are appended, +// so these tests observe the *effect* of that subscriber rather than +// creating a competing one. + +@MainActor +@Test("a successful start appends an ActivityEntry with the resulting .online status") +func startAppendsOnlineActivityEntry() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(stopOnStdinScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + let serverId = try #require(viewModel.servers.first?.id) + let serverName = try #require(viewModel.servers.first?.name) + + await viewModel.startSelectedServer() + + // Unlike the .offline/.crashed entries below (which arrive later via + // the event-subscription Task, so tests need `waitUntil`), a start's + // `.online` entry is logged directly and synchronously from + // `startSelectedServer()`'s success path -- no polling needed, matching + // how `startSelectedServerSetsStatusToOnlineOnSuccess` asserts + // `.online` status with no `waitUntil` either. This is the regression + // test for the task 3-10 code-review finding that a successful start + // never produced an `ActivityEntry` at all. + #expect(viewModel.activityLog.count == 1) + let entry = try #require(viewModel.activityLog.first) + #expect(entry.serverId == serverId) + #expect(entry.serverName == serverName) + #expect(entry.kind == .serverStatusChange(.online)) + + // Clean up: stop the real child process so the test doesn't leak it. + await viewModel.stopSelectedServer() +} + +@MainActor +@Test("stopping a server appends an ActivityEntry with the resulting .offline status") +func stopAppendsOfflineActivityEntry() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(stopOnStdinScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + let serverId = try #require(viewModel.servers.first?.id) + let serverName = try #require(viewModel.servers.first?.name) + + await viewModel.startSelectedServer() + // A successful start now logs its own `.online` entry directly from + // `startSelectedServer()`'s success path (task 3-10 review fix) -- see + // `ServerListViewModel.activityLog`'s doc comment. + #expect(viewModel.activityLog.count == 1) + #expect(viewModel.activityLog.first?.kind == .serverStatusChange(.online)) + + await viewModel.stopSelectedServer() + await waitUntil { viewModel.activityLog.count == 2 } + + #expect(viewModel.activityLog.count == 2) + let entry = try #require(viewModel.activityLog.first) + #expect(entry.serverId == serverId) + #expect(entry.serverName == serverName) + #expect(entry.kind == .serverStatusChange(.offline)) +} + +@MainActor +@Test("a server crashing on its own appends an ActivityEntry with the resulting .crashed status") +func crashAppendsCrashedActivityEntry() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(crashesShortlyScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService()) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + let serverId = try #require(viewModel.servers.first?.id) + let serverName = try #require(viewModel.servers.first?.name) + + await viewModel.startSelectedServer() + // The successful start already logged its own `.online` entry + // synchronously (see `stopAppendsOfflineActivityEntry` above), so this + // waits for a *second* entry -- the crash's -- rather than merely + // `!activityLog.isEmpty`, which the start's own entry would already + // satisfy before the crash is even observed. + await waitUntil { viewModel.activityLog.count == 2 } + + #expect(viewModel.activityLog.count == 2) + let entry = try #require(viewModel.activityLog.first) + #expect(entry.serverId == serverId) + #expect(entry.serverName == serverName) + #expect(entry.kind == .serverStatusChange(.crashed)) + #expect(viewModel.activityLog.last?.kind == .serverStatusChange(.online)) +} + +@MainActor +@Test("activityLog trims to activityLogCap, dropping the oldest entries once exceeded") +func activityLogTrimsToCapacity() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + let scriptURL = try makeScriptFixture(crashesShortlyScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(javaPath: scriptURL.path)])) + + // A small cap keeps this test fast: it only needs more launches than the + // cap to prove the trim-on-overflow behavior, not hundreds of them. + let cap = 3 + let viewModel = ServerListViewModel(store: store, processService: ServerProcessService(), activityLogCap: cap) + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + + let iterations = cap + 2 + for _ in 0 ..< iterations { + await viewModel.startSelectedServer() + // Each crash removes the prior run's tracked process (see + // `ServerProcessService.handleTermination`), so re-starting the same + // server id on the next iteration is safe -- it won't hit + // `.alreadyRunning`. + await waitUntil { viewModel.selectedServer?.status == .crashed } + } + + #expect(viewModel.activityLog.count == cap) +} diff --git a/apps/native-macos/Tests/CoreTests/ServerListViewModelTests.swift b/apps/native-macos/Tests/CoreTests/ServerListViewModelTests.swift new file mode 100644 index 0000000..0fe3d4a --- /dev/null +++ b/apps/native-macos/Tests/CoreTests/ServerListViewModelTests.swift @@ -0,0 +1,143 @@ +import Foundation +import Testing +@testable import Core + +private func makeTempFileURL() -> URL { + FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-servers-vm-test-\(UUID().uuidString)", isDirectory: false) + .appendingPathExtension("json") +} + +private func makeServer(id: String = "srv-1", name: String = "Survival") -> Server { + Server( + id: id, + name: name, + version: "1.21.1", + software: "paper", + port: 25565, + memory: 4096, + path: "/servers/\(id)", + status: .online, + ) +} + +@MainActor +@Test("load() populates servers from an existing servers.json file") +func loadPopulatesServersFromExistingFile() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(id: "srv-1"), makeServer(id: "srv-2", name: "Modded")])) + + let viewModel = ServerListViewModel(store: store) + #expect(viewModel.servers.isEmpty) + + await viewModel.load() + + #expect(viewModel.servers.count == 2) + #expect(viewModel.servers.map(\.id) == ["srv-1", "srv-2"]) + #expect(viewModel.error == nil) +} + +@MainActor +@Test("load() treats a missing servers.json as an empty list, not an error") +func loadTreatsMissingFileAsEmptyList() async { + let fileURL = makeTempFileURL() + let store = ServerStore(fileURL: fileURL) + + let viewModel = ServerListViewModel(store: store) + await viewModel.load() + + #expect(viewModel.servers.isEmpty) + #expect(viewModel.error == nil) +} + +@MainActor +@Test("load() surfaces unexpected decode failures via error instead of crashing") +func loadSurfacesUnexpectedErrorsViaError() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + // Malformed JSON -- not a "missing file" case, so this should surface + // as `error` rather than being swallowed as an empty list. + try Data("not valid json".utf8).write(to: fileURL) + + let store = ServerStore(fileURL: fileURL) + let viewModel = ServerListViewModel(store: store) + + await viewModel.load() + + #expect(viewModel.servers.isEmpty) + #expect(viewModel.error != nil) +} + +@MainActor +@Test("selection starts nil and can be set to a loaded server's id") +func selectionStartsNilAndCanBeSet() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(id: "srv-1")])) + + let viewModel = ServerListViewModel(store: store) + #expect(viewModel.selection == nil) + + await viewModel.load() + viewModel.selection = viewModel.servers.first?.id + + #expect(viewModel.selection == "srv-1") +} + +@MainActor +@Test("selectedServer is nil when selection is nil") +func selectedServerIsNilWhenSelectionIsNil() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(id: "srv-1")])) + + let viewModel = ServerListViewModel(store: store) + await viewModel.load() + + #expect(viewModel.selection == nil) + #expect(viewModel.selectedServer == nil) +} + +@MainActor +@Test("selectedServer resolves the matching Server when selection is a loaded id") +func selectedServerResolvesMatchingServer() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [ + makeServer(id: "srv-1", name: "Survival"), + makeServer(id: "srv-2", name: "Modded") + ])) + + let viewModel = ServerListViewModel(store: store) + await viewModel.load() + viewModel.selection = "srv-2" + + #expect(viewModel.selectedServer?.id == "srv-2") + #expect(viewModel.selectedServer?.name == "Modded") +} + +@MainActor +@Test("selectedServer is nil when selection doesn't match any loaded server") +func selectedServerIsNilWhenSelectionDoesNotMatch() async throws { + let fileURL = makeTempFileURL() + defer { try? FileManager.default.removeItem(at: fileURL) } + + let store = ServerStore(fileURL: fileURL) + try await store.save(ServersFile(servers: [makeServer(id: "srv-1")])) + + let viewModel = ServerListViewModel(store: store) + await viewModel.load() + viewModel.selection = "srv-does-not-exist" + + #expect(viewModel.selectedServer == nil) +} diff --git a/apps/native-macos/Tests/CoreTests/ServerLogViewModelTests.swift b/apps/native-macos/Tests/CoreTests/ServerLogViewModelTests.swift new file mode 100644 index 0000000..5637c78 --- /dev/null +++ b/apps/native-macos/Tests/CoreTests/ServerLogViewModelTests.swift @@ -0,0 +1,108 @@ +import Foundation +import Testing +@testable import Core + +/// Writes an executable shell script fixture to a temp file and returns its +/// URL. Mirrors `ServerProcessServiceTests`'/`ServerListViewModelProcessTests`' +/// fixture-script pattern -- duplicated here rather than shared because the +/// originals are file-private to their respective test files. +private func makeScriptFixture(_ contents: String) throws -> URL { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("mc-vector-log-vm-fixture-\(UUID().uuidString)", isDirectory: false) + .appendingPathExtension("sh") + try Data(contents.utf8).write(to: url) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: url.path) + return url +} + +private func makeServer(id: String = "srv-1", javaPath: String) -> Server { + Server( + id: id, + name: "Test Server", + version: "1.21.1", + software: "paper", + port: 25565, + memory: 512, + path: FileManager.default.temporaryDirectory.path, + status: .offline, + javaPath: javaPath, + ) +} + +/// Echoes five known lines to stdout, then exits cleanly. `streamLogs()` +/// completing (its `for await` reaching EOF) is what makes these tests +/// deterministic rather than timing-based: every assertion below runs only +/// after the script has already exited and the view model's final flush +/// has already happened, so there's no flush-interval race to wait out. +private let echoesFiveLinesScript = """ +#!/bin/sh +echo "one" +echo "two" +echo "three" +echo "four" +echo "five" +exit 0 +""" + +@MainActor +@Test("streamLogs() populates lines with every line the process wrote to stdout, in order") +func streamLogsPopulatesLinesInOrder() async throws { + let service = ServerProcessService() + let scriptURL = try makeScriptFixture(echoesFiveLinesScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let server = makeServer(javaPath: scriptURL.path) + try await service.start(server: server) + + let viewModel = ServerLogViewModel(serverId: server.id, processService: service) + await viewModel.streamLogs() + + #expect(viewModel.lines.map(\.text) == ["one", "two", "three", "four", "five"]) +} + +@MainActor +@Test("streamLogs() trims down to retainedLineCount, keeping the newest lines, once overshoot is exceeded") +func streamLogsAppliesLogLineBufferTrimming() async throws { + let service = ServerProcessService() + let scriptURL = try makeScriptFixture(echoesFiveLinesScript) + defer { try? FileManager.default.removeItem(at: scriptURL) } + + let server = makeServer(javaPath: scriptURL.path) + try await service.start(server: server) + + // A small buffer (well under the 5 lines the fixture writes) exercises + // `LogLineBuffer`'s hysteresis trim end-to-end through this view + // model's batching/flush glue -- not `LogLineBuffer` itself (already + // covered by `LogLineBufferTests`), but that this view model actually + // routes every arriving line through `buffer.append` (one call per + // line, matching `LogLineBufferTests`' own trim semantics) rather + // than, say, only applying the last flushed batch. + // + // With `retainedLineCount: 2, trimOvershoot: 1` (trim threshold: 3), + // appending "one".."four" crosses the threshold on "four" (count 4 > 3) + // and trims to the newest 2 (["three", "four"]); appending "five" then + // brings the count back to 3, which is at, not over, the threshold, so + // no further trim fires -- this is the same hysteresis behavior + // `LogLineBufferTests` exercises directly, just reached here via real + // stdout lines instead of synthetic ones. + let viewModel = ServerLogViewModel( + serverId: server.id, + processService: service, + retainedLineCount: 2, + trimOvershoot: 1, + ) + await viewModel.streamLogs() + + #expect(viewModel.lines.map(\.text) == ["three", "four", "five"]) +} + +@MainActor +@Test("streamLogs() returns without changing lines when the server has no tracked running process") +func streamLogsIsNoOpWhenServerIsNotRunning() async { + let service = ServerProcessService() + let viewModel = ServerLogViewModel(serverId: "srv-never-started", processService: service) + + await viewModel.streamLogs() + + #expect(viewModel.lines.isEmpty) +} diff --git a/apps/native-macos/Tests/CoreTests/ServerProcessServiceTests.swift b/apps/native-macos/Tests/CoreTests/ServerProcessServiceTests.swift new file mode 100644 index 0000000..795b089 --- /dev/null +++ b/apps/native-macos/Tests/CoreTests/ServerProcessServiceTests.swift @@ -0,0 +1,397 @@ +import Foundation +import Testing +@testable import Core + +/// Writes an executable shell script fixture to a temp file and returns its +/// URL. Following `JavaLaunchHarnessTests`' precedent of exercising real +/// system executables (`/bin/echo`, `/usr/bin/false`) rather than mocking +/// `Process` -- here that means real, tiny, purpose-built shell scripts +/// standing in for a long-running Minecraft server process, since none of +/// the system executables used by that precedent stay alive to read stdin. +/// +/// Each fixture is used as the `javaPath` itself (not invoked via `sh +///