diff --git a/Foreman/Foreman/AGENTS.md b/Foreman/Foreman/AGENTS.md index adebb0e5..cc895ff1 100644 --- a/Foreman/Foreman/AGENTS.md +++ b/Foreman/Foreman/AGENTS.md @@ -40,8 +40,10 @@ formatting, global conventions) and ForemanCore's - **Hiding the window does not cancel `.task`** — an ordered-out `NSWindow` keeps its SwiftUI hierarchy alive and loops keep ticking (verified empirically). Any periodic work in this window must gate on - `WindowVisibilityReader` (see `WorkerLogView`), and view drafts re-seed on - visibility (see `SettingsView`). + `WindowVisibilityReader` (see `WorkerLogView`), and the settings General + pane re-reads the login-item status on visibility (see `SettingsView`). The + settings path fields are edited in an explicit Save/Cancel sheet, so there + is no commit-on-blur to lose when switching panes. - The target is an `LSUIElement` with a hand-written Info.plist in [`Project.swift`](../../Project.swift) — don't switch to `.extendingDefault`, which injects `NSMainStoryboardFile` on macOS. diff --git a/Foreman/Foreman/README.md b/Foreman/Foreman/README.md index 86ba5185..1fad07f5 100644 --- a/Foreman/Foreman/README.md +++ b/Foreman/Foreman/README.md @@ -43,14 +43,28 @@ closed. Closing the window just hides it — the app keeps running until Quit. verbose startup logs. Editable while the worker is stopped — options apply on the next start. - **Toolbar** — a "Preventing sleep" badge while the sleep assertion is held, - Rescan, Settings (opens the standard settings window: the scan directory - and an explicit `cursor-agent` path, empty = auto-detect, applied as - fields commit), and Quit. + Rescan, Settings (opens the settings window — see below), and Quit. + +## Settings + +The settings window is a macOS System-Settings-style sidebar with three panes. +The *Launch at login* toggle applies immediately; the path settings open a +small editor sheet that commits only on **Save** (Cancel or Escape discards). + +- **General** — *Launch Foreman at login*. Registers Foreman as a login item + via `SMAppService`, so it starts (and restores your enabled workers) when + you log in. If macOS needs you to approve the item first, the pane says so + and links straight to System Settings; a failed toggle shows its error here. +- **Repositories** — the directory scanned for git repositories (empty = + `~/Development`), edited via a sheet with a folder picker. +- **Agent** — an explicit `cursor-agent` executable path (empty = auto-detect), + edited via a sheet. ## Lifecycle - On launch, Foreman restores the saved configuration and restarts the workers - that were enabled last time. + that were enabled last time. With *Launch Foreman at login* on, this happens + automatically after you log in. - Quitting stops every worker (stop-on-quit: the app owns its processes and never leaves orphans). - The repo list refreshes every time the window is opened or focused, and on diff --git a/Foreman/Foreman/Sources/ForemanSession.swift b/Foreman/Foreman/Sources/ForemanSession.swift index aaebcd85..f2e069ce 100644 --- a/Foreman/Foreman/Sources/ForemanSession.swift +++ b/Foreman/Foreman/Sources/ForemanSession.swift @@ -37,6 +37,24 @@ final class ForemanSession { services.isAnyWorkerLive } + /// Whether Foreman launches at login. `SettingsView` binds this two-way; + /// the setter registers/unregisters the login item in Core (which logs and + /// surfaces any failure on `loginItemError`). + var startsAtLogin: Bool { + get { services.startsAtLogin } + set { services.startsAtLogin = newValue } + } + + /// The login item is registered but awaiting approval in System Settings. + var loginItemNeedsApproval: Bool { + services.loginItemNeedsApproval + } + + /// The most recent login-item failure (shown in the General settings pane). + var loginItemError: String? { + services.loginItemError + } + init(services: ForemanServices) { self.services = services } @@ -63,4 +81,16 @@ final class ForemanSession { func rescan() { services.rescan() } + + /// Re-reads the login-item status from the OS (it can change in System + /// Settings while Foreman runs). + func refreshLoginItemStatus() { + services.refreshLoginItemStatus() + } + + /// Opens System Settings › General › Login Items (to approve a pending + /// login item). + func openSystemSettingsLoginItems() { + services.openSystemSettingsLoginItems() + } } diff --git a/Foreman/Foreman/Sources/MainWindowView.swift b/Foreman/Foreman/Sources/MainWindowView.swift index 148f3d05..d8f981bc 100644 --- a/Foreman/Foreman/Sources/MainWindowView.swift +++ b/Foreman/Foreman/Sources/MainWindowView.swift @@ -89,7 +89,7 @@ struct MainWindowView: View { SettingsLink { Label("Settings", systemImage: "gearshape") } - .help("Change the scan directory or the cursor-agent executable.") + .help("Open Foreman's settings.") } ToolbarItem { Button { diff --git a/Foreman/Foreman/Sources/SettingsView.swift b/Foreman/Foreman/Sources/SettingsView.swift index add945c1..dc87d64b 100644 --- a/Foreman/Foreman/Sources/SettingsView.swift +++ b/Foreman/Foreman/Sources/SettingsView.swift @@ -2,121 +2,312 @@ import AppKit import ForemanCore import SwiftUI -/// Global settings: where to look for repositories and which `cursor-agent` -/// to run. Hosted by the `Settings` scene (app menu / Cmd-, / the toolbar's -/// `SettingsLink`), so it behaves like a standard macOS settings window: -/// edits apply on field commit (Return / focus change / panel selection) — -/// no Save button. Values write through the observable `AppSettings`, which -/// persists and rescans in Core. -struct SettingsView: View { - private enum Field { - case scanDirectory - case agentExecutable +/// A pane in the settings sidebar: static identity/metadata plus its own +/// content view, built from the session. Conformers are ordinary `View`s; +/// `SettingsView` discovers their title/icon and builds them without a central +/// enum or switch. +private protocol SettingsPane: View { + /// Sidebar label and navigation title. + static var title: String { get } + /// Sidebar SF Symbol. + static var icon: String { get } + + init(session: ForemanSession) +} + +/// Type-erased sidebar entry: a pane's metadata plus a builder for its content. +/// Keyed by the pane type's `ObjectIdentifier`, so the sidebar selection stays +/// a typed token rather than a stringly value. +private struct SettingsPaneItem: Identifiable { + let id: ObjectIdentifier + let title: String + let icon: String + let makeView: (ForemanSession) -> AnyView + + init(_ pane: (some SettingsPane).Type) { + id = ObjectIdentifier(pane) + title = pane.title + icon = pane.icon + makeView = { AnyView(pane.init(session: $0)) } } +} +/// Global settings, laid out like a modern macOS System Settings window: a +/// `NavigationSplitView` sidebar over the registered ``SettingsPaneItem``s. +/// +/// Hosted by the `Settings` scene (app menu / Cmd-, / the toolbar's +/// `SettingsLink`), so it follows settings-window conventions: the toggle in +/// General applies immediately, and the path panes commit through an explicit +/// Save/Cancel sheet. Values write through the observable Core (`AppSettings` +/// for the paths, `LoginItemController` for the login item), which persists. +struct SettingsView: View { let session: ForemanSession - @State private var scanDirectory: String - @State private var agentExecutable: String + private let panes: [SettingsPaneItem] = [ + SettingsPaneItem(GeneralSettingsPane.self), + SettingsPaneItem(RepositoriesSettingsPane.self), + SettingsPaneItem(AgentSettingsPane.self), + ] + + @State private var selection: ObjectIdentifier? + + var body: some View { + // The sidebar is always open: pin the visibility and drop the default + // sidebar-toggle button so the panes can't be collapsed away. + NavigationSplitView(columnVisibility: .constant(.all)) { + List(panes, selection: $selection) { pane in + Label(pane.title, systemImage: pane.icon) + } + .navigationSplitViewColumnWidth(min: 170, ideal: 190, max: 220) + .toolbar(removing: .sidebarToggle) + } detail: { + detail + } + .frame(minWidth: 420, minHeight: 380) + .onAppear { + if selection == nil { selection = panes.first?.id } + } + } + + /// The selected pane's content, falling back to the first pane so the + /// detail always shows something meaningful. + private var detail: AnyView { + let pane = panes.first { $0.id == selection } ?? panes.first + return pane?.makeView(session) ?? AnyView(EmptyView()) + } +} + +/// General preferences: whether Foreman launches at login. +private struct GeneralSettingsPane: SettingsPane { + static let title = "General" + static let icon = "gearshape" + + @Bindable var session: ForemanSession + @State private var isWindowVisible = true - @FocusState private var focusedField: Field? init(session: ForemanSession) { - self.session = session - _scanDirectory = State( - initialValue: session.settings.scanDirectory?.path ?? "", - ) - _agentExecutable = State( - initialValue: session.settings.agentExecutable?.path ?? "", - ) + _session = Bindable(session) } var body: some View { Form { - LabeledContent("Scan directory") { - HStack(spacing: 4) { - TextField( - "Scan directory", - text: $scanDirectory, - prompt: Text("~/Development"), + Toggle("Launch Foreman at login", isOn: $session.startsAtLogin) + Text( + "Foreman lives in the menu bar. Turn this on to start it automatically when you log in — it will restore any enabled workers.", + ) + .font(.caption) + .foregroundStyle(.secondary) + + // Registered but macOS is waiting for the user to approve it. + if session.loginItemNeedsApproval { + LabeledContent { + Button("Open Login Items…") { + session.openSystemSettingsLoginItems() + } + } label: { + Label( + "Approve Foreman in System Settings to finish enabling this.", + systemImage: "exclamationmark.triangle.fill", ) - .labelsHidden() - .focused($focusedField, equals: .scanDirectory) - .onSubmit { applyScanDirectory() } - Button("Choose…") { chooseScanDirectory() } + .foregroundStyle(.orange) } } - LabeledContent("cursor-agent") { - VStack(alignment: .leading, spacing: 4) { - TextField( - "cursor-agent executable", - text: $agentExecutable, - prompt: Text("Auto-detect"), - ) - .labelsHidden() - .focused($focusedField, equals: .agentExecutable) - .onSubmit { applyAgentExecutable() } - Text( - "Leave empty to search: \(CursorAgentLocator.defaultSearchPaths.joined(separator: ", "))", - ) - .font(.caption2) - .foregroundStyle(.tertiary) - } + // A failed register/unregister — shown here (not the main-window + // banner) because the toggle lives in this window. + if let error = session.loginItemError { + Label(error, systemImage: "xmark.octagon.fill") + .font(.callout) + .foregroundStyle(.red) } } .formStyle(.grouped) - .frame(width: 480) - // Tabbing or clicking out of a field commits it, like Return does. - .onChange(of: focusedField) { previous, _ in - switch previous { - case .scanDirectory: applyScanDirectory() - case .agentExecutable: applyAgentExecutable() - case nil: break - } - } + .navigationTitle(Self.title) + // The OS owns the login-item state and the user can change it in + // System Settings, so re-read it whenever this pane comes back on + // screen. The Settings window keeps its hierarchy across opens. + .onAppear { session.refreshLoginItemStatus() } .background(WindowVisibilityReader(isVisible: $isWindowVisible)) - // The Settings window keeps its hierarchy across opens, so init-time - // drafts go stale; re-seed them whenever the window comes back. .onChange(of: isWindowVisible) { _, visible in - if visible { reseedDrafts() } + if visible { session.refreshLoginItemStatus() } + } + } +} + +/// Repository discovery: where to scan for git repositories. The value is +/// read-only here; editing happens in an explicit-commit sheet so switching +/// panes can't drop a half-typed path. +private struct RepositoriesSettingsPane: SettingsPane { + static let title = "Repositories" + static let icon = "folder" + + let session: ForemanSession + + @State private var isEditing = false + + init(session: ForemanSession) { + self.session = session + } + + var body: some View { + Form { + LabeledContent("Scan directory") { + Text(session.settings.scanDirectory?.path ?? "~/Development (default)") + .foregroundStyle(.secondary) + .textSelection(.enabled) + } + Button("Change…") { isEditing = true } + Text("Foreman lists the git repositories directly inside this directory.") + .font(.caption) + .foregroundStyle(.secondary) } + .formStyle(.grouped) + .navigationTitle(Self.title) + .sheet(isPresented: $isEditing) { + PathEditorSheet( + title: "Scan directory", + prompt: "~/Development", + caption: "Leave empty to use the default (~/Development).", + directoryPickerStart: session.settings.resolvedScanDirectory, + initialValue: session.settings.scanDirectory?.path ?? "", + onSave: { session.settings.scanDirectory = $0 }, + ) + } + } +} + +/// Agent settings: which `cursor-agent` executable to run. Read-only here; +/// edited via an explicit-commit sheet (see `RepositoriesSettingsPane`). +private struct AgentSettingsPane: SettingsPane { + static let title = "Agent" + static let icon = "terminal" + + let session: ForemanSession + + @State private var isEditing = false + + init(session: ForemanSession) { + self.session = session } - private func reseedDrafts() { - scanDirectory = session.settings.scanDirectory?.path ?? "" - agentExecutable = session.settings.agentExecutable?.path ?? "" + var body: some View { + Form { + LabeledContent("cursor-agent") { + Text(session.settings.agentExecutable?.path ?? "Auto-detect") + .foregroundStyle(.secondary) + .textSelection(.enabled) + } + Button("Change…") { isEditing = true } + Text( + "Leave empty to search: \(CursorAgentLocator.defaultSearchPaths.joined(separator: ", "))", + ) + .font(.caption2) + .foregroundStyle(.tertiary) + } + .formStyle(.grouped) + .navigationTitle(Self.title) + .sheet(isPresented: $isEditing) { + PathEditorSheet( + title: "cursor-agent executable", + prompt: "Auto-detect", + caption: "Leave empty to auto-detect from the known install locations.", + directoryPickerStart: nil, + initialValue: session.settings.agentExecutable?.path ?? "", + onSave: { session.settings.agentExecutable = $0 }, + ) + } } +} + +/// A modal editor for a single optional path setting. The draft only commits +/// when the user taps Save (Cancel/Escape discards it), so there is no +/// commit-on-blur and no way to lose a half-typed value by navigating away. +/// Empty commits as `nil` (the setting's default). +private struct PathEditorSheet: View { + let title: String + let prompt: String + let caption: String + /// When non-nil, shows a "Choose…" folder picker starting at this URL. + let directoryPickerStart: URL? + let initialValue: String + let onSave: (URL?) -> Void - private func applyScanDirectory() { - session.settings.scanDirectory = parseURL(from: scanDirectory) + @Environment(\.dismiss) private var dismiss + @State private var draft: String + @FocusState private var isFocused: Bool + + init( + title: String, + prompt: String, + caption: String, + directoryPickerStart: URL?, + initialValue: String, + onSave: @escaping (URL?) -> Void, + ) { + self.title = title + self.prompt = prompt + self.caption = caption + self.directoryPickerStart = directoryPickerStart + self.initialValue = initialValue + self.onSave = onSave + _draft = State(initialValue: initialValue) } - private func applyAgentExecutable() { - session.settings.agentExecutable = parseURL(from: agentExecutable) + var body: some View { + VStack(alignment: .leading, spacing: 16) { + Text(title) + .font(.headline) + HStack(spacing: 8) { + TextField(title, text: $draft, prompt: Text(prompt)) + .labelsHidden() + .focused($isFocused) + .onSubmit { save() } + if directoryPickerStart != nil { + Button("Choose…") { choose() } + } + } + Text(caption) + .font(.caption) + .foregroundStyle(.secondary) + HStack { + Spacer() + Button("Cancel", role: .cancel) { dismiss() } + .keyboardShortcut(.cancelAction) + Button("Save") { save() } + .keyboardShortcut(.defaultAction) + } + } + .padding(20) + .frame(width: 460) + .onAppear { isFocused = true } } - /// Empty means "use the default" (`nil`); otherwise a tilde-expanded - /// file URL. - private func parseURL(from text: String) -> URL? { - let trimmed = text.trimmingCharacters(in: .whitespaces) - guard !trimmed.isEmpty else { return nil } - return URL(fileURLWithPath: (trimmed as NSString).expandingTildeInPath) + private func save() { + onSave(parseURL(from: draft)) + dismiss() } - private func chooseScanDirectory() { + private func choose() { + guard let directoryPickerStart else { return } let panel = NSOpenPanel() panel.canChooseDirectories = true panel.canChooseFiles = false panel.allowsMultipleSelection = false - panel.directoryURL = session.settings.resolvedScanDirectory + panel.directoryURL = directoryPickerStart if panel.runModal() == .OK, let url = panel.url { - scanDirectory = url.path - applyScanDirectory() + draft = url.path } } } +/// Empty means "use the default" (`nil`); otherwise a tilde-expanded file URL. +private func parseURL(from text: String) -> URL? { + let trimmed = text.trimmingCharacters(in: .whitespaces) + guard !trimmed.isEmpty else { return nil } + return URL(fileURLWithPath: (trimmed as NSString).expandingTildeInPath) +} + #if DEBUG #Preview { SettingsView(session: PreviewSupport.emptySession()) diff --git a/Foreman/ForemanCore/AGENTS.md b/Foreman/ForemanCore/AGENTS.md index 784da552..85de2789 100644 --- a/Foreman/ForemanCore/AGENTS.md +++ b/Foreman/ForemanCore/AGENTS.md @@ -11,7 +11,8 @@ and per-type detail. ForemanServices ── AppSettings ├────────── RepoDiscovery ── [Repo] ── Worker ├────────── WorkerConfigStore (ForemanConfiguration JSON) - └────────── SleepInhibitor + ├────────── SleepInhibitor + └────────── LoginItemController (SMAppService) ``` This file complements the root [`AGENTS.md`](../../AGENTS.md), which owns the @@ -49,6 +50,16 @@ build system, formatting, and global conventions. Read that first. start. - **The sleep assertion tracks liveness, not toggles** — recomputed on every worker transition, held while any worker (draining included) is live. +- **The login item is OS-owned, not persisted config.** `LoginItemController` + reads/writes `SMAppService.mainApp` (behind an `@_spi(Testing)` backend) as a + typed `LoginItemStatus`; `requiresApproval` counts as *on* (registered but + pending), so it never reads as off. `ForemanServices.startsAtLogin` is a live + read of that status, and its setter surfaces failures on the dedicated + `loginItemError` (the toggle lives in the settings window, not the main + banner) while keeping the observed value honest. Never mirror it into + `ForemanConfiguration` — the system is the source of truth. Launch-at-login + just relaunches the app, which reuses the existing `start()` restore of + enabled workers. - **Absence vs failure.** Missing options read as `WorkerOptions.standard` and a missing config file is `.initial`; an unreadable/undecodable file throws. **CLI defaults are deferred to, not duplicated** — omit a flag diff --git a/Foreman/ForemanCore/README.md b/Foreman/ForemanCore/README.md index 088f6c79..e85ff9a5 100644 --- a/Foreman/ForemanCore/README.md +++ b/Foreman/ForemanCore/README.md @@ -53,7 +53,12 @@ let argv = repo.options.arguments(workerDirectory: repo.rootURL) path. Funnels every persisted mutation (repo toggles/options, settings) into the saved JSON, recomputes the sleep assertion on every worker transition, and surfaces tree-level problems (unreadable config, failed - scan, failed save) on the observable `issueMessage`. + scan, failed save) on the observable `issueMessage`. `startsAtLogin` is a + two-way property over the login item (see `LoginItemController`), with + `loginItemNeedsApproval` and a dedicated `loginItemError` (login failures + surface here, not on the shared `issueMessage`); `refreshLoginItemStatus()` + re-reads it from the OS and `openSystemSettingsLoginItems()` jumps to the + approval UI. - **`Repo` / `RepoID`** — one discovered repository as an `@Observable` object: identity (`name`, `rootURL`, typed `RepoID` = canonical absolute path), the persisted intent (`isEnabled`, `options` — mutations start/stop @@ -107,6 +112,17 @@ let argv = repo.options.arguments(workerDirectory: repo.rootURL) equivalent), so the machine won't doze off mid-agent-run. Display sleep and an explicit lid close are unaffected. The observable `isActive` backs the app's "preventing sleep" indicator. +- **`LoginItemController`** — reflects and toggles whether Foreman launches at + login, wrapping `SMAppService.mainApp` (no helper bundle or entitlement + needed for the main app). The OS owns the real state, so the observable + `status` (a `LoginItemStatus`: `enabled` / `requiresApproval` / + `notRegistered`) is read from the service and `refresh()` re-reads it (the + user can change it in System Settings). `isEnabled` treats a pending + (`requiresApproval`) item as on — not off — with `needsApproval` flagging + that the user still has to confirm it (`openSystemSettingsLoginItems()` + jumps there). `setEnabled(_:)` registers/unregisters and re-syncs so a + failed attempt never reads as falsely on. The real service sits behind an + `@_spi(Testing)` `LoginItemBackend` so tests inject a double. - **`CursorAgentLocator`** — resolves the `cursor-agent` executable. GUI apps don't inherit the shell `PATH`, so it checks the CLI's known install locations (`~/.local/bin`, `/usr/local/bin`, `/opt/homebrew/bin`); an diff --git a/Foreman/ForemanCore/Sources/ForemanServices.swift b/Foreman/ForemanCore/Sources/ForemanServices.swift index b3233541..87436154 100644 --- a/Foreman/ForemanCore/Sources/ForemanServices.swift +++ b/Foreman/ForemanCore/Sources/ForemanServices.swift @@ -55,6 +55,51 @@ public final class ForemanServices { sleepInhibitor.isActive } + /// Whether Foreman is registered to launch at login. Backed by + /// `SMAppService` (the OS owns the real state), so this is a live read of + /// the login-item status. The setter registers/unregisters and, on + /// failure, logs *and* surfaces ``loginItemError`` while leaving the + /// observed value honest — a failed toggle stays off rather than falsely + /// on. A successful toggle clears the error. + public var startsAtLogin: Bool { + get { loginItem.isEnabled } + set { + do { + try loginItem.setEnabled(newValue) + loginItemError = nil + } catch { + Self.logger.error("Couldn't update the login item: \(error)") + loginItemError = newValue + ? "Couldn't turn on “Launch at login”: \(error.localizedDescription)" + : "Couldn't turn off “Launch at login”: \(error.localizedDescription)" + } + } + } + + /// The login item is registered but macOS is waiting for the user to + /// approve it in System Settings before it will launch. + public var loginItemNeedsApproval: Bool { + loginItem.needsApproval + } + + /// The most recent login-item failure, surfaced in the settings window's + /// General pane (not the main-window banner — the toggle lives here). + /// Cleared by the next successful toggle. + public private(set) var loginItemError: String? + + /// Re-reads the login-item status from the OS; it can change outside the + /// app (System Settings › General › Login Items), so the UI refreshes it + /// when the settings window reappears. + public func refreshLoginItemStatus() { + loginItem.refresh() + } + + /// Opens System Settings › General › Login Items so the user can approve a + /// pending login item. + public func openSystemSettingsLoginItems() { + loginItem.openSystemSettingsLoginItems() + } + /// Where worker logs land: `~/Library/Logs/Foreman`. public static var defaultLogDirectory: URL { FileManager.default.homeDirectoryForCurrentUser @@ -70,6 +115,7 @@ public final class ForemanServices { private let configStore: WorkerConfigStore private let logDirectory: URL private let sleepInhibitor: SleepInhibitor + private let loginItem: LoginItemController private let locator = CursorAgentLocator() public convenience init(configStore: WorkerConfigStore, logDirectory: URL) { @@ -77,6 +123,7 @@ public final class ForemanServices { configStore: configStore, logDirectory: logDirectory, sleepInhibitor: SleepInhibitor(), + loginItem: LoginItemController(), ) } @@ -89,10 +136,12 @@ public final class ForemanServices { configStore: WorkerConfigStore, logDirectory: URL, sleepInhibitor: SleepInhibitor, + loginItem: LoginItemController, ) { self.configStore = configStore self.logDirectory = logDirectory self.sleepInhibitor = sleepInhibitor + self.loginItem = loginItem do { configuration = try configStore.load() configLoadFailure = nil diff --git a/Foreman/ForemanCore/Sources/LoginItemController.swift b/Foreman/ForemanCore/Sources/LoginItemController.swift new file mode 100644 index 00000000..eb604415 --- /dev/null +++ b/Foreman/ForemanCore/Sources/LoginItemController.swift @@ -0,0 +1,126 @@ +import Foundation +import Observation +import ServiceManagement + +/// The login-item state Foreman cares about, distilled from +/// `SMAppService.Status`. +/// +/// `requiresApproval` is distinct from `notRegistered`: the app *is* registered +/// but macOS is waiting for the user to approve it in System Settings before it +/// will actually launch. Collapsing it into "off" would misreport an enabled +/// (pending) item as disabled. +public enum LoginItemStatus: Sendable, Equatable { + case enabled + case requiresApproval + case notRegistered +} + +/// Registers, unregisters, and reports the app's login-item state behind +/// ``LoginItemController``. Production uses the `SMAppService.mainApp`-backed +/// implementation; tests conform a fake so no real login item is touched. +@MainActor +@_spi(Testing) +public protocol LoginItemBackend: AnyObject { + var status: LoginItemStatus { get } + func register() throws + func unregister() throws + /// Opens System Settings › General › Login Items so the user can approve + /// or inspect the item. + func openSystemSettingsLoginItems() +} + +/// The real backend: `SMAppService.mainApp`. Registering the *main app* as a +/// login item needs no helper bundle and no special entitlement. +@MainActor +final class MainAppLoginItemBackend: LoginItemBackend { + var status: LoginItemStatus { + switch SMAppService.mainApp.status { + case .enabled: .enabled + case .requiresApproval: .requiresApproval + // `.notFound` means the service isn't registered from this bundle; + // for the toggle that reads the same as "not registered". + case .notRegistered, .notFound: .notRegistered + @unknown default: .notRegistered + } + } + + func register() throws { + try SMAppService.mainApp.register() + } + + func unregister() throws { + try SMAppService.mainApp.unregister() + } + + func openSystemSettingsLoginItems() { + SMAppService.openSystemSettingsLoginItems() + } +} + +/// Reflects and controls whether Foreman launches at login, wrapping +/// `SMAppService.mainApp`. +/// +/// The OS owns the real state, so ``status`` is read from the backend (and +/// re-read via ``refresh()``, since the user can change it in System Settings +/// while the app runs). ``setEnabled(_:)`` registers or unregisters and then +/// re-syncs ``status`` from the backend so the observed value never lies — a +/// failed registration leaves the toggle honestly off, not falsely on. +@MainActor +@Observable +public final class LoginItemController { + /// The current login-item status. Observable so the settings UI reflects + /// it (including the pending-approval state). + public private(set) var status: LoginItemStatus + + @ObservationIgnored private let backend: any LoginItemBackend + + /// Whether the login item is on. Includes ``LoginItemStatus/requiresApproval``: + /// the user asked for it and it's registered — it just needs a nod in + /// System Settings — so the toggle should read on, not off. + public var isEnabled: Bool { + status != .notRegistered + } + + /// The login item is registered but macOS needs the user to approve it in + /// System Settings before it will actually launch. + public var needsApproval: Bool { + status == .requiresApproval + } + + public init() { + backend = MainAppLoginItemBackend() + status = backend.status + } + + /// Swaps the real login-item backend for a test double. + @_spi(Testing) + public init(backend: any LoginItemBackend) { + self.backend = backend + status = backend.status + } + + /// Re-reads the OS status; it can change outside the app (System Settings + /// › General › Login Items). + public func refresh() { + status = backend.status + } + + /// Registers or unregisters the login item, then re-syncs ``status`` from + /// the backend. Rethrows the underlying `SMAppService` error; the observed + /// value stays honest whether it succeeds or throws. + public func setEnabled(_ enabled: Bool) throws { + defer { status = backend.status } + guard enabled != isEnabled else { return } + if enabled { + try backend.register() + } else { + try backend.unregister() + } + } + + /// Opens System Settings › General › Login Items (used to approve a + /// pending item). + public func openSystemSettingsLoginItems() { + backend.openSystemSettingsLoginItems() + } +} diff --git a/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift b/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift index 6896be88..96d21d59 100644 --- a/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift +++ b/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift @@ -18,6 +18,45 @@ final class SleepAssertionRecorder: SleepAssertionBackend { } } +/// A `LoginItemBackend` that records register/unregister/open calls in memory +/// and can be told to fail, so login-item wiring is testable without touching +/// the real `SMAppService`. +@MainActor +final class LoginItemRecorder: LoginItemBackend { + private(set) var status: LoginItemStatus + private(set) var registerCount = 0 + private(set) var unregisterCount = 0 + private(set) var openCount = 0 + + /// When set, both `register()` and `unregister()` throw it (and leave + /// `status` unchanged), simulating an `SMAppService` failure. + var failure: (any Error)? + + init(status: LoginItemStatus = .notRegistered, failure: (any Error)? = nil) { + self.status = status + self.failure = failure + } + + func register() throws { + registerCount += 1 + if let failure { throw failure } + status = .enabled + } + + func unregister() throws { + unregisterCount += 1 + if let failure { throw failure } + status = .notRegistered + } + + func openSystemSettingsLoginItems() { + openCount += 1 + } +} + +/// A stand-in error for login-item failure injection. +struct LoginItemTestError: Error {} + /// Builds a live `Repo` over a fixed executable for tree-level tests: /// disabled, standard options, no-op persistence and state-change hooks. @MainActor diff --git a/Foreman/ForemanCore/Tests/ForemanServicesTests.swift b/Foreman/ForemanCore/Tests/ForemanServicesTests.swift index 837cd2f9..77c9d3f5 100644 --- a/Foreman/ForemanCore/Tests/ForemanServicesTests.swift +++ b/Foreman/ForemanCore/Tests/ForemanServicesTests.swift @@ -11,14 +11,17 @@ struct ForemanServicesTests { let scanDirectory: URL let executable: URL let sleep: SleepAssertionRecorder + let login: LoginItemRecorder } /// A config-store + scan-directory sandbox. `repoNames` become git repos /// in the scan directory; `configure` edits the saved configuration /// (which already points `agentExecutable` at a long-running stub) and - /// receives the scan directory for building `RepoID`s. + /// receives the scan directory for building `RepoID`s. `loginBackend` + /// injects a login-item double (defaults to a fresh one that succeeds). private func makeFixture( repoNames: [String], + loginBackend: LoginItemRecorder? = nil, configure: (inout ForemanConfiguration, _ scanDirectory: URL) -> Void = { _, _ in }, ) throws -> Fixture { let base = try makeTemporaryDirectory() @@ -46,10 +49,12 @@ struct ForemanServicesTests { try store.save(configuration) let recorder = SleepAssertionRecorder() + let login = loginBackend ?? LoginItemRecorder() let services = ForemanServices( configStore: store, logDirectory: base.appendingPathComponent("logs"), sleepInhibitor: SleepInhibitor(backend: recorder), + loginItem: LoginItemController(backend: login), ) return Fixture( services: services, @@ -58,6 +63,7 @@ struct ForemanServicesTests { scanDirectory: scanDirectory, executable: executable, sleep: recorder, + login: login, ) } @@ -114,6 +120,7 @@ struct ForemanServicesTests { configStore: WorkerConfigStore(directory: configDirectory), logDirectory: base.appendingPathComponent("logs"), sleepInhibitor: SleepInhibitor(backend: SleepAssertionRecorder()), + loginItem: LoginItemController(backend: LoginItemRecorder()), ) services.start() @@ -179,6 +186,63 @@ struct ForemanServicesTests { #expect(fixture.services.repos.map(\.name) == ["New"]) } + // MARK: - Launch at login + + @Test func startsAtLoginWritesThroughToTheLoginItem() throws { + let fixture = try makeFixture(repoNames: []) + #expect(!fixture.services.startsAtLogin) + + fixture.services.startsAtLogin = true + #expect(fixture.services.startsAtLogin) + #expect(fixture.login.registerCount == 1) + + fixture.services.startsAtLogin = false + #expect(!fixture.services.startsAtLogin) + #expect(fixture.login.unregisterCount == 1) + } + + @Test func aFailedLoginItemToggleSurfacesTheErrorAndStaysHonest() throws { + let fixture = try makeFixture( + repoNames: [], + loginBackend: LoginItemRecorder(failure: LoginItemTestError()), + ) + fixture.services.start() + #expect(fixture.services.loginItemError == nil) + + fixture.services.startsAtLogin = true + + // The registration failed, so the toggle stays off and the failure is + // observable on the login-item channel (not the tree-level banner) + // rather than silently swallowed. + #expect(!fixture.services.startsAtLogin) + #expect(fixture.services.loginItemError != nil) + #expect(fixture.services.issueMessage == nil) + } + + @Test func aSuccessfulToggleClearsAPriorLoginItemError() throws { + let recorder = LoginItemRecorder(failure: LoginItemTestError()) + let fixture = try makeFixture(repoNames: [], loginBackend: recorder) + + fixture.services.startsAtLogin = true + #expect(fixture.services.loginItemError != nil) + + // Recover: the next toggle succeeds and the stale error clears. + recorder.failure = nil + fixture.services.startsAtLogin = true + #expect(fixture.services.startsAtLogin) + #expect(fixture.services.loginItemError == nil) + } + + @Test func pendingApprovalReadsAsEnabledAndNeedsApproval() throws { + let fixture = try makeFixture( + repoNames: [], + loginBackend: LoginItemRecorder(status: .requiresApproval), + ) + + #expect(fixture.services.startsAtLogin) + #expect(fixture.services.loginItemNeedsApproval) + } + // MARK: - Rescan @Test func rescanPrunesStaleEntriesButKeepsForeignScanDirectoryOnes() throws { diff --git a/Foreman/ForemanCore/Tests/LoginItemControllerTests.swift b/Foreman/ForemanCore/Tests/LoginItemControllerTests.swift new file mode 100644 index 00000000..516043ae --- /dev/null +++ b/Foreman/ForemanCore/Tests/LoginItemControllerTests.swift @@ -0,0 +1,84 @@ +@_spi(Testing) import ForemanCore +import Testing + +@MainActor +struct LoginItemControllerTests { + @Test func seedsStateFromTheBackend() { + let off = LoginItemController(backend: LoginItemRecorder(status: .notRegistered)) + #expect(!off.isEnabled) + #expect(!off.needsApproval) + + let on = LoginItemController(backend: LoginItemRecorder(status: .enabled)) + #expect(on.isEnabled) + #expect(!on.needsApproval) + } + + @Test func requiresApprovalReadsAsEnabledButPending() { + let controller = LoginItemController(backend: LoginItemRecorder(status: .requiresApproval)) + + // Registered-but-pending is "on" for the toggle — not off — with a + // flag the UI can use to nudge the user toward System Settings. + #expect(controller.isEnabled) + #expect(controller.needsApproval) + } + + @Test func enablingRegistersAndDisablingUnregisters() throws { + let recorder = LoginItemRecorder() + let controller = LoginItemController(backend: recorder) + + try controller.setEnabled(true) + #expect(controller.isEnabled) + #expect(recorder.registerCount == 1) + + // Re-enabling is a no-op: the backend isn't touched again. + try controller.setEnabled(true) + #expect(recorder.registerCount == 1) + + try controller.setEnabled(false) + #expect(!controller.isEnabled) + #expect(recorder.unregisterCount == 1) + } + + @Test func disablingAPendingItemStillUnregisters() throws { + let recorder = LoginItemRecorder(status: .requiresApproval) + let controller = LoginItemController(backend: recorder) + + try controller.setEnabled(false) + #expect(recorder.unregisterCount == 1) + #expect(!controller.isEnabled) + } + + @Test func aFailedRegistrationLeavesTheStateHonestlyOff() { + let recorder = LoginItemRecorder(failure: LoginItemTestError()) + let controller = LoginItemController(backend: recorder) + + #expect(throws: LoginItemTestError.self) { + try controller.setEnabled(true) + } + // The register attempt happened, but it failed — the observed value + // must not read as "on". + #expect(recorder.registerCount == 1) + #expect(!controller.isEnabled) + } + + @Test func refreshPicksUpAnOutOfBandChange() { + let recorder = LoginItemRecorder(status: .notRegistered) + let controller = LoginItemController(backend: recorder) + #expect(!controller.isEnabled) + + // Simulate the user enabling the login item in System Settings. + try? recorder.register() + #expect(!controller.isEnabled) + + controller.refresh() + #expect(controller.isEnabled) + } + + @Test func openSystemSettingsForwardsToTheBackend() { + let recorder = LoginItemRecorder() + let controller = LoginItemController(backend: recorder) + + controller.openSystemSettingsLoginItems() + #expect(recorder.openCount == 1) + } +} diff --git a/Foreman/TODOs.md b/Foreman/TODOs.md index d5fd94dd..ab194ea5 100644 --- a/Foreman/TODOs.md +++ b/Foreman/TODOs.md @@ -29,3 +29,8 @@ ## P2s (Nice to have) +- feat: Redesign the Settings window as a macOS System-Settings-style sidebar + (`NavigationSplitView`) with General / Repositories / Agent panes. +- feat: Add "Launch Foreman at login" (`SMAppService` via `LoginItemController`) + to General settings; launching at login reuses the existing `start()` restore + of enabled workers.