diff --git a/apps/cmd-ime-swift/Sources/CmdIMESwift/Settings/AppSettings.swift b/apps/cmd-ime-swift/Sources/CmdIMESwift/Settings/AppSettings.swift index 06aa7f1..3ffb97f 100644 --- a/apps/cmd-ime-swift/Sources/CmdIMESwift/Settings/AppSettings.swift +++ b/apps/cmd-ime-swift/Sources/CmdIMESwift/Settings/AppSettings.swift @@ -37,6 +37,7 @@ final class AppSettings: ObservableObject { } private let defaults: UserDefaults + private let loginItemService: LoginItemService @Published var launchAtStartup: Bool @Published var showMenuBarIcon: Bool @@ -49,8 +50,9 @@ final class AppSettings: ObservableObject { private var cancellables: Set = [] private var isApplyingExternalUpdate = false - init(defaults: UserDefaults = .standard) { + init(defaults: UserDefaults = .standard, loginItemService: LoginItemService = SMAppService.mainApp) { self.defaults = defaults + self.loginItemService = loginItemService Self.migrateLegacyKeys(in: defaults) @@ -60,7 +62,7 @@ final class AppSettings: ObservableObject { self.quitOnCommandQ = (defaults.object(forKey: Keys.quitOnCommandQ) as? Int ?? 0) != 0 let stored = (defaults.object(forKey: Keys.launchAtStartup) as? Int ?? 0) != 0 - let serviceEnabled = SMAppService.mainApp.status == .enabled + let serviceEnabled = loginItemService.status == .enabled self.launchAtStartup = stored || serviceEnabled self.keyMappings = Self.loadKeyMappings(from: defaults) @@ -74,13 +76,13 @@ final class AppSettings: ObservableObject { /// Call once on app launch to reconcile any drift between SMAppService /// (the OS-side login item registry) and our stored toggle. func bootstrap() { - let serviceEnabled = SMAppService.mainApp.status == .enabled + let serviceEnabled = loginItemService.status == .enabled if launchAtStartup && !serviceEnabled { // Stored intent is "on" but the OS is not enabled. Which way to // resolve this depends on *why* the service isn't enabled, so // switch on the actual status instead of treating every // non-enabled state the same: - switch SMAppService.mainApp.status { + switch loginItemService.status { case .requiresApproval: // The item IS registered with SMAppService, but the user // disabled it (or hasn't approved it yet) in System @@ -105,7 +107,7 @@ final class AppSettings: ObservableObject { // SMAppService registration. Honor the stored intent and // register now — this restores the pre-fix behavior for // this specific case only. - setLaunchAtStartup(true) + setLaunchAtStartup(true, service: loginItemService) } } else if !launchAtStartup && serviceEnabled { // OS already registered us (e.g. enabled in System Settings) but @@ -163,7 +165,7 @@ final class AppSettings: ObservableObject { .sink { [weak self] newValue in guard let self = self, !self.isApplyingExternalUpdate else { return } self.defaults.set(newValue ? 1 : 0, forKey: Keys.launchAtStartup) - if !setLaunchAtStartup(newValue) { + if !setLaunchAtStartup(newValue, service: self.loginItemService) { self.isApplyingExternalUpdate = true self.launchAtStartup = !newValue self.defaults.set(!newValue ? 1 : 0, forKey: Keys.launchAtStartup) diff --git a/apps/cmd-ime-swift/Sources/CmdIMESwift/toggleLaunchAtStartup.swift b/apps/cmd-ime-swift/Sources/CmdIMESwift/toggleLaunchAtStartup.swift index 21b3a3c..ba58598 100644 --- a/apps/cmd-ime-swift/Sources/CmdIMESwift/toggleLaunchAtStartup.swift +++ b/apps/cmd-ime-swift/Sources/CmdIMESwift/toggleLaunchAtStartup.swift @@ -10,9 +10,20 @@ import Cocoa import ServiceManagement +/// Seam over `SMAppService.mainApp` so tests can inject a fake instead of +/// hitting the real OS-level login item registry. `SMAppService` already has +/// matching `status`/`register()`/`unregister()` members, so it conforms for +/// free — see the extension below. +protocol LoginItemService { + var status: SMAppService.Status { get } + func register() throws + func unregister() throws +} + +extension SMAppService: LoginItemService {} + @discardableResult -func setLaunchAtStartup(_ enabled: Bool) -> Bool { - let service = SMAppService.mainApp +func setLaunchAtStartup(_ enabled: Bool, service: LoginItemService = SMAppService.mainApp) -> Bool { do { if enabled { guard service.status != .enabled else { return true } diff --git a/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/AppSettingsTests.swift b/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/AppSettingsTests.swift index 7e2cfd6..fdbf2dc 100644 --- a/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/AppSettingsTests.swift +++ b/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/AppSettingsTests.swift @@ -156,22 +156,38 @@ final class AppSettingsTests: XCTestCase { } func testBootstrapWithStoredTrueAndServiceNotFoundRegistersInstead() { - // SMAppService.Status isn't injectable here, so this only exercises - // whatever status the real API reports for the `swift test` process - // (an unsigned, unregistered binary — not a proper .app bundle). - // That is reliably `.notFound` (confirmed empirically), which is the - // "never registered" branch: bootstrap() must honor the stored - // intent and attempt to register, NOT flip the toggle off. The - // `.requiresApproval` branch (registered but user-disabled) isn't - // reachable from this test process and has no automated coverage. + // Never-registered case (.notFound, the real status a bare + // `swift test` process reported before this test used a fake — see + // FakeLoginItemService): bootstrap() must honor the stored intent + // and attempt to register, NOT flip the toggle off. + let fakeService = FakeLoginItemService(status: .notFound) defaults.set(1, forKey: "launchAtStartup") - let settings = AppSettings(defaults: defaults) + let settings = AppSettings(defaults: defaults, loginItemService: fakeService) XCTAssertTrue(settings.launchAtStartup, "precondition: stored value loaded as on") settings.bootstrap() XCTAssertTrue(settings.launchAtStartup, "never-registered case must honor stored intent, not follow OS off") XCTAssertEqual(defaults.object(forKey: "launchAtStartup") as? Int, 1) + XCTAssertEqual(fakeService.registerCallCount, 1, "bootstrap() must register the never-registered case") + } + + func testBootstrapWithStoredTrueAndRequiresApprovalFollowsOSStateInstead() { + // Registered but user-disabled (or not yet approved) in System + // Settings -> Login Items: bootstrap() must defer to the OS state, + // not force re-registration behind the user's back. This branch was + // previously unreachable from a real `swift test` process (see + // git history) and had no automated coverage. + let fakeService = FakeLoginItemService(status: .requiresApproval) + defaults.set(1, forKey: "launchAtStartup") + let settings = AppSettings(defaults: defaults, loginItemService: fakeService) + XCTAssertTrue(settings.launchAtStartup, "precondition: stored value loaded as on") + + settings.bootstrap() + + XCTAssertFalse(settings.launchAtStartup, "requiresApproval case must follow the OS state, not stored intent") + XCTAssertEqual(defaults.object(forKey: "launchAtStartup") as? Int, 0) + XCTAssertEqual(fakeService.registerCallCount, 0, "must not force re-registration behind the user's back") } func testExclusionMutationsPropagateToGlobals() { diff --git a/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/FakeLoginItemService.swift b/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/FakeLoginItemService.swift new file mode 100644 index 0000000..4a31ed0 --- /dev/null +++ b/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/FakeLoginItemService.swift @@ -0,0 +1,32 @@ +import ServiceManagement +@testable import CmdIMESwift + +/// In-memory stand-in for `SMAppService.mainApp` (see LoginItemService in +/// toggleLaunchAtStartup.swift). Tests must never touch the real OS-level +/// login item registry: a prior bug had a test call the real API, which +/// registered the `xctest` test-runner binary itself as a login item on the +/// developer's Mac (see CLAUDE.md and AppSettingsTests.swift for the +/// incident this class exists to prevent). +final class FakeLoginItemService: LoginItemService { + var status: SMAppService.Status + private(set) var registerCallCount = 0 + private(set) var unregisterCallCount = 0 + var registerError: Error? + var unregisterError: Error? + + init(status: SMAppService.Status = .notRegistered) { + self.status = status + } + + func register() throws { + registerCallCount += 1 + if let registerError { throw registerError } + status = .enabled + } + + func unregister() throws { + unregisterCallCount += 1 + if let unregisterError { throw unregisterError } + status = .notRegistered + } +} diff --git a/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/SMAppServiceTestIsolationTests.swift b/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/SMAppServiceTestIsolationTests.swift new file mode 100644 index 0000000..6ac1059 --- /dev/null +++ b/apps/cmd-ime-swift/Tests/CmdIMESwiftTests/SMAppServiceTestIsolationTests.swift @@ -0,0 +1,31 @@ +import XCTest + +/// Regression guard for the incident where a test called the real +/// `SMAppService.mainApp` API and registered the `xctest` test-runner binary +/// itself as a login item on the developer's Mac (bootstrap() -> +/// setLaunchAtStartup(true) -> SMAppService.mainApp.register()). Every test +/// must go through the injectable `LoginItemService` seam (see +/// toggleLaunchAtStartup.swift / FakeLoginItemService.swift) instead. +final class SMAppServiceTestIsolationTests: XCTestCase { + func testNoTestFileReferencesTheRealSMAppServiceMainApp() throws { + let testsDirectory = URL(fileURLWithPath: #filePath).deletingLastPathComponent() + let contents = try FileManager.default.contentsOfDirectory( + at: testsDirectory, includingPropertiesForKeys: nil + ) + let swiftFiles = contents.filter { $0.pathExtension == "swift" } + XCTAssertFalse(swiftFiles.isEmpty, "sanity check: the test directory listing must not be empty") + + for file in swiftFiles { + // FakeLoginItemService itself is a legitimate SMAppService.Status + // consumer (it stands in for the real type's status enum), and + // this file's own doc comment names the hazard string it scans + // for — neither is an actual live call. + guard file.lastPathComponent != "FakeLoginItemService.swift", + file.lastPathComponent != "SMAppServiceTestIsolationTests.swift" else { continue } + let contents = try String(contentsOf: file, encoding: .utf8) + XCTAssertFalse(contents.contains("SMAppService.mainApp"), + "\(file.lastPathComponent) references the real SMAppService.mainApp — " + + "inject FakeLoginItemService instead (see AppSettingsTests.swift)") + } + } +}