From 376b648ac4819cc100d37b7ed19c1f4274d4b2bb Mon Sep 17 00:00:00 2001 From: Qiu Yuzhou Date: Tue, 22 Sep 2026 17:32:46 +0800 Subject: [PATCH] refactor(login): independent opt-in login item Drop the UserDefaults intent mirror and read the toggle state only from the system login-item registration status (approval-pending counts as enabled); default to off until the user enables it, and stop resetting it with proxy preferences (ADR-0005). --- CONTEXT.md | 1 + .../App/LoginAtLoginService.swift | 39 +++-------- ShadowsocksX-NG2/App/MainApp.swift | 3 - ShadowsocksX-NG2/App/SettingsView.swift | 1 - .../Tests/LoginAtLoginServiceTests.swift | 69 +++++++++++-------- .../adr/0005-independent-opt-in-login-item.md | 18 +++++ 6 files changed, 71 insertions(+), 60 deletions(-) create mode 100644 docs/adr/0005-independent-opt-in-login-item.md diff --git a/CONTEXT.md b/CONTEXT.md index 662685f5..e8bb227f 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -13,6 +13,7 @@ - **Managed plugin**: A SIP003 plugin executable that the product packages, hash-verifies, re-signs, and distributes only inside the app bundle. The managed set changes only with app releases; users cannot supply their own plugin executables. - **Proxy mode**: The user-facing way traffic is routed through the local proxy, such as PAC, global, manual, or an external PAC configuration. - **Proxy mode selector**: The common user action for choosing one mutually exclusive proxy mode; it is distinct from enabling or disabling the proxy runtime and from configuring an external PAC URL. +- **Login item**: The GUI app's registration in the user's login items. It is an independent preference domain: off until the user enables it, its state is only ever the system login-item registration status, and resetting preferences never changes it. - **HTTP proxy mode**: A local proxy mode that accepts HTTP/HTTPS proxy requests directly through the Shadowsocks runtime; it does not require a separate adapter process. - **Legacy Privoxy adapter**: The frozen implementation's HTTP(S)-to-local-SOCKS5 bridge. It is a Legacy-only dependency and is not migrated into 2.0. - **Listen scope**: A user-facing two-state choice of whether the locally provided proxy endpoints — the PAC HTTP endpoint and the external tunnel service's inbound listeners — bind to loopback only or to the host's network-facing address. The host-facing scope intentionally exposes an unauthenticated local proxy to other devices on the network, so users can share the PAC URL or proxy address to machines other than this one. diff --git a/ShadowsocksX-NG2/App/LoginAtLoginService.swift b/ShadowsocksX-NG2/App/LoginAtLoginService.swift index 8d541b7b..759d9806 100644 --- a/ShadowsocksX-NG2/App/LoginAtLoginService.swift +++ b/ShadowsocksX-NG2/App/LoginAtLoginService.swift @@ -43,50 +43,31 @@ struct SMAppLaunchAtLoginService: LaunchAtLoginControlling { } /// Owns the GUI login-item intent independently from the proxy LaunchAgent. -/// The setting is enabled by default; registration is attempted at launch and -/// again whenever the user changes the toggle. +/// The system's login-item registration is the only source of truth: the +/// toggle is off until the user enables it, and enabling or disabling only +/// issues the corresponding system call before re-reading the status. @MainActor final class LaunchAtLoginController: ObservableObject { - static let preferenceKey = "launchAtLogin.enabled" - - @Published private(set) var isEnabled: Bool @Published private(set) var status: LoginItemStatus @Published private(set) var errorMessage: String? private let service: LaunchAtLoginControlling - private let defaults: UserDefaults - init( - service: LaunchAtLoginControlling = SMAppLaunchAtLoginService(), - defaults: UserDefaults = .standard - ) { + init(service: LaunchAtLoginControlling = SMAppLaunchAtLoginService()) { self.service = service - self.defaults = defaults - isEnabled = defaults.object(forKey: Self.preferenceKey) as? Bool ?? true status = service.status } + /// 已注册或已请求注册待批准都算开启——两者都只来自系统 API 的状态。 + var isEnabled: Bool { + status == .registered || status == .requiresApproval + } + var requiresApproval: Bool { status == .requiresApproval } - func syncAtLaunch() { - applyDesiredState() - } - func setEnabled(_ enabled: Bool) { - isEnabled = enabled - defaults.set(enabled, forKey: Self.preferenceKey) - applyDesiredState() - } - - func resetToDefaults() { - defaults.removeObject(forKey: Self.preferenceKey) - isEnabled = true - applyDesiredState() - } - - private func applyDesiredState() { do { - if isEnabled { + if enabled { try service.register() } else { try service.unregister() diff --git a/ShadowsocksX-NG2/App/MainApp.swift b/ShadowsocksX-NG2/App/MainApp.swift index c41d53f5..892dba09 100644 --- a/ShadowsocksX-NG2/App/MainApp.swift +++ b/ShadowsocksX-NG2/App/MainApp.swift @@ -29,9 +29,6 @@ struct ShadowsocksXNG2App: App { postLegacyImport: { _ in await controller.legacyImportDidCommit() })) - Task { @MainActor in - loginController.syncAtLaunch() - } // GUI 事件接入内存环形缓冲（spec #21 D5，issue #34）：主窗口日志查看器与 // 诊断导出的来源；wrapper 侧不注册，仍走 stderr → agent.log 收敛。 RuntimeLog.setSink(RuntimeEventStore.shared) diff --git a/ShadowsocksX-NG2/App/SettingsView.swift b/ShadowsocksX-NG2/App/SettingsView.swift index e580da54..6961d44e 100644 --- a/ShadowsocksX-NG2/App/SettingsView.swift +++ b/ShadowsocksX-NG2/App/SettingsView.swift @@ -353,7 +353,6 @@ extension SettingsView { Task { @MainActor in do { try await proxyController.resetPreferences() - loginController.resetToDefaults() draft = proxyController.settings refreshPortOccupancy() } catch { diff --git a/ShadowsocksX-NG2/Tests/LoginAtLoginServiceTests.swift b/ShadowsocksX-NG2/Tests/LoginAtLoginServiceTests.swift index bf526023..d48dd223 100644 --- a/ShadowsocksX-NG2/Tests/LoginAtLoginServiceTests.swift +++ b/ShadowsocksX-NG2/Tests/LoginAtLoginServiceTests.swift @@ -4,62 +4,77 @@ import XCTest @MainActor final class LaunchAtLoginServiceTests: XCTestCase { - private var defaults: UserDefaults! - private var defaultsSuiteName: String! - - override func setUpWithError() throws { - defaultsSuiteName = "ssxng-login-tests-\(UUID().uuidString)" - defaults = try XCTUnwrap(UserDefaults(suiteName: defaultsSuiteName)) - } - - override func tearDownWithError() throws { - defaults.removePersistentDomain(forName: defaultsSuiteName) - defaults = nil - defaultsSuiteName = nil - } - - func testDefaultIntentRegistersAtLaunch() { + func testStartsOffUntilTheUserEnablesIt() { let service = FakeLoginItemService() - let controller = LaunchAtLoginController(service: service, defaults: defaults) + let controller = LaunchAtLoginController(service: service) - controller.syncAtLaunch() + XCTAssertFalse(controller.isEnabled) + XCTAssertEqual(controller.status, .notRegistered) + XCTAssertEqual(service.registerCount, 0, "启动不自动注册,默认关闭") + } + + func testEnablingRegistersAndReflectsSystemState() { + let service = FakeLoginItemService() + let controller = LaunchAtLoginController(service: service) + + controller.setEnabled(true) XCTAssertTrue(controller.isEnabled) XCTAssertEqual(service.registerCount, 1) XCTAssertEqual(controller.status, .registered) + XCTAssertNil(controller.errorMessage) } - func testToggleUnregistersAndPersistsAcrossControllerInstances() { + func testDisablingUnregistersAndReflectsSystemState() { let service = FakeLoginItemService() - let controller = LaunchAtLoginController(service: service, defaults: defaults) + let controller = LaunchAtLoginController(service: service) + controller.setEnabled(true) controller.setEnabled(false) XCTAssertFalse(controller.isEnabled) XCTAssertEqual(service.unregisterCount, 1) - let restored = LaunchAtLoginController(service: service, defaults: defaults) - XCTAssertFalse(restored.isEnabled) + XCTAssertEqual(controller.status, .notRegistered) } - func testResetReturnsToEnabledDefault() { + func testApprovalPendingCountsAsEnabled() { let service = FakeLoginItemService() - let controller = LaunchAtLoginController(service: service, defaults: defaults) - controller.setEnabled(false) - - controller.resetToDefaults() + service.status = .requiresApproval + let controller = LaunchAtLoginController(service: service) XCTAssertTrue(controller.isEnabled) + XCTAssertTrue(controller.requiresApproval) + XCTAssertEqual(service.registerCount, 0) + } + + func testRegistrationFailureNamesErrorAndKeepsSystemState() { + let service = FakeLoginItemService() + service.registerError = FakeError.system + let controller = LaunchAtLoginController(service: service) + + controller.setEnabled(true) + + XCTAssertFalse(controller.isEnabled) + XCTAssertEqual(controller.status, .notRegistered) XCTAssertEqual(service.registerCount, 1) - XCTAssertNil(defaults.object(forKey: LaunchAtLoginController.preferenceKey)) + XCTAssertTrue(controller.errorMessage?.contains("fake-system-error") == true) + } + + private enum FakeError: Error, CustomStringConvertible { + case system + + var description: String { "fake-system-error" } } private final class FakeLoginItemService: LaunchAtLoginControlling { var status: LoginItemStatus = .notRegistered var registerCount = 0 var unregisterCount = 0 + var registerError: Error? func register() throws { registerCount += 1 + if let registerError { throw registerError } status = .registered } diff --git a/docs/adr/0005-independent-opt-in-login-item.md b/docs/adr/0005-independent-opt-in-login-item.md new file mode 100644 index 00000000..e83759d9 --- /dev/null +++ b/docs/adr/0005-independent-opt-in-login-item.md @@ -0,0 +1,18 @@ +# Independent opt-in login item + +The GUI's login-item registration is its own preference domain, separate from proxy preferences. It is off until the user explicitly enables it: no registration happens at launch. The system's login-item registration status (`SMAppService.mainApp.status`) is the only source of truth — there is no UserDefaults mirror — and a pending-approval registration counts as enabled because both states come from that system query. Resetting preferences never touches the login item. + +## Status + +Accepted. + +## Considered options + +- Keep the persisted-intent mirror (`launchAtLogin.enabled`, default on) and reconcile at launch: rejected because the mirror can disagree with the system after the user edits the login item in System Settings, producing a toggle that lies about the actual state. +- Drop the mirror but keep default-on via a one-time launch registration: rejected because opting the user's machine into a background login item without an explicit action is not 2.0's default posture; the status-menu app is deliberately explicit. + +## Consequences + +- `LaunchAtLoginController` reads `isEnabled` from the system status only; enabling and disabling issue the corresponding system call and re-read the status. +- The toggle reflects external changes only after the controller re-reads the status (on init, on toggle, and when approval completes); no continuous observation is added. +- Resetting preferences in the settings window no longer calls into the login-item domain; its confirmation message already names only ports, listen scope, and PAC settings.