From e242bbf42635c47e0623b9b7fe6bf292d814f12f Mon Sep 17 00:00:00 2001 From: Qiu Yuzhou Date: Tue, 22 Sep 2026 17:36:24 +0800 Subject: [PATCH] feat(ng2): persist proxy mode selection and drop enabled-modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 模式选择(菜单唯一入口)随设置快照持久化,GUI 重启恢复所选模式;持久化 失败保留旧模式并点名呈现,不静默切换。外部 PAC 模式下保存设置时,模式从 新快照解析后与 URL 一起作为一个逻辑变更提交(清空 URL 回落 PAC 并同步 持久化),校验失败先于写入。重置偏好把模式回 PAC。 enabledModes 自 ADR-0002 停止偏好迁移后无任何写入方,且 2.0 无编辑 UI, 按死状态删除:内置三模式恒可选,外部 PAC 仍以已配置的有效 URL 为门槛 (ADR-0004)。 测试:模式持久化与重启恢复、持久化失败保留旧值、外部 PAC 一个逻辑变更 回落;makeController 的 proxyMode 参数改为可选以便测试走恢复路径。 --- CONTEXT.md | 3 +- .../App/ProxyRuntimeController.swift | 37 +++++++--- ShadowsocksX-NG2/App/ProxyStatusMenu.swift | 23 ++---- ShadowsocksX-NG2/Domain/ProxySettings.swift | 27 ++----- .../Tests/CatalogWorkflowRollbackTests.swift | 1 - .../Tests/LegacyImportTests.swift | 1 - .../Tests/ProxyRuntimeControllerTests.swift | 71 ++++++++++++++++++- .../Tests/ProxySettingsFileStoreTests.swift | 1 - docs/adr/0004-persist-proxy-mode-selector.md | 20 ++++++ 9 files changed, 134 insertions(+), 50 deletions(-) create mode 100644 docs/adr/0004-persist-proxy-mode-selector.md diff --git a/CONTEXT.md b/CONTEXT.md index e8bb227f..d548a6cb 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -12,7 +12,7 @@ - **Activation candidate**: A server configuration with no known local blocking problem for proxy configuration generation. This status does not guarantee remote reachability or correctness of opaque plugin options. - **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. +- **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. Its choice persists across GUI restarts. - **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. @@ -58,4 +58,5 @@ - The ports of the locally provided proxy endpoints are explicit user configuration: the system never rewrites a port on its own, and a port conflict rejects proxy start with an error naming the endpoint until the user explicitly chooses a new port. A recovery suggestion only proposes candidate ports (32768–65535, free and distinct from the other endpoints' configured values) and applies only after the user confirms and saves; changing the PAC port warns that previously shared PAC URLs become invalid. - The PAC HTTP endpoint and the external tunnel service start and stop together inside the proxy runtime: both are reachable exactly while proxying is on, and neither depends on the GUI being open. The GUI writes the system proxy configuration and gates that write on endpoint health. - Proxy modes are mutually exclusive: PAC writes the runtime PAC URL, global writes only the local SOCKS proxy, manual restores the user's prior system proxy state, and external PAC writes a validated HTTP/HTTPS URL. System proxy writes are allowed only after endpoint health; 2.0 stores the complete per-service Proxies dictionaries it owns and restores them only while the applied values still match, otherwise it reports an ownership conflict. +- The proxy mode selector's choice persists with the settings snapshot and survives GUI restarts; a persistence failure keeps the previous mode in force and names the reason. The built-in modes are always available; the external PAC mode requires a configured valid URL. Resetting preferences returns the mode to PAC. - The GUI is the user-facing manager; the Shadowsocks tunnel service is an external runtime boundary rather than part of the GUI's domain model. diff --git a/ShadowsocksX-NG2/App/ProxyRuntimeController.swift b/ShadowsocksX-NG2/App/ProxyRuntimeController.swift index 7ef2afd5..fafb15ff 100644 --- a/ShadowsocksX-NG2/App/ProxyRuntimeController.swift +++ b/ShadowsocksX-NG2/App/ProxyRuntimeController.swift @@ -208,8 +208,21 @@ final class ProxyRuntimeController: ObservableObject { /// Changes the current mode without rebuilding the tunnel runtime. A mode /// that writes system settings reuses the same endpoint health gate; manual /// mode restores the snapshot immediately and leaves system settings alone. + /// The choice persists with the settings snapshot first, so a GUI restart + /// restores it; a persistence failure keeps the previous mode in force and + /// names the reason instead of switching silently. func setProxyMode(_ mode: ProxyMode) async { guard mode != proxyMode else { return } + var next = settings + next.preferredMode = mode.kind + do { + try settingsStore.save(next) + } catch { + RuntimeLog.emit(.runtimePersistFailed(detail: String(describing: error))) + state = .serviceFailed(detail: String(describing: error)) + return + } + settings = next proxyMode = mode guard state != .off, let document = lastDocument ?? runtimeFileStore.loadDocument() else { return @@ -456,21 +469,27 @@ extension ProxyRuntimeController { } /// Persists a fully validated settings snapshot and, when the proxy is - /// active, re-derives the same runtime path with the new snapshot. - func updateSettings(_ next: ProxySettings) async throws { - try settingsStore.save(next) - settings = next - listenUnreadableReason = nil - settingsUnreadableReason = nil - + /// active, re-derives the same runtime path with the new snapshot. While the + /// live mode is external PAC, the mode is re-resolved from the new snapshot + /// before persistence, so the mode kind and its URL commit as one logical + /// change and a validation failure precedes the write. + func updateSettings(_ proposed: ProxySettings) async throws { + var next = proposed + var resolvedMode = proxyMode if case .externalPAC = proxyMode { if let url = URL(string: next.externalPACURL), !next.externalPACURL.isEmpty { try ProxyMode.validateExternalPACURL(url) - proxyMode = .externalPAC(url) + resolvedMode = .externalPAC(url) } else { - proxyMode = .pac + resolvedMode = .pac + next.preferredMode = .pac } } + try settingsStore.save(next) + settings = next + proxyMode = resolvedMode + listenUnreadableReason = nil + settingsUnreadableReason = nil guard state != .off else { return } switch reexpand() { diff --git a/ShadowsocksX-NG2/App/ProxyStatusMenu.swift b/ShadowsocksX-NG2/App/ProxyStatusMenu.swift index edd224a7..20803848 100644 --- a/ShadowsocksX-NG2/App/ProxyStatusMenu.swift +++ b/ShadowsocksX-NG2/App/ProxyStatusMenu.swift @@ -39,17 +39,12 @@ struct ProxyStatusMenu: View { Task { await controller.setProxyEnabled(!summary.isOn) } } - // ③ 模式选择（勾选态） + // ③ 模式选择（勾选态）：内置三模式恒可选；外部 PAC 仅在已配置有效 URL + // 时成为选项。 Picker("模式", selection: modeBinding) { - if isModeAvailable(.pac) { - Text(ProxyMode.pac.label).tag(ProxyMode.pac) - } - if isModeAvailable(.global) { - Text(ProxyMode.global.label).tag(ProxyMode.global) - } - if isModeAvailable(.manual) { - Text(ProxyMode.manual.label).tag(ProxyMode.manual) - } + Text(ProxyMode.pac.label).tag(ProxyMode.pac) + Text(ProxyMode.global.label).tag(ProxyMode.global) + Text(ProxyMode.manual.label).tag(ProxyMode.manual) if let externalMode = configuredExternalPACMode { Text(externalMode.label).tag(externalMode) } @@ -120,16 +115,12 @@ struct ProxyStatusMenu: View { set: { mode in Task { await controller.setProxyMode(mode) } }) } - private func isModeAvailable(_ mode: ProxyMode) -> Bool { - mode.kind == controller.proxyMode.kind || controller.settings.enabledModes.contains(mode.kind) - } - + /// 外部 PAC 仅在已配置有效 URL 时成为菜单选项；URL 的编辑面在设置区。 private var configuredExternalPACMode: ProxyMode? { if case .externalPAC(let url) = controller.proxyMode { return .externalPAC(url) } - guard controller.settings.enabledModes.contains(.externalPAC), - let url = URL(string: controller.settings.externalPACURL), + guard let url = URL(string: controller.settings.externalPACURL), !controller.settings.externalPACURL.isEmpty, (try? ProxyMode.validateExternalPACURL(url)) != nil else { return nil } diff --git a/ShadowsocksX-NG2/Domain/ProxySettings.swift b/ShadowsocksX-NG2/Domain/ProxySettings.swift index af4019a2..d7529efb 100644 --- a/ShadowsocksX-NG2/Domain/ProxySettings.swift +++ b/ShadowsocksX-NG2/Domain/ProxySettings.swift @@ -6,8 +6,7 @@ import Foundation struct ProxySettings: Equatable, Sendable { static let defaultGFWListURL = "https://cdn.jsdelivr.net/gh/gfwlist/gfwlist/gfwlist.txt" static let defaultProxyExceptions = - "127.0.0.1, localhost, 192.168.0.0/16, 10.0.0.0/8, FE80::/64, ::1, FD00::/8" - static let defaultEnabledModes: Set = [.pac, .global, .manual] + "127.0.0.1, localhost, 192.168.0.0/16, 10.0.0/8, FE80::/64, ::1, FD00::/8" var listen: SslocalListenSettings var timeoutSeconds: Int @@ -16,12 +15,10 @@ struct ProxySettings: Equatable, Sendable { var externalPACURL: String var gfwListURL: String var pacUserRules: String - /// Mode selected after a restart. An external PAC URL is resolved from - /// `externalPACURL` when this kind is `.externalPAC`. + /// The persisted current mode: the mode selector's choice survives GUI + /// restarts. An external PAC URL is resolved from `externalPACURL` when + /// this kind is `.externalPAC`. var preferredMode: ProxyModeKind - /// The set is used by the menu/settings views to keep disabled modes out of - /// the cycle. - var enabledModes: Set init( listen: SslocalListenSettings = SslocalListenSettings(), @@ -31,8 +28,7 @@ struct ProxySettings: Equatable, Sendable { externalPACURL: String = "", gfwListURL: String = ProxySettings.defaultGFWListURL, pacUserRules: String = "", - preferredMode: ProxyModeKind = .pac, - enabledModes: Set = ProxySettings.defaultEnabledModes + preferredMode: ProxyModeKind = .pac ) { self.listen = listen self.timeoutSeconds = timeoutSeconds @@ -42,7 +38,6 @@ struct ProxySettings: Equatable, Sendable { self.gfwListURL = gfwListURL self.pacUserRules = pacUserRules self.preferredMode = preferredMode - self.enabledModes = enabledModes } /// 系统代理的 ExceptionsList；输入顺序保留，重复项只保留第一次出现的值。 @@ -356,8 +351,7 @@ struct ProxySettingsFileStore: ProxySettingsStoring { externalPACURL: externalPACURL, gfwListURL: gfwListURL, pacUserRules: record.pacUserRules, - preferredMode: record.preferredMode, - enabledModes: Set(record.enabledModes))) + preferredMode: record.preferredMode)) } private func record(from settings: ProxySettings) -> ProxySettingsRecord { @@ -382,7 +376,6 @@ struct ProxySettingsFileStore: ProxySettingsStoring { record.gfwListURLConfigured = true record.pacUserRules = settings.pacUserRules record.preferredMode = settings.preferredMode - record.enabledModes = settings.enabledModes.sorted { $0.rawValue < $1.rawValue } return record } @@ -409,9 +402,6 @@ private struct ProxySettingsRecord: Codable, Equatable, Sendable { var gfwListURLConfigured: Bool = false var pacUserRules: String = "" var preferredMode: ProxyModeKind = .pac - var enabledModes: [ProxyModeKind] = ProxySettings.defaultEnabledModes.sorted { - $0.rawValue < $1.rawValue - } init() {} @@ -444,9 +434,6 @@ private struct ProxySettingsRecord: Codable, Equatable, Sendable { pacUserRules = try container.decodeIfPresent(String.self, forKey: .pacUserRules) ?? "" preferredMode = try container.decodeIfPresent(ProxyModeKind.self, forKey: .preferredMode) ?? .pac - enabledModes = - try container.decodeIfPresent([ProxyModeKind].self, forKey: .enabledModes) - ?? ProxySettings.defaultEnabledModes.sorted { $0.rawValue < $1.rawValue } } } @@ -455,6 +442,6 @@ extension ProxySettingsRecord { case scopeKind, advertisedAddress, socksPort, httpProxyEnabled, httpPort, pacPort case udpRelayEnabled, timeoutSeconds, verboseLogging, proxyExceptions case externalPACCredentialReference, gfwListCredentialReference, gfwListURLConfigured - case pacUserRules, preferredMode, enabledModes + case pacUserRules, preferredMode } } diff --git a/ShadowsocksX-NG2/Tests/CatalogWorkflowRollbackTests.swift b/ShadowsocksX-NG2/Tests/CatalogWorkflowRollbackTests.swift index 630103bf..53af3b17 100644 --- a/ShadowsocksX-NG2/Tests/CatalogWorkflowRollbackTests.swift +++ b/ShadowsocksX-NG2/Tests/CatalogWorkflowRollbackTests.swift @@ -174,7 +174,6 @@ final class CatalogWorkflowRollbackTests: XCTestCase { existingSettings.listen.httpPort = 2087 existingSettings.listen.pacPort = 2089 existingSettings.preferredMode = .global - existingSettings.enabledModes = [.global] let settingsStore = ProxySettingsFileStore( fileURL: settingsURL, legacyListenFileURL: workDir.appendingPathComponent("legacy-listen.json"), diff --git a/ShadowsocksX-NG2/Tests/LegacyImportTests.swift b/ShadowsocksX-NG2/Tests/LegacyImportTests.swift index ac209141..227d1007 100644 --- a/ShadowsocksX-NG2/Tests/LegacyImportTests.swift +++ b/ShadowsocksX-NG2/Tests/LegacyImportTests.swift @@ -72,7 +72,6 @@ final class LegacyImportTests: XCTestCase { existingSettings.listen.pacPort = 2089 existingSettings.timeoutSeconds = 120 existingSettings.preferredMode = .global - existingSettings.enabledModes = [.global] let settingsStore = ProxySettingsFileStore( fileURL: settingsURL, legacyListenFileURL: directory.appendingPathComponent("legacy-listen.json"), diff --git a/ShadowsocksX-NG2/Tests/ProxyRuntimeControllerTests.swift b/ShadowsocksX-NG2/Tests/ProxyRuntimeControllerTests.swift index e9cd33c8..a9d8f453 100644 --- a/ShadowsocksX-NG2/Tests/ProxyRuntimeControllerTests.swift +++ b/ShadowsocksX-NG2/Tests/ProxyRuntimeControllerTests.swift @@ -70,7 +70,7 @@ final class ProxyRuntimeControllerTests: XCTestCase { settingsStore: ProxySettingsStoring? = nil, settingsRestore: RestoredProxySettings? = nil, pacProbe: PACHealthProbing = ProxyRuntimeFixture.FakePACProbe(), - proxyMode: ProxyMode = .pac, + proxyMode: ProxyMode? = .pac, systemProxy: SystemProxyControlling? = nil, firewallChecker: FirewallStatusChecking = ProxyRuntimeFixture.FakeFirewallChecker(), firewallExecutableURLs: [URL] = [URL(fileURLWithPath: "/bundle/Helpers/sslocal")], @@ -427,6 +427,67 @@ final class ProxyRuntimeControllerTests: XCTestCase { } extension ProxyRuntimeControllerTests { + func testSetProxyModePersistsTheChoiceAndARestoreRestoresIt() async throws { + let settingsStore = InMemoryProxySettingsStore() + let controller = makeController( + probe: ProxyRuntimeFixture.FakeProbe.reachable(), settingsStore: settingsStore) + + await controller.setProxyMode(.global) + + XCTAssertEqual(controller.proxyMode, .global) + XCTAssertEqual(controller.settings.preferredMode, .global) + XCTAssertEqual(settingsStore.saved?.preferredMode, .global, "模式选择随快照持久化") + + // GUI 重启路径：恢复出的控制器不注入显式模式，从持久快照读回。 + let restored = makeController( + probe: ProxyRuntimeFixture.FakeProbe.reachable(), + settingsRestore: RestoredProxySettings( + settings: try XCTUnwrap(settingsStore.load()), unreadableError: nil), + proxyMode: nil) + XCTAssertEqual(restored.proxyMode, .global) + } + + func testSetProxyModePersistenceFailureKeepsPreviousModeAndNamesReason() async throws { + let settingsStore = InMemoryProxySettingsStore() + settingsStore.saveError = FakeSettingsSaveError.system + let controller = makeController( + probe: ProxyRuntimeFixture.FakeProbe.reachable(), settingsStore: settingsStore) + + await controller.setProxyMode(.global) + + XCTAssertEqual(controller.proxyMode, .pac, "持久化失败保留旧模式") + XCTAssertEqual(controller.settings.preferredMode, .pac) + guard case .serviceFailed(let detail) = controller.state else { + XCTFail("应点名持久化失败，实际 \(controller.state)") + return + } + XCTAssertTrue(detail.contains("fake-save-error")) + } + + func testUpdateSettingsWhileInExternalPACModeCommitsKindAndURLAsOneChange() async throws { + let settingsStore = InMemoryProxySettingsStore() + let externalURL = URL(string: "https://pac.example.test/proxy.pac")! + var external = ProxySettings() + external.externalPACURL = externalURL.absoluteString + external.preferredMode = .externalPAC + settingsStore.saved = external + let controller = makeController( + probe: ProxyRuntimeFixture.FakeProbe.reachable(), + settingsStore: settingsStore, + settingsRestore: RestoredProxySettings(settings: external, unreadableError: nil), + proxyMode: nil) + XCTAssertEqual(controller.proxyMode, .externalPAC(externalURL)) + + // 清空外部 PAC URL：模式回落 PAC 且持久化的 kind 同步回落。 + var cleared = external + cleared.externalPACURL = "" + try await controller.updateSettings(cleared) + + XCTAssertEqual(controller.proxyMode, .pac) + XCTAssertEqual(settingsStore.saved?.preferredMode, .pac) + XCTAssertEqual(settingsStore.saved?.externalPACURL, "") + } + func testLegacyImportBoundaryStopsRuntimeWithoutRestoringSystemProxy() async throws { let seeded = try makeSeededCatalog() let settingsStore = InMemoryProxySettingsStore() @@ -497,14 +558,22 @@ extension ProxyRuntimeControllerTests { ["localhost", "127.0.0.1"]) } + private enum FakeSettingsSaveError: Error, CustomStringConvertible { + case system + + var description: String { "fake-save-error" } + } + private final class InMemoryProxySettingsStore: ProxySettingsStoring { var saved: ProxySettings? + var saveError: Error? func load() throws -> ProxySettings { saved ?? ProxySettings() } func save(_ settings: ProxySettings) throws { + if let saveError { throw saveError } saved = settings } diff --git a/ShadowsocksX-NG2/Tests/ProxySettingsFileStoreTests.swift b/ShadowsocksX-NG2/Tests/ProxySettingsFileStoreTests.swift index 2a21d79d..f1e50a85 100644 --- a/ShadowsocksX-NG2/Tests/ProxySettingsFileStoreTests.swift +++ b/ShadowsocksX-NG2/Tests/ProxySettingsFileStoreTests.swift @@ -46,7 +46,6 @@ final class ProxySettingsFileStoreTests: XCTestCase { settings.gfwListURL = "https://lists.example.test/gfw.txt?token=secret" settings.pacUserRules = "@@||example.com^" settings.preferredMode = .externalPAC - settings.enabledModes = [.pac, .externalPAC] try store.save(settings) diff --git a/docs/adr/0004-persist-proxy-mode-selector.md b/docs/adr/0004-persist-proxy-mode-selector.md new file mode 100644 index 00000000..ad405ae8 --- /dev/null +++ b/docs/adr/0004-persist-proxy-mode-selector.md @@ -0,0 +1,20 @@ +# Persist the proxy mode selector and drop enabled-modes + +The proxy mode selector (status menu, the single entry point since the settings-page selector was removed) persists its choice with the settings snapshot, so a GUI restart restores the selected mode instead of reverting to PAC. A persistence failure keeps the previous mode in force and names the reason rather than switching silently. While the live mode is external PAC, saving settings resolves the mode from the new snapshot before persisting, so the mode kind and its URL commit as one logical change. The `enabledModes` set is removed from the persisted snapshot: since ADR-0002 dropped Legacy preference migration it had no writer, and 2.0 exposes no editing UI for it. The built-in modes (PAC, global, manual) are always available; the external PAC mode requires a configured valid URL. + +## Status + +Accepted. + +## Considered options + +- Keep mode switches as transient runtime state: rejected because the selector is the common control for how macOS traffic is routed; losing the choice on every GUI restart contradicts its persisted-snapshot slot (`preferredMode`) and surprises users. +- Persist the mode in a separate preference key: rejected because the mode already belongs to the settings snapshot (an external PAC mode resolves its URL from it); a second key would split one logical change into two stores. +- Keep `enabledModes` as read-only legacy data: rejected because a persisted set with no writer is dead state that still gates the menu for no product meaning. + +## Consequences + +- `setProxyMode` writes the settings snapshot before switching the live mode; on write failure it presents a service failure and leaves the mode unchanged. +- `updateSettings` re-resolves an external-PAC mode from the incoming snapshot before persisting; clearing the URL falls back to PAC and persists that fallback. +- Resetting preferences returns the mode to PAC, consistent with the factory snapshot. +- The persisted settings document no longer contains `enabledModes`; existing files simply stop reading it, and no migration is needed because no release shipped with it populated by any 2.0 path.