diff --git a/App/Composition/AppEnvironment.swift b/App/Composition/AppEnvironment.swift index 66e87af..ac307be 100644 --- a/App/Composition/AppEnvironment.swift +++ b/App/Composition/AppEnvironment.swift @@ -627,6 +627,29 @@ final class AppEnvironment: ObservableObject { // who opts in *after* a crash still has a report to send. let crashReportService = CrashReportService() Self.installCrashHandler(service: crashReportService) + // Publish this machine's device id to the shared Keychain group at + // launch (GitHub issue #104), not when Settings ▸ Applications is first + // opened. The document-sync agent needs the id to address its + // per-machine settings document, and most users never visit that pane — + // leaving the agent stranded on local `UserDefaults` forever. + // + // **Detached, and that is not a nicety.** `DeviceIdentity.current()` + // reads and writes the Keychain, which is synchronous IPC to `securityd` + // against a shared access group. Calling it inline here hung the process + // at launch outright — the App test host never finished starting, and + // `xcodebuild test` failed with "The test runner hung before + // establishing connection" on every run. An unsigned or + // wrongly-entitled build has no claim on that access group, and the + // failure mode is a stall rather than a clean `errSecMissingEntitlement`. + // + // Even signed and entitled, a locked or first-unlock Keychain can make + // this slow. Nothing on the launch path should wait on it: the agent + // reads the published id on its own schedule, so being a few hundred + // milliseconds late costs nothing, and the Applications pane calls + // `current()` directly when it genuinely needs the value synchronously. + Task.detached(priority: .utility) { + DeviceIdentity.current() + } return AppEnvironment( messages: messages, lists: lists, diff --git a/App/Composition/DeviceIdentity.swift b/App/Composition/DeviceIdentity.swift index e58dc24..4aeaf8c 100644 --- a/App/Composition/DeviceIdentity.swift +++ b/App/Composition/DeviceIdentity.swift @@ -9,21 +9,63 @@ // It is an opaque UUID minted once per machine — no hardware identifier, so it // carries nothing personally identifying and resets cleanly if the user wipes // preferences. +// +// GitHub issue #104 adds a second home for the same value: the shared Keychain +// group, so the bundled document-sync agent — a separate process with its own +// defaults domain — can address the *same* per-machine settings document this +// app does. `UserDefaults` remains the primary source of truth so machines +// already registered under an id keep it. import Foundation +import InterlinedKit enum DeviceIdentity { private static let defaultsKey = "com.interlinedlist.deviceId" - /// The stable id for this machine, minting and persisting one on first use. - static func current(defaults: UserDefaults = .standard) -> String { + /// The shared-Keychain channel the sync agent reads. Service and group must + /// match the agent's `SyncConfiguration.deviceIDService` / + /// `sharedAccessGroup` — two independent codebases agreeing on one contract. + static let sharedStore: any DeviceIDStoring = KeychainDeviceIDStore( + service: "com.interlinedlist.macos.device-id", + accessGroup: "BJA9558E4B.com.interlinedlist.shared" + ) + + /// The stable id for this machine, minting and persisting one on first use, + /// and publishing it where the sync agent can find it. + /// + /// Resolution order matters: + /// + /// 1. **`UserDefaults`** — a machine already registered with the server + /// keeps the id its registry row is keyed on. Preferring anything else + /// would orphan that row and its per-machine settings. + /// 2. **the shared Keychain** — the app's preferences were wiped or + /// reinstalled while the agent stayed put. Adopting the published id + /// reunites the app with the machine's existing registry row instead of + /// minting a duplicate. + /// 3. **mint** — genuinely new machine. + @discardableResult + static func current( + defaults: UserDefaults = .standard, + sharedStore: any DeviceIDStoring = DeviceIdentity.sharedStore + ) -> String { + let resolved: String if let existing = defaults.string(forKey: defaultsKey), !existing.isEmpty { - return existing + resolved = existing + } else if let published = sharedStore.read(), !published.isEmpty { + resolved = published + defaults.set(published, forKey: defaultsKey) + } else { + resolved = UUID().uuidString + defaults.set(resolved, forKey: defaultsKey) + } + + // Publish only on a mismatch. The Keychain write is cheap but not free, + // and `current()` is called on every Applications-pane load. + if sharedStore.read() != resolved { + sharedStore.write(resolved) } - let minted = UUID().uuidString - defaults.set(minted, forKey: defaultsKey) - return minted + return resolved } /// A human-friendly default name for this machine, used when registering. diff --git a/App/Features/Settings/DevicesView.swift b/App/Features/Settings/DevicesView.swift index 2bc2cb5..cfb6581 100644 --- a/App/Features/Settings/DevicesView.swift +++ b/App/Features/Settings/DevicesView.swift @@ -310,6 +310,34 @@ struct DeviceSettingsInspector: View { Section("Settings saved for this machine") { SettingsDocumentSummary(document: document) } + // What the machine is actually doing, not just that it has a + // document (GitHub issue #104). Only the document-sync agent + // currently writes anything a human can be told about, so + // this is the one interpreted section on an otherwise + // deliberately opaque sheet. + if let sync = DocumentSyncStatus( + bag: document.bag, + deviceID: inspection.device.id + ) { + Section("Document Sync") { + LabeledContent("Sync folder") { + // The path itself is not shown — see the Keys + // section below on why values stay hidden. + Text(sync.isConfigured ? "Configured on this machine" : "Not configured here") + .foregroundStyle(sync.isConfigured ? .primary : .secondary) + } + LabeledContent("Syncing") { + Text(sync.isEnabled ? "On" : "Paused") + } + LabeledContent("Last synced") { + if let lastSyncAt = sync.lastSyncAt { + Text(lastSyncAt.formatted(.relative(presentation: .named))) + } else { + Text("Never").foregroundStyle(.secondary) + } + } + } + } Section("Keys") { if document.bag.isEmpty { Text("None").foregroundStyle(.secondary) diff --git a/AppTests/DeviceIdentityTests.swift b/AppTests/DeviceIdentityTests.swift new file mode 100644 index 0000000..ce14246 --- /dev/null +++ b/AppTests/DeviceIdentityTests.swift @@ -0,0 +1,105 @@ +// DeviceIdentityTests +// +// This machine's app-settings device id, and the shared-Keychain channel that +// lets the bundled document-sync agent address the *same* per-machine settings +// document the app does (GitHub issue #104). +// +// Two ids for one Mac would mean two rows in the device registry and a +// configuration split across two documents, so the resolution order is the +// behaviour under test — not an implementation detail. + +import XCTest +import InterlinedKit +@testable import InterlinedList + +final class DeviceIdentityTests: XCTestCase { + + private func freshDefaults() -> UserDefaults { + UserDefaults(suiteName: "iltest-device-\(UUID().uuidString)")! + } + + // MARK: - Happy path + + func test_givenAMachineAlreadyRegistered_whenResolving_thenItKeepsTheIdItsRegistryRowUses() { + let defaults = freshDefaults() + defaults.set("existing-id", forKey: "com.interlinedlist.deviceId") + let shared = InMemoryDeviceIDStore() + + let resolved = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + + // Preferring anything else would orphan the registry row and the + // per-machine settings filed under it. + XCTAssertEqual(resolved, "existing-id") + XCTAssertEqual(shared.read(), "existing-id", "…and the agent is told about it") + } + + func test_givenANewMachine_whenResolving_thenItMintsOnceAndPublishesIt() { + let defaults = freshDefaults() + let shared = InMemoryDeviceIDStore() + + let first = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + let second = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + + XCTAssertFalse(first.isEmpty) + XCTAssertEqual(first, second, "Minting twice would register the Mac twice") + XCTAssertEqual(shared.read(), first) + } + + // MARK: - Boundary: the app's preferences were wiped + + func test_givenTheAppsPreferencesWereWipedButTheAgentRemains_whenResolving_thenThePublishedIdIsAdopted() { + let defaults = freshDefaults() + let shared = InMemoryDeviceIDStore(initial: "published-id") + + let resolved = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + + // Reinstalling the app must reunite it with this machine's existing + // registry row rather than minting a duplicate alongside it. + XCTAssertEqual(resolved, "published-id") + XCTAssertEqual(defaults.string(forKey: "com.interlinedlist.deviceId"), "published-id") + } + + // MARK: - Invalid input + + func test_givenAnEmptyStoredId_whenResolving_thenItIsTreatedAsAbsent() { + let defaults = freshDefaults() + defaults.set("", forKey: "com.interlinedlist.deviceId") + let shared = InMemoryDeviceIDStore() + + let resolved = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + + XCTAssertFalse(resolved.isEmpty) + } + + func test_givenAnEmptyPublishedId_whenResolving_thenItIsTreatedAsAbsent() { + let defaults = freshDefaults() + let shared = InMemoryDeviceIDStore(initial: "") + + let resolved = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + + XCTAssertFalse(resolved.isEmpty) + XCTAssertEqual(shared.read(), resolved) + } + + // MARK: - Upstream failure: the Keychain is unavailable + + func test_givenTheSharedStoreCannotBeWritten_whenResolving_thenTheAppStillGetsAnId() { + // An unsigned build, or one without the shared access-group entitlement, + // cannot use the Keychain at all. The app degrades to local-only — the + // agent falls back to its own settings — rather than failing at launch. + let defaults = freshDefaults() + let shared = UnwritableDeviceIDStore() + + let resolved = DeviceIdentity.current(defaults: defaults, sharedStore: shared) + + XCTAssertFalse(resolved.isEmpty) + XCTAssertEqual(defaults.string(forKey: "com.interlinedlist.deviceId"), resolved) + } +} + +/// A shared store that silently drops every write, standing in for a process +/// that is not entitled to the shared Keychain group. +private final class UnwritableDeviceIDStore: DeviceIDStoring, @unchecked Sendable { + func read() -> String? { nil } + func write(_ id: String) {} +} diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/DocumentSyncStatus.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/DocumentSyncStatus.swift new file mode 100644 index 0000000..dc4ef50 --- /dev/null +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/DocumentSyncStatus.swift @@ -0,0 +1,75 @@ +import Foundation +import InterlinedKit + +/// What the Document Sync Agent is doing on one machine, read out of that +/// machine's per-machine app-settings document (GitHub issue #104). +/// +/// The agent stores its configuration there rather than in `UserDefaults`, which +/// is what makes this readable at all: before the move, Settings ▸ Applications +/// could list a machine and say nothing whatsoever about what it was set up to +/// do. +/// +/// **This type reads keys written by a different codebase.** The agent +/// (`SyncAgent/`) is a clean-room package with no dependency on `InterlinedKit` +/// or `InterlinedDomain`, so the key names below are a wire contract with +/// `DocumentSyncSettingsKeys` in +/// `SyncAgent/Sources/InterlinedListSyncCore/Storage/SyncAgentConfiguration.swift`. +/// Renaming one side without the other does not fail to build — it silently +/// stops the pane reporting anything. +public struct DocumentSyncStatus: Sendable, Equatable { + + /// The wire keys, mirrored from the agent. + private enum Key { + static let prefix = "documentSync." + static let enabled = prefix + "enabled" + static let folderBookmark = prefix + "folderBookmark" + static let folderMachineID = prefix + "folderMachineId" + static let lastSyncAt = prefix + "lastSyncAt" + } + + /// Whether a sync folder is set up **on the machine that owns this + /// document**. + /// + /// False when the stored bookmark was created on another Mac. That can + /// happen legitimately — copying a machine's settings to shared, or a new + /// machine seeding from the main workstation, both move the document across + /// machines — and a bookmark is meaningless anywhere but where it was made. + /// Reporting it as configured would promise the user something that is not + /// true on that computer. + public let isConfigured: Bool + + /// Whether syncing is switched on. A machine can be configured but paused. + public let isEnabled: Bool + + /// When that machine last completed a sync cycle, if it has reported one. + /// Published on a throttle by the agent, so it lags by up to an hour. + public let lastSyncAt: Date? + + public init(isConfigured: Bool, isEnabled: Bool, lastSyncAt: Date? = nil) { + self.isConfigured = isConfigured + self.isEnabled = isEnabled + self.lastSyncAt = lastSyncAt + } + + /// Projects the agent's status out of a settings document. + /// + /// Returns nil when the document holds none of the agent's keys — that + /// machine has never run the agent, which is a different statement from + /// "has it installed but switched off", and the pane says so differently. + /// + /// - Parameter deviceID: the machine the document belongs to, used to tell + /// its own sync folder from one that travelled in from elsewhere. + public init?(bag: AppSettingsBag, deviceID: String) { + guard bag.keys.contains(where: { $0.hasPrefix(Key.prefix) }) else { return nil } + let bookmark = bag[string: Key.folderBookmark] + let owner = bag[string: Key.folderMachineID] + self.init( + // An unstamped bookmark counts as foreign: nothing has ever written + // one, so the only way to see one is a document this machine did not + // produce, and "cannot prove it is ours" must read as "not ours". + isConfigured: !(bookmark ?? "").isEmpty && owner == deviceID, + isEnabled: bag[bool: Key.enabled] ?? false, + lastSyncAt: bag[string: Key.lastSyncAt].flatMap(JSONCoders.parseDate) + ) + } +} diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/DocumentSyncStatusTests.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/DocumentSyncStatusTests.swift new file mode 100644 index 0000000..ce0b147 --- /dev/null +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/DocumentSyncStatusTests.swift @@ -0,0 +1,128 @@ +import XCTest +@testable import InterlinedDomain + +/// Reading the Document Sync Agent's state out of a machine's per-machine +/// settings document (GitHub issue #104), so Settings ▸ Applications can say +/// what a machine is doing rather than only that it exists. +/// +/// The keys are a wire contract with `DocumentSyncSettingsKeys` in the agent +/// package, which shares no code with this one — so these tests spell the key +/// strings out literally. A test that reused a constant from the same file it is +/// testing would pass just as happily after a rename that broke the contract. +final class DocumentSyncStatusTests: XCTestCase { + + private let deviceID = "mac-1" + + private func bag( + enabled: Bool = true, + bookmark: String? = "YmFzZTY0", + machineID: String? = "mac-1", + lastSyncAt: String? = "2026-09-16T19:39:07.135Z" + ) -> AppSettingsBag { + var bag = AppSettingsBag() + bag[bool: "documentSync.enabled"] = enabled + bag[string: "documentSync.folderBookmark"] = bookmark + bag[string: "documentSync.folderMachineId"] = machineID + bag[string: "documentSync.lastSyncAt"] = lastSyncAt + return bag + } + + // MARK: - Happy path + + func test_givenAMachineRunningTheAgent_whenReadingItsDocument_thenItReportsConfiguredAndOn() throws { + let status = try XCTUnwrap(DocumentSyncStatus(bag: bag(), deviceID: deviceID)) + + XCTAssertTrue(status.isConfigured) + XCTAssertTrue(status.isEnabled) + let lastSync = try XCTUnwrap(status.lastSyncAt) + XCTAssertEqual(lastSync.timeIntervalSince1970, 1_789_587_547.135, accuracy: 0.01) + } + + func test_givenAConfiguredButPausedMachine_whenReadingItsDocument_thenItIsConfiguredAndOff() throws { + let status = try XCTUnwrap( + DocumentSyncStatus(bag: bag(enabled: false), deviceID: deviceID) + ) + + // Configured and paused is a real, distinct state — the folder is set up, + // the user just switched syncing off. + XCTAssertTrue(status.isConfigured) + XCTAssertFalse(status.isEnabled) + } + + // MARK: - Invalid input + + func test_givenAFolderChosenOnAnotherMac_whenReadingTheDocument_thenItIsNotConfiguredHere() throws { + let status = try XCTUnwrap( + DocumentSyncStatus(bag: bag(machineID: "mac-2"), deviceID: deviceID) + ) + + // A security-scoped bookmark is meaningless anywhere but where it was + // made, so reporting it as configured would promise something untrue. + XCTAssertFalse(status.isConfigured) + } + + func test_givenABookmarkWithNoMachineStamp_whenReadingTheDocument_thenItIsNotConfiguredHere() throws { + let status = try XCTUnwrap( + DocumentSyncStatus(bag: bag(machineID: nil), deviceID: deviceID) + ) + + XCTAssertFalse(status.isConfigured) + } + + // MARK: - Upstream failure (a payload this build does not understand) + + func test_givenAnUnparseableTimestamp_whenReadingTheDocument_thenTheRestStillReports() throws { + let status = try XCTUnwrap( + DocumentSyncStatus(bag: bag(lastSyncAt: "yesterday"), deviceID: deviceID) + ) + + // The payload is client-owned and opaque to the server, so a future build + // may store something this one cannot read. One unreadable field must not + // take the whole pane down with it. + XCTAssertNil(status.lastSyncAt) + XCTAssertTrue(status.isConfigured) + } + + func test_givenAFieldOfTheWrongType_whenReadingTheDocument_thenItDegradesToUnset() throws { + var mixed = bag() + // A build that stored the flag as a string rather than a bool. + mixed[string: "documentSync.enabled"] = "yes" + + let status = try XCTUnwrap(DocumentSyncStatus(bag: mixed, deviceID: deviceID)) + + XCTAssertFalse(status.isEnabled) + XCTAssertTrue(status.isConfigured) + } + + // MARK: - Boundary + + func test_givenAMachineThatHasNeverRunTheAgent_whenReadingItsDocument_thenThereIsNothingToReport() { + var other = AppSettingsBag() + other[int: "sidebarWidth"] = 280 + + // Distinct from "installed but switched off" — the pane shows no Document + // Sync section at all rather than asserting the agent is off. + XCTAssertNil(DocumentSyncStatus(bag: other, deviceID: deviceID)) + } + + func test_givenAnEmptyDocument_whenReadingIt_thenThereIsNothingToReport() { + XCTAssertNil(DocumentSyncStatus(bag: AppSettingsBag(), deviceID: deviceID)) + } + + func test_givenTheAgentRanButNoFolderWasChosen_whenReadingTheDocument_thenItIsNotConfigured() throws { + let status = try XCTUnwrap( + DocumentSyncStatus(bag: bag(bookmark: nil), deviceID: deviceID) + ) + + XCTAssertFalse(status.isConfigured) + XCTAssertTrue(status.isEnabled) + } + + func test_givenAnEmptyBookmarkString_whenReadingTheDocument_thenItIsNotConfigured() throws { + let status = try XCTUnwrap( + DocumentSyncStatus(bag: bag(bookmark: ""), deviceID: deviceID) + ) + + XCTAssertFalse(status.isConfigured) + } +} diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/Auth/DeviceIDStore.swift b/Packages/InterlinedKit/Sources/InterlinedKit/Auth/DeviceIDStore.swift new file mode 100644 index 0000000..dfdb409 --- /dev/null +++ b/Packages/InterlinedKit/Sources/InterlinedKit/Auth/DeviceIDStore.swift @@ -0,0 +1,146 @@ +import Foundation +import Security +import os + +/// Publishes this Mac's app-settings device id to a place the bundled +/// document-sync agent can read it (GitHub issue #104). +/// +/// The agent keeps its configuration in per-machine app settings, which live at +/// `…/devices/{deviceId}/settings`. Addressing that document means the agent and +/// the app must agree on **one** device id per machine — two ids would produce +/// two rows in the registry for the same computer and split its settings in +/// half. +/// +/// They cannot simply share `UserDefaults`: the agent is a separate process with +/// its own bundle identifier (`com.interlinedlist.macos.sync`) and therefore its +/// own defaults domain. The shared Keychain access group is the channel that +/// already exists and already works — the agent reads the bearer token through +/// it — so the device id travels the same way rather than adding an app-group +/// entitlement, which would mean re-provisioning both signed bundles. +/// +/// The device id is **not a secret**; the Keychain is used here purely as the +/// cross-process store both sandboxes are entitled to. That is why this type is +/// separate from ``KeychainTokenStore`` despite the similar mechanics: the +/// handling rules for a bearer token and for an opaque machine id are not the +/// same, and sharing one type would invite treating them as if they were. +public protocol DeviceIDStoring: Sendable { + /// The published id, or nil when none has been written yet. + func read() -> String? + /// Publishes `id`, replacing any previous value. + func write(_ id: String) +} + +// MARK: - Keychain + +/// Shared-Keychain implementation. Deliberately non-throwing. +/// +/// A device id that cannot be shared is a degraded-but-working state, not a +/// failure: the app keeps its own copy in `UserDefaults` and the agent falls +/// back to local settings. Propagating a Keychain `OSStatus` out of here would +/// give every caller an error it can do nothing about, on a path that must +/// never block launch. +public struct KeychainDeviceIDStore: DeviceIDStoring { + + private let service: String + private let account: String + private let accessGroup: String? + private let logger = Logger( + subsystem: Bundle.main.bundleIdentifier ?? "com.interlinedlist.kit", + category: "DeviceIDStore" + ) + + /// - Parameters: + /// - service: `kSecAttrService`. **Must match the agent's + /// `SyncConfiguration.deviceIDService`** — the two processes are + /// independent codebases agreeing on one wire contract. + /// - accessGroup: the shared group both bundles list in + /// `keychain-access-groups`. + public init( + service: String = "com.interlinedlist.macos.device-id", + account: String = "default", + accessGroup: String? = nil + ) { + self.service = service + self.account = account + self.accessGroup = accessGroup + } + + /// `errSecMissingEntitlement` — returned when querying an access group the + /// process is not entitled to, which is the normal state for an ad-hoc + /// signed test host. Treated as a miss so tests and unsigned local builds + /// degrade to "nothing published" instead of failing. + private static let missingEntitlement: OSStatus = -34018 + + private func baseQuery() -> [CFString: Any] { + var query: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrService: service, + kSecAttrAccount: account + ] + if let accessGroup { query[kSecAttrAccessGroup] = accessGroup } + return query + } + + public func read() -> String? { + var query = baseQuery() + query[kSecMatchLimit] = kSecMatchLimitOne + query[kSecReturnData] = true + + var item: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &item) + switch status { + case errSecSuccess: + guard let data = item as? Data, + let id = String(data: data, encoding: .utf8), + !id.isEmpty else { return nil } + return id + case errSecItemNotFound, Self.missingEntitlement: + return nil + default: + logger.error("Device id read failed with OSStatus \(status, privacy: .public)") + return nil + } + } + + public func write(_ id: String) { + guard let data = id.data(using: .utf8) else { return } + let query = baseQuery() + + // Update first, then add. `SecItemAdd` on an existing item answers + // `errSecDuplicateItem` rather than replacing, so add-then-update would + // leave a stale id in place on every machine that already has one. + let update = SecItemUpdate(query as CFDictionary, [kSecValueData: data] as CFDictionary) + if update == errSecSuccess { return } + + var insert = query + insert[kSecValueData] = data + // The agent runs as a login item, so the item has to be readable before + // the user unlocks the screen for the first time after a reboot. + insert[kSecAttrAccessible] = kSecAttrAccessibleAfterFirstUnlock + let status = SecItemAdd(insert as CFDictionary, nil) + if status != errSecSuccess && status != Self.missingEntitlement { + logger.error("Device id write failed with OSStatus \(status, privacy: .public)") + } + } +} + +// MARK: - In-memory (tests + previews) + +/// In-memory implementation for unit tests and previews, where touching the +/// real Keychain is both undesirable and (unsigned) impossible. +public final class InMemoryDeviceIDStore: DeviceIDStoring, @unchecked Sendable { + private let lock = NSLock() + private var id: String? + + public init(initial: String? = nil) { self.id = initial } + + public func read() -> String? { + lock.lock(); defer { lock.unlock() } + return id + } + + public func write(_ id: String) { + lock.lock(); defer { lock.unlock() } + self.id = id + } +} diff --git a/SyncAgent/Sources/InterlinedListSyncCore/API/AppSettingsModels.swift b/SyncAgent/Sources/InterlinedListSyncCore/API/AppSettingsModels.swift new file mode 100644 index 0000000..32942e9 --- /dev/null +++ b/SyncAgent/Sources/InterlinedListSyncCore/API/AppSettingsModels.swift @@ -0,0 +1,169 @@ +import Foundation + +// Wire models for the **per-machine app-settings** routes the agent stores its +// configuration in (GitHub issue #104): +// +// GET /api/user/app-settings/{appKey}/devices/{deviceId}/settings +// PUT /api/user/app-settings/{appKey}/devices/{deviceId}/settings +// +// Independent of the main app's `InterlinedKit` DTOs, like every other model in +// this package — the agent is a clean-room implementation. Both sides were +// verified live against the same routes on 2026-09-16 (PR #102), so they agree +// because the wire says so, not because they share code. + +// MARK: - Opaque settings values + +/// A losslessly round-trippable JSON value. +/// +/// The server stores the settings payload **verbatim and uninterpreted**, which +/// means this agent shares one document with the main app. Decoding into a +/// concrete struct would drop every key this build does not know about — and +/// since the PUT *replaces* the document rather than merging it, the next write +/// would then delete the main app's per-machine settings outright. +/// +/// Keeping values opaque is what makes the overlay in +/// ``SyncAgentConfiguration/apply(to:)`` safe. +public enum SettingsValue: Codable, Sendable, Equatable { + case null + case bool(Bool) + case number(Double) + case string(String) + case array([SettingsValue]) + case object([String: SettingsValue]) + + public init(from decoder: any Decoder) throws { + let c = try decoder.singleValueContainer() + if c.decodeNil() { self = .null; return } + if let v = try? c.decode(Bool.self) { self = .bool(v); return } + if let v = try? c.decode(Double.self) { self = .number(v); return } + if let v = try? c.decode(String.self) { self = .string(v); return } + if let v = try? c.decode([SettingsValue].self) { self = .array(v); return } + if let v = try? c.decode([String: SettingsValue].self) { self = .object(v); return } + throw DecodingError.dataCorruptedError(in: c, debugDescription: "Unrecognised JSON value") + } + + public func encode(to encoder: any Encoder) throws { + var c = encoder.singleValueContainer() + switch self { + case .null: try c.encodeNil() + case .bool(let v): try c.encode(v) + case .number(let v): try c.encode(v) + case .string(let v): try c.encode(v) + case .array(let v): try c.encode(v) + case .object(let v): try c.encode(v) + } + } + + // Readers that answer nil rather than trapping when the stored value turns + // out to be another type — a settings blob is client-owned and a future + // build may legitimately change a field's shape. + public var boolValue: Bool? { if case .bool(let v) = self { return v }; return nil } + public var stringValue: String? { if case .string(let v) = self { return v }; return nil } + public var doubleValue: Double? { if case .number(let v) = self { return v }; return nil } +} + +// MARK: - Documents + +/// A stored settings document. +/// +/// Shape verified live 2026-09-16 (PR #102): the document comes back **bare**, +/// with no envelope. +/// +/// ``` +/// {"appKey":"interlinedlist-macos","scope":"device","deviceId":"…", +/// "version":3,"updatedAt":"2026-09-16T19:39:07.135Z","schemaVersion":1, +/// "settings":{…}} +/// ``` +/// +/// `version` cannot be optional: it is the `baseVersion` of the next write, and +/// a client that cannot read it cannot legally write at all. +public struct DeviceSettingsDocument: Decodable, Sendable, Equatable { + public let version: Int + public let updatedAt: Date? + /// The machine this document belongs to. Present on device-scoped + /// documents; nil on the account-wide one. + public let deviceId: String? + public let schemaVersion: Int? + /// Opaque, client-owned payload — see ``SettingsValue``. + public let settings: [String: SettingsValue] + + public init( + version: Int, + updatedAt: Date? = nil, + deviceId: String? = nil, + schemaVersion: Int? = nil, + settings: [String: SettingsValue] = [:] + ) { + self.version = version + self.updatedAt = updatedAt + self.deviceId = deviceId + self.schemaVersion = schemaVersion + self.settings = settings + } +} + +/// `PUT …/devices/{deviceId}/settings` body. +/// +/// **`baseVersion` is mandatory**, verified live: omitting it fails every write +/// with `400 {"error":"baseVersion must be an integer >= 0"}`. Send `0` to +/// create, or the `version` last read to update. A stale value answers `409` and +/// writes nothing — see ``APIError/versionConflict(current:)``. +public struct WriteDeviceSettingsBody: Encodable, Sendable, Equatable { + public let settings: [String: SettingsValue] + public let baseVersion: Int + public let schemaVersion: Int? + + public init(settings: [String: SettingsValue], baseVersion: Int, schemaVersion: Int? = nil) { + self.settings = settings + self.baseVersion = baseVersion + self.schemaVersion = schemaVersion + } +} + +/// `POST …/devices` body — register this machine. +/// +/// The agent sends this on exactly one path: a settings write that answered 404 +/// because the device is absent from the registry. See +/// ``DeviceSettingsAPI/registerDevice(deviceId:deviceName:)``. +public struct RegisterDeviceBody: Encodable, Sendable, Equatable { + public let deviceId: String + public let deviceName: String + public let platform: String + + public init(deviceId: String, deviceName: String, platform: String = "macos") { + self.deviceId = deviceId + self.deviceName = deviceName + self.platform = platform + } +} + +// MARK: - API surface + +/// The per-machine settings operations the agent needs. +/// +/// Separate from ``DocumentSyncAPI`` rather than bolted onto it: the sync engine +/// has no business with settings, and widening its protocol would force every +/// engine test double to implement routes it never calls. +public protocol DeviceSettingsAPI: Sendable { + /// This machine's stored document, or nil when it has never written one + /// (`404`, the ordinary first-run state). + func fetchDeviceSettings(deviceId: String) async throws -> DeviceSettingsDocument? + + /// Compare-and-set write. Throws ``APIError/versionConflict(current:)`` when + /// `baseVersion` is stale, and ``APIError/deviceNotRegistered`` when the + /// machine is absent from the registry. + func writeDeviceSettings( + deviceId: String, + settings: [String: SettingsValue], + baseVersion: Int + ) async throws -> DeviceSettingsDocument + + /// Adds this machine to the registry. + /// + /// Verified live 2026-09-16: `POST …/devices` is an **upsert keyed on + /// `deviceId`** — re-posting an existing id overwrites its `deviceName` + /// rather than duplicating the row. Calling it unconditionally would reset a + /// machine the user had renamed back to its hostname. The agent therefore + /// calls it only after a write proved the device absent. + func registerDevice(deviceId: String, deviceName: String) async throws +} diff --git a/SyncAgent/Sources/InterlinedListSyncCore/API/SyncAPIClient.swift b/SyncAgent/Sources/InterlinedListSyncCore/API/SyncAPIClient.swift index 6265ed8..71f830d 100644 --- a/SyncAgent/Sources/InterlinedListSyncCore/API/SyncAPIClient.swift +++ b/SyncAgent/Sources/InterlinedListSyncCore/API/SyncAPIClient.swift @@ -11,6 +11,23 @@ public enum APIError: Error, Sendable, Equatable { case transport(String) case decoding(String) case invalidResponse + /// A compare-and-set settings write lost the race: the stored document moved + /// on since it was read and **nothing was written**. + /// + /// `current` is the winning document, taken straight from the 409 body, so a + /// retry can re-base without spending another request. The main app cannot + /// do this — its `APIClient` reduces every non-2xx body to a message string — + /// but this client owns its error path end to end, and the agent writes from + /// the background where losing the race is likeliest. + case versionConflict(current: DeviceSettingsDocument?) + /// A per-device settings write was addressed to a machine that is not in the + /// registry. Retrying cannot fix it; registering the device can. + case deviceNotRegistered + /// The main app has not published a device id into the shared Keychain + /// group yet, so there is no per-machine document to address. Distinct from + /// ``deviceNotRegistered``: nothing is wrong, the app just has not launched + /// since this agent was installed. + case noDeviceIdentity } /// The document-sync operations the engine needs. A protocol so tests can @@ -29,7 +46,7 @@ public protocol DocumentSyncAPI: Sendable { /// URLSession-backed client for the InterlinedList Documents API. Reads the /// bearer token fresh on every request via the injected ``TokenProviding`` so a /// mid-run sign-in / sign-out in the main app is picked up immediately. -public actor SyncAPIClient: DocumentSyncAPI { +public actor SyncAPIClient: DocumentSyncAPI, DeviceSettingsAPI { private let baseURL: URL private let session: URLSession @@ -140,6 +157,82 @@ public actor SyncAPIClient: DocumentSyncAPI { } } + // MARK: - DeviceSettingsAPI (GitHub issue #104) + + private var deviceSettingsPathPrefix: String { + "/api/user/app-settings/\(pathEncode(SyncConfiguration.appSettingsKey))/devices" + } + + public func fetchDeviceSettings(deviceId: String) async throws -> DeviceSettingsDocument? { + let request = try makeRequest( + method: "GET", + path: "\(deviceSettingsPathPrefix)/\(pathEncode(deviceId))/settings" + ) + do { + return try await perform(request, as: DeviceSettingsDocument.self) + } catch APIError.notFound { + // A machine that has never written settings answers 404. That is the + // ordinary first-run state — the distinction the caller needs is + // "nothing stored" (nil) versus "could not ask" (a thrown error), + // because only the first one may trigger a migration. + return nil + } + } + + public func writeDeviceSettings( + deviceId: String, + settings: [String: SettingsValue], + baseVersion: Int + ) async throws -> DeviceSettingsDocument { + let body = WriteDeviceSettingsBody( + settings: settings, + baseVersion: baseVersion, + schemaVersion: SyncConfiguration.settingsSchemaVersion + ) + let request = try makeRequest( + method: "PUT", + path: "\(deviceSettingsPathPrefix)/\(pathEncode(deviceId))/settings", + jsonBody: body + ) + do { + return try await perform(request, as: DeviceSettingsDocument.self) + } catch APIError.notFound { + // 404 on the *write* route is not "no document yet" — `baseVersion: + // 0` creates one happily. It means the device is missing from the + // registry, which a retry will never fix. + throw APIError.deviceNotRegistered + } catch APIError.http(let status, let responseBody) where status == 409 { + throw APIError.versionConflict(current: Self.conflictDocument(from: responseBody)) + } + } + + public func registerDevice(deviceId: String, deviceName: String) async throws { + let request = try makeRequest( + method: "POST", + path: deviceSettingsPathPrefix, + jsonBody: RegisterDeviceBody(deviceId: deviceId, deviceName: deviceName) + ) + // The response wraps the device (`{"device":{…}}`) but the agent has no + // use for it: it registers only so the settings write that just failed + // can be retried. + _ = try await sendExpectingSuccess(request) + } + + /// Pulls the winning document out of a 409 body. + /// + /// ``` + /// 409 {"error":"version_conflict","code":"version_conflict","current":{…SettingsDoc…}} + /// ``` + /// + /// Best-effort by design: a conflict whose body cannot be parsed still has + /// to surface as a conflict, so the retry falls back to a fresh read rather + /// than the whole write failing on a decoding error. + static func conflictDocument(from body: String?) -> DeviceSettingsDocument? { + guard let data = body?.data(using: .utf8) else { return nil } + struct ConflictEnvelope: Decodable { let current: DeviceSettingsDocument? } + return try? JSONCoding.makeDecoder().decode(ConflictEnvelope.self, from: data).current + } + // MARK: - Request building private func makeRequest( diff --git a/SyncAgent/Sources/InterlinedListSyncCore/App/AppDelegate.swift b/SyncAgent/Sources/InterlinedListSyncCore/App/AppDelegate.swift index e73924a..2d901f6 100644 --- a/SyncAgent/Sources/InterlinedListSyncCore/App/AppDelegate.swift +++ b/SyncAgent/Sources/InterlinedListSyncCore/App/AppDelegate.swift @@ -9,12 +9,29 @@ public final class AppDelegate: NSObject, NSApplicationDelegate { private let logger = Logger(subsystem: SyncConfiguration.logSubsystem, category: "AppDelegate") - private let prefs = PreferencesManager() private let state = SyncStateModel() private let notifications = NotificationManager() private let loginItems: any LoginItemManaging = LoginItemManager() private let tokenStore = SharedTokenStore() + /// The agent's configuration lives in per-machine app settings + /// (GitHub issue #104); `UserDefaults` is the mirror it falls back to until + /// a write is confirmed. + /// + /// Its own `SharedTokenStore` and `SyncAPIClient`, not the engine's: a + /// property initialiser cannot reach `self`, and the two have different + /// lifetimes anyway — the engine's client is torn down and rebuilt whenever + /// the sync folder changes, while settings must stay writable throughout. + /// Both token stores are stateless readers of the same shared Keychain item, + /// so a mid-run sign-in or sign-out is still picked up by both. + private let prefs = PreferencesManager( + remote: RemoteConfigurationStore( + api: SyncAPIClient(tokenProvider: SharedTokenStore()), + identity: SharedDeviceIdentity(), + deviceName: DeviceNaming.suggestedName + ) + ) + private var engine: SyncEngine? private var statusItem: StatusItemController? private var eventTask: Task? @@ -27,8 +44,29 @@ public final class AppDelegate: NSObject, NSApplicationDelegate { statusItem = StatusItemController(state: state, actions: makeActions()) + // Reconcile with per-machine app settings *before* deciding whether this + // machine is configured (GitHub issue #104). The stored configuration + // can name a sync folder the local mirror has never heard of — a + // reinstall is exactly that case — so asking `hasSyncFolder` first would + // prompt the user to choose a folder they already chose. + // + // Signed out there is nothing to read, so the old local-only path runs + // unchanged rather than waiting on a request that cannot succeed. + if tokenStore.hasToken { + Task { [weak self] in + await self?.prefs.synchronize() + self?.startOrPrompt() + } + } else { + startOrPrompt() + } + } + + /// Starts the engine, or asks for a sync folder when this machine has none. + private func startOrPrompt() { if !prefs.hasSyncFolder { - // First run without a folder: let the user pick one. + // No folder here — either a genuine first run, or a configuration + // whose bookmark belongs to another Mac and was therefore refused. openPreferences() } else if tokenStore.hasToken { startEngine() @@ -90,6 +128,10 @@ public final class AppDelegate: NSObject, NSApplicationDelegate { state.apply(event) switch event { case .cycleCompleted(let summary): + // Report the cycle to per-machine app settings so Settings ▸ + // Applications can say what this machine is doing, not just that it + // exists. Throttled inside `recordSync`. + prefs.recordSync(at: summary.finishedAt) if prefs.notificationsEnabled && prefs.notifyOnCompletion { notifications.notifySyncCompleted(summary) } diff --git a/SyncAgent/Sources/InterlinedListSyncCore/Auth/SharedDeviceIdentity.swift b/SyncAgent/Sources/InterlinedListSyncCore/Auth/SharedDeviceIdentity.swift new file mode 100644 index 0000000..b041785 --- /dev/null +++ b/SyncAgent/Sources/InterlinedListSyncCore/Auth/SharedDeviceIdentity.swift @@ -0,0 +1,97 @@ +import Foundation +import Security +import os + +/// Supplies this machine's app-settings device id — the address of the +/// per-machine settings document the agent stores its configuration in +/// (GitHub issue #104). +public protocol DeviceIdentifying: Sendable { + /// The id for this machine, or nil when the main app has not published one. + func currentDeviceID() -> String? +} + +/// Reads the app-settings device id the **main app** publishes into the shared +/// Keychain access group. +/// +/// **Read-only, and that is the whole point.** The id keys a row in the server's +/// device registry; if this process minted its own when it found none, one Mac +/// would appear twice and its settings would split across two documents. The +/// app owns minting (`DeviceIdentity.current()`); the agent only ever consumes. +/// +/// No published id therefore means "per-machine settings are not addressable +/// yet", which the configuration store treats as +/// ``RemoteConfigurationState/unavailable`` — the agent keeps running off local +/// `UserDefaults` exactly as it did before this feature existed. +/// +/// Sharing contract (must match the main app's `KeychainDeviceIDStore`): +/// `kSecClassGenericPassword`, service ``SyncConfiguration/deviceIDService``, +/// account ``SyncConfiguration/deviceIDAccount``, access group +/// ``SyncConfiguration/sharedAccessGroup``. +public struct SharedDeviceIdentity: DeviceIdentifying { + + private let service: String + private let account: String + private let accessGroup: String? + private let logger = Logger(subsystem: SyncConfiguration.logSubsystem, category: "DeviceIdentity") + + public init( + service: String = SyncConfiguration.deviceIDService, + account: String = SyncConfiguration.deviceIDAccount, + accessGroup: String? = SyncConfiguration.sharedAccessGroup + ) { + self.service = service + self.account = account + self.accessGroup = accessGroup + } + + public func currentDeviceID() -> String? { + var query: [CFString: Any] = [ + kSecClass: kSecClassGenericPassword, + kSecAttrService: service, + kSecAttrAccount: account, + kSecMatchLimit: kSecMatchLimitOne, + kSecReturnData: true + ] + if let accessGroup { + query[kSecAttrAccessGroup] = accessGroup + } + + var item: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &item) + switch status { + case errSecSuccess: + guard let data = item as? Data, + let id = String(data: data, encoding: .utf8), + !id.isEmpty else { + logger.error("Device id item present but not decodable") + return nil + } + return id + case errSecItemNotFound: + return nil + default: + logger.error("Device id read failed: OSStatus \(status, privacy: .public)") + return nil + } + } +} + +/// A fixed in-memory device id, for tests and previews. +public struct StaticDeviceIdentity: DeviceIdentifying { + private let id: String? + public init(_ id: String?) { self.id = id } + public func currentDeviceID() -> String? { id } +} + +/// How this machine names itself when it has to add its own registry row. +/// +/// Mirrors the main app's `DeviceIdentity.suggestedName` so a machine registered +/// by either process gets the same label, and the user sees one name rather than +/// two spellings of the same Mac. +public enum DeviceNaming { + public static var suggestedName: String { + let host = ProcessInfo.processInfo.hostName + // `hostName` usually comes back as "studio-mac.local"; trim the suffix. + return host.hasSuffix(".local") ? String(host.dropLast(6)) : host + } +} diff --git a/SyncAgent/Sources/InterlinedListSyncCore/Configuration.swift b/SyncAgent/Sources/InterlinedListSyncCore/Configuration.swift index 0cc959c..a46e68c 100644 --- a/SyncAgent/Sources/InterlinedListSyncCore/Configuration.swift +++ b/SyncAgent/Sources/InterlinedListSyncCore/Configuration.swift @@ -26,6 +26,39 @@ public enum SyncConfiguration { /// Fully-qualified access group used in Keychain queries. public static let sharedAccessGroup = "\(teamIdentifier).\(sharedAccessGroupSuffix)" + /// `kSecAttrService` of the app-settings **device id** the main app + /// publishes into the same shared group (GitHub issue #104). + /// + /// Must match the main app's `DeviceIdentity.sharedStore`. The id is not a + /// secret; the Keychain is simply the cross-process channel both sandboxes + /// are already entitled to — the agent has its own bundle identifier and so + /// its own `UserDefaults` domain, which is why the value cannot just be a + /// shared preference. + public static let deviceIDService = "com.interlinedlist.macos.device-id" + /// `kSecAttrAccount` of that item. + public static let deviceIDAccount = "default" + + // MARK: - Per-machine app settings (GitHub issue #104) + + /// The app-settings namespace this client's settings live under. **Must not + /// change** — it is the key every stored setting is filed under, and a new + /// one orphans all of them. Matches the main app's + /// `AppEnvironment.appSettingsKey`. + public static let appSettingsKey = "interlinedlist-macos" + + /// Version stamped on the settings payload this build writes, so a future + /// build can tell what it is reading before it interprets it. + public static let settingsSchemaVersion = 1 + + /// How stale a published "last synced" timestamp is allowed to get. + /// + /// The timestamp is status, not configuration: it exists so Settings ▸ + /// Applications can say what a machine is actually doing rather than only + /// that it exists. Writing it on every poll cycle would mean a settings PUT + /// every minute per machine, forever, to move a value nobody is watching in + /// real time — so it rides an hourly throttle instead. + public static let lastSyncPublishInterval: TimeInterval = 3600 + // MARK: - Filesystem correlation /// Extended-attribute key holding the server document id on each `.md` file. diff --git a/SyncAgent/Sources/InterlinedListSyncCore/Storage/PreferencesManager.swift b/SyncAgent/Sources/InterlinedListSyncCore/Storage/PreferencesManager.swift index 8be5362..978818a 100644 --- a/SyncAgent/Sources/InterlinedListSyncCore/Storage/PreferencesManager.swift +++ b/SyncAgent/Sources/InterlinedListSyncCore/Storage/PreferencesManager.swift @@ -1,9 +1,26 @@ import Foundation import AppKit +import os -/// User-facing settings, persisted to `UserDefaults`, plus the security-scoped -/// bookmark for the sync folder. `@MainActor` because it is observed by SwiftUI -/// and drives `NSOpenPanel`. +/// User-facing settings plus the security-scoped bookmark for the sync folder. +/// `@MainActor` because it is observed by SwiftUI and drives `NSOpenPanel`. +/// +/// Two homes, one of them authoritative (GitHub issue #104): +/// +/// - **Per-machine app settings** (`…/devices/{deviceId}/settings`) are where +/// the configuration actually lives. That is what `/help/app-settings` +/// describes per-machine storage as being for — a value meaningful on one +/// machine only — and it means a reinstall recovers the user's setup instead +/// of starting from nothing. +/// - **`UserDefaults`** stays as the local mirror and the fallback. It is what +/// the agent runs on before the first successful remote write is confirmed, +/// while offline, and on a machine the main app has not yet published a device +/// id for. Every setter still writes it, so nothing in the agent has to care +/// whether the network is up. +/// +/// The bookmark is written to both, but it is only ever *resolved* from +/// `UserDefaults` — a bookmark stamped with another machine is dropped on the +/// way in, so what reaches local storage is always this Mac's own. @MainActor public final class PreferencesManager: ObservableObject { @@ -17,37 +34,67 @@ public final class PreferencesManager: ObservableObject { static let launchAtLogin = "launchAtLogin" static let folderBookmark = "syncFolderBookmark" static let folderPath = "syncFolderPath" + /// Set only once a migration write has been **confirmed** by the server. + /// Until then the agent keeps reading `UserDefaults`, which is the whole + /// point: a half-finished migration must not look like a finished one. + static let didMigrateToAppSettings = "didMigrateSettingsToAppSettings" + /// When the reported "last synced" value was last published, so the + /// hourly throttle survives a relaunch instead of firing a write on + /// every launch. + static let lastSyncPublishedAt = "lastSyncPublishedAt" } private let defaults: UserDefaults + private let remote: RemoteConfigurationStore? + private let logger = Logger(subsystem: SyncConfiguration.logSubsystem, category: "Preferences") private var accessingURL: URL? + /// Coalescing window for remote writes. Each toggle in the preferences + /// window is one `didSet`; without this, dragging the poll-interval slider + /// would issue a settings PUT per step. + private let remoteSaveDebounce: Duration + private var pendingSave: Task? + /// Suppresses the write-back that applying a freshly-read remote + /// configuration would otherwise trigger. + private var isApplyingRemote = false + @Published public var pollIntervalSeconds: TimeInterval { - didSet { defaults.set(pollIntervalSeconds, forKey: Key.pollInterval) } + didSet { defaults.set(pollIntervalSeconds, forKey: Key.pollInterval); scheduleRemoteSave() } } @Published public var syncEnabled: Bool { - didSet { defaults.set(syncEnabled, forKey: Key.syncEnabled) } + didSet { defaults.set(syncEnabled, forKey: Key.syncEnabled); scheduleRemoteSave() } } @Published public var notificationsEnabled: Bool { - didSet { defaults.set(notificationsEnabled, forKey: Key.notificationsEnabled) } + didSet { defaults.set(notificationsEnabled, forKey: Key.notificationsEnabled); scheduleRemoteSave() } } @Published public var notifyOnCompletion: Bool { - didSet { defaults.set(notifyOnCompletion, forKey: Key.notifyOnCompletion) } + didSet { defaults.set(notifyOnCompletion, forKey: Key.notifyOnCompletion); scheduleRemoteSave() } } @Published public var notifyOnErrors: Bool { - didSet { defaults.set(notifyOnErrors, forKey: Key.notifyOnErrors) } + didSet { defaults.set(notifyOnErrors, forKey: Key.notifyOnErrors); scheduleRemoteSave() } } @Published public var notifyOnConflicts: Bool { - didSet { defaults.set(notifyOnConflicts, forKey: Key.notifyOnConflicts) } + didSet { defaults.set(notifyOnConflicts, forKey: Key.notifyOnConflicts); scheduleRemoteSave() } } @Published public var launchAtLogin: Bool { - didSet { defaults.set(launchAtLogin, forKey: Key.launchAtLogin) } + didSet { defaults.set(launchAtLogin, forKey: Key.launchAtLogin); scheduleRemoteSave() } } /// Display-only path of the chosen folder (the source of truth is the bookmark). @Published public private(set) var syncFolderPath: String? - public init(defaults: UserDefaults = .standard) { + /// When the last completed sync cycle was reported to app settings. + /// Reporting only — the engine's own cursor lives in the on-disk ledger. + private var lastSyncAt: Date? + private var lastSyncPublishedAt: Date? + + public init( + defaults: UserDefaults = .standard, + remote: RemoteConfigurationStore? = nil, + remoteSaveDebounce: Duration = .seconds(2) + ) { self.defaults = defaults + self.remote = remote + self.remoteSaveDebounce = remoteSaveDebounce let stored = defaults.object(forKey: Key.pollInterval) as? TimeInterval self.pollIntervalSeconds = stored.map { min(max($0, SyncConfiguration.minPollInterval), SyncConfiguration.maxPollInterval) @@ -59,6 +106,143 @@ public final class PreferencesManager: ObservableObject { self.notifyOnConflicts = (defaults.object(forKey: Key.notifyOnConflicts) as? Bool) ?? true self.launchAtLogin = (defaults.object(forKey: Key.launchAtLogin) as? Bool) ?? false self.syncFolderPath = defaults.string(forKey: Key.folderPath) + self.lastSyncPublishedAt = defaults.object(forKey: Key.lastSyncPublishedAt) as? Date + } + + // MARK: - Per-machine app settings + + /// True once a configuration write to per-machine app settings has been + /// confirmed. Until then `UserDefaults` is the fallback the agent runs on. + public var hasMigratedToAppSettings: Bool { + defaults.bool(forKey: Key.didMigrateToAppSettings) + } + + /// Brings this machine's configuration in line with per-machine app + /// settings, migrating the local one up exactly once if the server has none. + /// + /// Call once at launch, before starting the engine: a remote configuration + /// may name a different sync folder than the local mirror does. + public func synchronize() async { + guard let remote, let machineID = await remote.currentDeviceID else { return } + + let state = await remote.load() + let resolution = ConfigurationResolver.resolve( + remote: state, + local: configuration(machineID: machineID), + hasMigrated: hasMigratedToAppSettings + ) + apply(resolution.configuration) + + if resolution.shouldMigrate { + await write(configuration(machineID: machineID), to: remote) + return + } + // Applying a stored configuration can legitimately leave this machine + // holding more than the server does — a folder chosen while offline is + // kept rather than cleared (see `apply`). Push the difference back so the + // two agree, instead of silently diverging until the next toggle. + let effective = configuration(machineID: machineID) + if case .stored(let stored) = state, stored != effective { + await write(effective, to: remote) + } + } + + /// Reports a completed sync cycle, throttled. + /// + /// Status, not configuration: Settings ▸ Applications shows it so a machine + /// can say what it is doing, and nothing reads it back. Writing it every + /// cycle would be a settings PUT a minute, per machine, forever. + /// + /// Returns the publishing task, or nil when the throttle swallowed the + /// report. Callers in the agent ignore it; tests await it, so the publish is + /// observable without sleeping on a timer. + @discardableResult + public func recordSync(at date: Date) -> Task? { + lastSyncAt = date + guard let remote else { return nil } + if let published = lastSyncPublishedAt, + date.timeIntervalSince(published) < SyncConfiguration.lastSyncPublishInterval { + return nil + } + lastSyncPublishedAt = date + defaults.set(date, forKey: Key.lastSyncPublishedAt) + return Task { [weak self] in + guard let self, let machineID = await remote.currentDeviceID else { return } + await self.write(self.configuration(machineID: machineID), to: remote) + } + } + + /// The current configuration as stored remotely. + func configuration(machineID: String) -> SyncAgentConfiguration { + SyncAgentConfiguration( + syncEnabled: syncEnabled, + pollIntervalSeconds: pollIntervalSeconds, + launchAtLogin: launchAtLogin, + notificationsEnabled: notificationsEnabled, + notifyOnCompletion: notifyOnCompletion, + notifyOnErrors: notifyOnErrors, + notifyOnConflicts: notifyOnConflicts, + folder: defaults.data(forKey: Key.folderBookmark).map { + SyncFolderReference( + bookmark: $0, + displayPath: defaults.string(forKey: Key.folderPath) ?? "", + machineID: machineID + ) + }, + lastSyncAt: lastSyncAt + ) + } + + /// Adopts a configuration read from per-machine app settings. + /// + /// The folder is only ever *added*, never cleared. A stored configuration + /// with no folder means one had not been chosen when it was written, or that + /// it belonged to another machine and was dropped on the way in — neither is + /// evidence the user unset anything, and there is no UI to unset a folder, + /// only to change it. Clearing would throw away a folder chosen while this + /// machine was offline. + func apply(_ configuration: SyncAgentConfiguration) { + isApplyingRemote = true + defer { isApplyingRemote = false } + + pollIntervalSeconds = configuration.pollIntervalSeconds + syncEnabled = configuration.syncEnabled + launchAtLogin = configuration.launchAtLogin + notificationsEnabled = configuration.notificationsEnabled + notifyOnCompletion = configuration.notifyOnCompletion + notifyOnErrors = configuration.notifyOnErrors + notifyOnConflicts = configuration.notifyOnConflicts + lastSyncAt = configuration.lastSyncAt ?? lastSyncAt + + if let folder = configuration.folder { + defaults.set(folder.bookmark, forKey: Key.folderBookmark) + defaults.set(folder.displayPath, forKey: Key.folderPath) + syncFolderPath = folder.displayPath + } + } + + private func write(_ configuration: SyncAgentConfiguration, to remote: RemoteConfigurationStore) async { + do { + try await remote.save(configuration) + defaults.set(true, forKey: Key.didMigrateToAppSettings) + } catch { + // Deliberately silent. The agent has no window open most of the + // time, and a failed settings write costs the user nothing: the + // local mirror is still authoritative until a write is confirmed, + // and the next change tries again. + logger.error("Per-machine settings write failed: \(error.localizedDescription, privacy: .public)") + } + } + + private func scheduleRemoteSave() { + guard !isApplyingRemote, let remote else { return } + pendingSave?.cancel() + pendingSave = Task { [weak self] in + guard let self else { return } + try? await Task.sleep(for: self.remoteSaveDebounce) + guard !Task.isCancelled, let machineID = await remote.currentDeviceID else { return } + await self.write(self.configuration(machineID: machineID), to: remote) + } } // MARK: - Sync folder @@ -84,6 +268,10 @@ public final class PreferencesManager: ObservableObject { } defaults.set(url.path, forKey: Key.folderPath) syncFolderPath = url.path + // Not a `@Published` setter, so it has no `didSet` to ride on — and a + // folder change is the single most valuable thing to get stored + // remotely, since it is what a reinstall needs back. + scheduleRemoteSave() } /// Resolves the stored bookmark and begins accessing it. Returns the URL, or diff --git a/SyncAgent/Sources/InterlinedListSyncCore/Storage/RemoteConfigurationStore.swift b/SyncAgent/Sources/InterlinedListSyncCore/Storage/RemoteConfigurationStore.swift new file mode 100644 index 0000000..ef71189 --- /dev/null +++ b/SyncAgent/Sources/InterlinedListSyncCore/Storage/RemoteConfigurationStore.swift @@ -0,0 +1,223 @@ +import Foundation +import os + +/// What a read of this machine's per-machine settings found. +/// +/// The three cases are not interchangeable, and conflating two of them is the +/// bug GitHub issue #104 warns about. "Nothing stored" invites a one-time +/// migration; "could not ask" must never invite anything, because the document +/// on the server may be newer than the local values and re-uploading them would +/// destroy it. +public enum RemoteConfigurationState: Sendable, Equatable { + /// The server holds a configuration written by this agent. + case stored(SyncAgentConfiguration) + /// The server holds no configuration for this machine — a fresh install. + case absent + /// The question could not be asked: no device id published yet, no token, + /// or the request failed. + case unavailable +} + +/// Which configuration wins at launch, and whether the one-way migration should +/// run. +public struct ConfigurationResolution: Sendable, Equatable { + public let configuration: SyncAgentConfiguration + public let shouldMigrate: Bool + + public init(configuration: SyncAgentConfiguration, shouldMigrate: Bool) { + self.configuration = configuration + self.shouldMigrate = shouldMigrate + } +} + +/// The launch-time decision, extracted as a pure function. +/// +/// It is four lines of logic guarding the one thing in this feature that can +/// lose a user's settings for good, so it is worth being able to test without a +/// network, a Keychain, or a clock. +public enum ConfigurationResolver { + + /// - Parameters: + /// - remote: what the server said. + /// - local: the `UserDefaults` configuration this machine has been using. + /// - hasMigrated: whether a migration write has already been *confirmed*. + public static func resolve( + remote: RemoteConfigurationState, + local: SyncAgentConfiguration, + hasMigrated: Bool + ) -> ConfigurationResolution { + switch remote { + case .stored(let configuration): + // Once a document exists it is authoritative, full stop. It may have + // been written minutes ago by this same machine, or by a user who + // reinstalled and expects their folder back. Merging it with local + // values would mean choosing a winner field by field with no + // timestamps to choose by. + return ConfigurationResolution(configuration: configuration, shouldMigrate: false) + + case .absent where !hasMigrated: + // The one moment migration is safe: the server has nothing, so + // nothing can be overwritten. + return ConfigurationResolution(configuration: local, shouldMigrate: true) + + case .absent: + // Already migrated once and the document is gone — the machine was + // deregistered, or the settings were deleted deliberately. Writing + // the local values back would resurrect a configuration the user + // removed. The agent keeps running on them locally; the next change + // the user makes will store them again as a deliberate act. + return ConfigurationResolution(configuration: local, shouldMigrate: false) + + case .unavailable: + // Offline, signed out, or not yet addressable. Carry on locally and + // decide nothing — the server's answer is simply not in evidence. + return ConfigurationResolution(configuration: local, shouldMigrate: false) + } + } +} + +/// Reads and writes the agent's configuration in per-machine app settings +/// (`…/devices/{deviceId}/settings`, GitHub issue #104). +/// +/// **Per-machine, never shared.** The account-wide document seeds a brand-new +/// machine on first sign-in, so a sync-folder path stored there would arrive +/// pre-filled on a Mac where it means nothing — the exact failure this feature +/// exists to prevent. +/// +/// An actor because the agent writes from wherever a preference changed and from +/// the sync loop's completion handler, and the cached `baseVersion` must not be +/// read by one of those while the other is replacing it. +public actor RemoteConfigurationStore { + + private let api: any DeviceSettingsAPI + private let identity: any DeviceIdentifying + /// Name to register this machine under if it turns out not to be in the + /// registry. Hostname-derived, matching what the main app would use. + private let deviceName: String + private let logger = Logger(subsystem: SyncConfiguration.logSubsystem, category: "RemoteConfig") + + /// The document as last seen, so a write can overlay rather than replace and + /// can send a `baseVersion` the server will accept. + private var cachedPayload: [String: SettingsValue] = [:] + private var cachedVersion = 0 + private var hasReadDocument = false + + /// Attempts a single `save` will make before giving up. + /// + /// Bounded on purpose. A 409 is re-based and retried rather than surfaced — + /// the agent runs in the background, where there is no one to show a failure + /// toast to — but an unbounded retry against a document another process is + /// rewriting in a loop would turn a lost race into a request storm. + private static let maxSaveAttempts = 3 + + public init( + api: any DeviceSettingsAPI, + identity: any DeviceIdentifying, + deviceName: String + ) { + self.api = api + self.identity = identity + self.deviceName = deviceName + } + + /// This machine's device id, or nil when the main app has not published one + /// into the shared Keychain group yet — in which case there is no + /// per-machine document to address and the agent stays on local settings. + public var currentDeviceID: String? { identity.currentDeviceID() } + + // MARK: - Reading + + public func load() async -> RemoteConfigurationState { + guard let deviceID = identity.currentDeviceID() else { + // The main app mints and publishes the id; the agent only consumes + // it (see `SharedDeviceIdentity`). Until it appears there is no + // document to address, which is "cannot ask", not "nothing stored". + logger.info("No device id published yet; per-machine settings unavailable") + return .unavailable + } + do { + guard let document = try await api.fetchDeviceSettings(deviceId: deviceID) else { + cachedPayload = [:] + cachedVersion = 0 + hasReadDocument = true + return .absent + } + cache(document) + guard let configuration = SyncAgentConfiguration( + settings: document.settings, + thisMachineID: deviceID + ) else { + // A document exists but holds none of this agent's keys — the + // main app stored per-machine settings of its own. Absent for + // our purposes, and the cached payload means the migration write + // will preserve those keys. + return .absent + } + return .stored(configuration) + } catch { + logger.error("Per-machine settings read failed: \(error.localizedDescription, privacy: .public)") + return .unavailable + } + } + + // MARK: - Writing + + /// Stores `configuration`, re-basing and retrying if the compare-and-set + /// write loses. + /// + /// Throws only when the write genuinely cannot be completed; the caller + /// treats that as "stay on local settings" rather than as an error to show. + public func save(_ configuration: SyncAgentConfiguration) async throws { + guard let deviceID = identity.currentDeviceID() else { throw APIError.noDeviceIdentity } + + // Read before the first write so the overlay has the real document to + // preserve. Skipping this and writing with `baseVersion: 0` would 409 + // anyway — but only after the payload had already been built from + // nothing, which is how the main app's keys would get dropped. + if !hasReadDocument { + _ = await load() + } + + var didRegister = false + var attempt = 0 + while true { + attempt += 1 + do { + let document = try await api.writeDeviceSettings( + deviceId: deviceID, + settings: configuration.apply(to: cachedPayload), + baseVersion: cachedVersion + ) + cache(document) + return + } catch APIError.versionConflict(let current) { + guard attempt < Self.maxSaveAttempts else { throw APIError.versionConflict(current: current) } + if let current { + // The 409 body carries the winning document, so re-basing + // costs nothing. This is why the agent's client keeps typed + // error payloads where the main app's does not. + cache(current) + } else if let refreshed = try? await api.fetchDeviceSettings(deviceId: deviceID) { + cache(refreshed) + } else { + throw APIError.versionConflict(current: nil) + } + } catch APIError.deviceNotRegistered { + // Registering is safe *here specifically*: the 404 proves the id + // is absent from the registry, so the upsert has no existing + // `deviceName` to clobber. Once only — a second 404 after a + // successful register is a server-side problem retrying cannot + // solve. + guard !didRegister else { throw APIError.deviceNotRegistered } + didRegister = true + try await api.registerDevice(deviceId: deviceID, deviceName: deviceName) + } + } + } + + private func cache(_ document: DeviceSettingsDocument) { + cachedPayload = document.settings + cachedVersion = document.version + hasReadDocument = true + } +} diff --git a/SyncAgent/Sources/InterlinedListSyncCore/Storage/SyncAgentConfiguration.swift b/SyncAgent/Sources/InterlinedListSyncCore/Storage/SyncAgentConfiguration.swift new file mode 100644 index 0000000..5c0d669 --- /dev/null +++ b/SyncAgent/Sources/InterlinedListSyncCore/Storage/SyncAgentConfiguration.swift @@ -0,0 +1,223 @@ +import Foundation + +/// The keys the agent's configuration occupies inside a per-machine app-settings +/// document (GitHub issue #104). +/// +/// **This list is a wire contract with the main app.** `InterlinedDomain`'s +/// `DocumentSyncStatus` reads the same keys out of the same document so +/// Settings ▸ Applications can report what a machine is doing. The two are +/// independent codebases — this package deliberately does not depend on +/// `InterlinedKit`/`InterlinedDomain` — so a rename here without a matching +/// rename there silently stops the pane reporting anything. Change both. +/// +/// Every key is prefixed. The document is **shared** with the main app's own +/// per-machine settings, and the PUT replaces it wholesale, so an unprefixed +/// `enabled` would be one careless commit away from meaning two things. +public enum DocumentSyncSettingsKeys { + /// Prefix identifying keys this agent owns. Everything else in the document + /// belongs to someone else and must survive our writes untouched. + public static let prefix = "documentSync." + + public static let schemaVersion = prefix + "schemaVersion" + public static let enabled = prefix + "enabled" + public static let pollIntervalSeconds = prefix + "pollIntervalSeconds" + public static let launchAtLogin = prefix + "launchAtLogin" + public static let notificationsEnabled = prefix + "notificationsEnabled" + public static let notifyOnCompletion = prefix + "notifyOnCompletion" + public static let notifyOnErrors = prefix + "notifyOnErrors" + public static let notifyOnConflicts = prefix + "notifyOnConflicts" + /// Human-readable path of the sync folder. **Display only** — a sandboxed + /// process cannot reopen a user-chosen folder from a path string. + public static let folderPath = prefix + "folderPath" + /// Base64 of the security-scoped bookmark. The actual key to the folder. + public static let folderBookmark = prefix + "folderBookmark" + /// The device id of the Mac that created the bookmark. + public static let folderMachineID = prefix + "folderMachineId" + /// ISO-8601 timestamp of the last completed sync cycle. Status, not + /// configuration — see ``SyncConfiguration/lastSyncPublishInterval``. + public static let lastSyncAt = prefix + "lastSyncAt" +} + +// MARK: - The sync folder + +/// A chosen sync folder: the bookmark that actually opens it, the path to show +/// a human, and the machine both belong to. +/// +/// The machine stamp is not redundant. Per-machine settings are *meant* to stay +/// on one Mac, but two documented paths move a document across machines anyway: +/// Settings ▸ Applications can copy a machine's settings to the account-wide +/// document, and a brand-new machine's first sign-in seeds from the main +/// workstation. Either one can hand this Mac a bookmark another Mac created. +/// +/// Resolving a foreign bookmark does not fail cleanly — at best it resolves to a +/// path that does not exist here, at worst to an unrelated folder with the same +/// name, which the agent would then start writing documents into. So a foreign +/// reference is dropped on read and the machine reports itself unconfigured, +/// which is both true and recoverable: the user picks a folder. +public struct SyncFolderReference: Sendable, Equatable { + /// Security-scoped bookmark data. Only meaningful on ``machineID``. + public let bookmark: Data + /// Where the folder lives, for display. Never used to open anything. + public let displayPath: String + /// The device id of the Mac this bookmark was created on. + public let machineID: String + + public init(bookmark: Data, displayPath: String, machineID: String) { + self.bookmark = bookmark + self.displayPath = displayPath + self.machineID = machineID + } +} + +// MARK: - The configuration + +/// Everything the Document Sync Agent needs to know about itself, in the shape +/// it is stored in per-machine app settings. +/// +/// This is the complete set of state the agent used to keep in `UserDefaults`, +/// enumerated from `PreferencesManager` rather than guessed. Two things named in +/// GitHub issue #104 are deliberately **not** here: +/// +/// - **The sync ledger's cursor.** `LedgerData.lastSyncAt` is the delta cursor, +/// and it is only meaningful next to the per-document entries stored in the +/// same file. Uploading the cursor alone would let a machine resume from a +/// cursor its ledger cannot account for, and conclude nothing had changed. +/// ``lastSyncAt`` here is a *report* of when the last cycle finished, read by +/// Settings ▸ Applications and never fed back into the engine. +/// - **A conflict policy.** There is not one to move: ``ConflictResolver`` is a +/// fixed remote-wins decision table with no user-facing setting. Inventing a +/// stored policy to satisfy the migration would create a setting nothing +/// reads. +public struct SyncAgentConfiguration: Sendable, Equatable { + + public var syncEnabled: Bool + public var pollIntervalSeconds: TimeInterval + public var launchAtLogin: Bool + public var notificationsEnabled: Bool + public var notifyOnCompletion: Bool + public var notifyOnErrors: Bool + public var notifyOnConflicts: Bool + /// The chosen folder, or nil when this machine has none. + public var folder: SyncFolderReference? + /// When the last sync cycle finished, for reporting only. + public var lastSyncAt: Date? + + public init( + syncEnabled: Bool = true, + pollIntervalSeconds: TimeInterval = SyncConfiguration.defaultPollInterval, + launchAtLogin: Bool = false, + notificationsEnabled: Bool = true, + notifyOnCompletion: Bool = false, + notifyOnErrors: Bool = true, + notifyOnConflicts: Bool = true, + folder: SyncFolderReference? = nil, + lastSyncAt: Date? = nil + ) { + self.syncEnabled = syncEnabled + self.pollIntervalSeconds = pollIntervalSeconds + self.launchAtLogin = launchAtLogin + self.notificationsEnabled = notificationsEnabled + self.notifyOnCompletion = notifyOnCompletion + self.notifyOnErrors = notifyOnErrors + self.notifyOnConflicts = notifyOnConflicts + self.folder = folder + self.lastSyncAt = lastSyncAt + } + + // MARK: - Reading a stored document + + /// Projects the agent's configuration out of a settings payload. + /// + /// Returns nil when the payload holds none of this agent's keys — the main + /// app may well have written per-machine settings of its own, but a document + /// without a `documentSync.` key has never been written by the agent, and + /// treating it as a stored-but-default configuration would suppress the + /// migration that is supposed to run exactly once. + /// + /// - Parameter thisMachineID: the device id of the Mac doing the reading. + /// A folder stamped with any other machine is dropped; see + /// ``SyncFolderReference``. + public init?(settings: [String: SettingsValue], thisMachineID: String) { + let keys = DocumentSyncSettingsKeys.self + guard settings.keys.contains(where: { $0.hasPrefix(keys.prefix) }) else { return nil } + + let defaults = SyncAgentConfiguration() + self.init( + syncEnabled: settings[keys.enabled]?.boolValue ?? defaults.syncEnabled, + // Clamped on read, not just on write: a value stored by a build with + // different bounds must not make this one poll every second. + pollIntervalSeconds: (settings[keys.pollIntervalSeconds]?.doubleValue) + .map { min(max($0, SyncConfiguration.minPollInterval), SyncConfiguration.maxPollInterval) } + ?? defaults.pollIntervalSeconds, + launchAtLogin: settings[keys.launchAtLogin]?.boolValue ?? defaults.launchAtLogin, + notificationsEnabled: settings[keys.notificationsEnabled]?.boolValue ?? defaults.notificationsEnabled, + notifyOnCompletion: settings[keys.notifyOnCompletion]?.boolValue ?? defaults.notifyOnCompletion, + notifyOnErrors: settings[keys.notifyOnErrors]?.boolValue ?? defaults.notifyOnErrors, + notifyOnConflicts: settings[keys.notifyOnConflicts]?.boolValue ?? defaults.notifyOnConflicts, + folder: Self.folder(from: settings, thisMachineID: thisMachineID), + lastSyncAt: settings[keys.lastSyncAt]?.stringValue.flatMap(JSONCoding.parseISO8601) + ) + } + + private static func folder( + from settings: [String: SettingsValue], + thisMachineID: String + ) -> SyncFolderReference? { + let keys = DocumentSyncSettingsKeys.self + guard let encoded = settings[keys.folderBookmark]?.stringValue, + let bookmark = Data(base64Encoded: encoded), + !bookmark.isEmpty + else { return nil } + + // An unstamped bookmark is treated as foreign too. Nothing has ever + // written one, so the only way to see one is a document this agent did + // not produce — and "cannot prove it is ours" has to read as "not ours", + // because the failure it prevents is syncing into the wrong folder. + guard let machineID = settings[keys.folderMachineID]?.stringValue, + machineID == thisMachineID + else { return nil } + + return SyncFolderReference( + bookmark: bookmark, + displayPath: settings[keys.folderPath]?.stringValue ?? "", + machineID: machineID + ) + } + + // MARK: - Writing a stored document + + /// Overlays this configuration onto an existing settings payload. + /// + /// The PUT **replaces** the document rather than merging it (verified live: + /// a write carrying only `theme` deleted a stored `sidebarWidth`), so the + /// payload handed to it has to be the whole document. Keys outside the + /// `documentSync.` namespace are copied through untouched — they belong to + /// the main app, or to a newer build of this one, and dropping them would + /// make every agent write silently delete settings it never owned. + /// + /// Keys inside the namespace are rebuilt from scratch, so unsetting a folder + /// removes its keys instead of leaving a stale bookmark behind. + public func apply(to existing: [String: SettingsValue]) -> [String: SettingsValue] { + let keys = DocumentSyncSettingsKeys.self + var payload = existing.filter { !$0.key.hasPrefix(keys.prefix) } + + payload[keys.schemaVersion] = .number(Double(SyncConfiguration.settingsSchemaVersion)) + payload[keys.enabled] = .bool(syncEnabled) + payload[keys.pollIntervalSeconds] = .number(pollIntervalSeconds) + payload[keys.launchAtLogin] = .bool(launchAtLogin) + payload[keys.notificationsEnabled] = .bool(notificationsEnabled) + payload[keys.notifyOnCompletion] = .bool(notifyOnCompletion) + payload[keys.notifyOnErrors] = .bool(notifyOnErrors) + payload[keys.notifyOnConflicts] = .bool(notifyOnConflicts) + + if let folder { + payload[keys.folderBookmark] = .string(folder.bookmark.base64EncodedString()) + payload[keys.folderPath] = .string(folder.displayPath) + payload[keys.folderMachineID] = .string(folder.machineID) + } + if let lastSyncAt { + payload[keys.lastSyncAt] = .string(JSONCoding.iso8601String(lastSyncAt)) + } + return payload + } +} diff --git a/SyncAgent/Tests/InterlinedListSyncCoreTests/FakeDeviceSettingsAPI.swift b/SyncAgent/Tests/InterlinedListSyncCoreTests/FakeDeviceSettingsAPI.swift new file mode 100644 index 0000000..588da1d --- /dev/null +++ b/SyncAgent/Tests/InterlinedListSyncCoreTests/FakeDeviceSettingsAPI.swift @@ -0,0 +1,81 @@ +import Foundation +@testable import InterlinedListSyncCore + +/// In-memory stand-in for the per-machine app-settings routes (GitHub issue +/// #104), with the server's real compare-and-set semantics: a write carrying a +/// `baseVersion` other than the stored one is rejected with the winning document +/// attached, exactly as the live `409 version_conflict` body does. +/// +/// Modelling the conflict rather than stubbing it matters here — the whole point +/// of the store's retry is that it re-bases onto a document it did not write. +actor FakeDeviceSettingsAPI: DeviceSettingsAPI { + + struct WriteRecord: Sendable, Equatable { + let deviceId: String + let settings: [String: SettingsValue] + let baseVersion: Int + } + + private(set) var reads: [String] = [] + private(set) var writes: [WriteRecord] = [] + private(set) var registrations: [String] = [] + + private var document: DeviceSettingsDocument? + private var readError: APIError? + /// Errors to throw from the next writes, consumed one per attempt. + private var writeFailures: [APIError] = [] + + // MARK: - Test control + + func seed(settings: [String: SettingsValue], version: Int, deviceId: String = "mac-1") { + document = DeviceSettingsDocument( + version: version, + updatedAt: Date(timeIntervalSince1970: 1_700_000_000), + deviceId: deviceId, + schemaVersion: 1, + settings: settings + ) + } + + func failReads(with error: APIError) { readError = error } + func failNextWrites(with errors: [APIError]) { writeFailures = errors } + + func storedSettings() -> [String: SettingsValue] { document?.settings ?? [:] } + func storedVersion() -> Int { document?.version ?? 0 } + + // MARK: - DeviceSettingsAPI + + func fetchDeviceSettings(deviceId: String) async throws -> DeviceSettingsDocument? { + reads.append(deviceId) + if let readError { throw readError } + return document + } + + func writeDeviceSettings( + deviceId: String, + settings: [String: SettingsValue], + baseVersion: Int + ) async throws -> DeviceSettingsDocument { + writes.append(WriteRecord(deviceId: deviceId, settings: settings, baseVersion: baseVersion)) + if !writeFailures.isEmpty { + throw writeFailures.removeFirst() + } + let currentVersion = document?.version ?? 0 + guard baseVersion == currentVersion else { + throw APIError.versionConflict(current: document) + } + let updated = DeviceSettingsDocument( + version: currentVersion + 1, + updatedAt: Date(timeIntervalSince1970: 1_700_000_100), + deviceId: deviceId, + schemaVersion: 1, + settings: settings + ) + document = updated + return updated + } + + func registerDevice(deviceId: String, deviceName: String) async throws { + registrations.append(deviceId) + } +} diff --git a/SyncAgent/Tests/InterlinedListSyncCoreTests/PreferencesManagerTests.swift b/SyncAgent/Tests/InterlinedListSyncCoreTests/PreferencesManagerTests.swift index 4630314..3bf8ae6 100644 --- a/SyncAgent/Tests/InterlinedListSyncCoreTests/PreferencesManagerTests.swift +++ b/SyncAgent/Tests/InterlinedListSyncCoreTests/PreferencesManagerTests.swift @@ -4,11 +4,30 @@ import XCTest @MainActor final class PreferencesManagerTests: XCTestCase { + private let deviceID = "mac-1" + private func freshDefaults() -> UserDefaults { let suite = "iltest-prefs-\(UUID().uuidString)" return UserDefaults(suiteName: suite)! } + /// Debounced saves are parked far enough out that no test observes one. The + /// writes these tests assert on are the explicit ones `synchronize()` makes; + /// a stray coalesced write would make the counts meaningless. + private func makeManager( + defaults: UserDefaults, + api: FakeDeviceSettingsAPI, + identity: any DeviceIdentifying + ) -> PreferencesManager { + PreferencesManager( + defaults: defaults, + remote: RemoteConfigurationStore(api: api, identity: identity, deviceName: "Studio Mac"), + remoteSaveDebounce: .seconds(3600) + ) + } + + // MARK: - Local behaviour (unchanged by the move to app settings) + func test_defaults_areSensible() { let prefs = PreferencesManager(defaults: freshDefaults()) XCTAssertEqual(prefs.pollIntervalSeconds, SyncConfiguration.defaultPollInterval) @@ -36,4 +55,222 @@ final class PreferencesManagerTests: XCTestCase { XCTAssertTrue(reloaded.notifyOnCompletion) XCTAssertTrue(reloaded.launchAtLogin) } + + // MARK: - Happy path: adopting stored per-machine settings + + func test_givenStoredSettingsForThisMac_whenSynchronizing_thenTheyReplaceTheLocalOnes() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let stored = SyncAgentConfiguration( + syncEnabled: false, + pollIntervalSeconds: 300, + notifyOnCompletion: true, + folder: SyncFolderReference( + bookmark: Data("bm".utf8), + displayPath: "/Users/someone/Vault", + machineID: deviceID + ) + ) + await api.seed(settings: stored.apply(to: [:]), version: 2) + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + + await prefs.synchronize() + + XCTAssertEqual(prefs.pollIntervalSeconds, 300) + XCTAssertFalse(prefs.syncEnabled) + XCTAssertTrue(prefs.notifyOnCompletion) + // The bookmark reaches local storage, which is the only place the agent + // ever resolves it from — so a reinstall recovers the folder. + XCTAssertTrue(prefs.hasSyncFolder) + XCTAssertEqual(prefs.syncFolderPath, "/Users/someone/Vault") + } + + // MARK: - Migration + + func test_givenLocalSettingsAndAnEmptyServer_whenSynchronizing_thenTheyMigrateUpAndAreMarkedDone() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + prefs.pollIntervalSeconds = 180 + prefs.notifyOnErrors = false + + await prefs.synchronize() + + let stored = await api.storedSettings() + XCTAssertEqual(stored[DocumentSyncSettingsKeys.pollIntervalSeconds]?.doubleValue, 180) + XCTAssertEqual(stored[DocumentSyncSettingsKeys.notifyOnErrors]?.boolValue, false) + XCTAssertTrue(prefs.hasMigratedToAppSettings) + } + + func test_givenAFreshInstallWithNoLocalSettings_whenSynchronizing_thenTheDefaultsAreStoredOnce() async { + // The boundary GitHub issue #104 calls out: `UserDefaults` is empty, so + // there is nothing to migrate but the defaults themselves — and the + // migration must still be recorded, or it would run again every launch. + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + + await prefs.synchronize() + + let writes = await api.writes + XCTAssertEqual(writes.count, 1) + let stored = await api.storedSettings() + XCTAssertEqual( + stored[DocumentSyncSettingsKeys.pollIntervalSeconds]?.doubleValue, + SyncConfiguration.defaultPollInterval + ) + XCTAssertTrue(prefs.hasMigratedToAppSettings) + } + + func test_givenTheMigrationAlreadyRan_whenSynchronizingAgain_thenNothingIsWrittenBack() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + await prefs.synchronize() + let writesAfterMigration = await api.writes.count + + // A second launch with the document now stored. It must read, adopt, and + // stop — a migration that re-ran every launch would be worse than none. + await prefs.synchronize() + + let writesAfterSecondLaunch = await api.writes.count + XCTAssertEqual(writesAfterSecondLaunch, writesAfterMigration) + } + + // MARK: - Upstream failure + + func test_givenTheServerCannotBeReached_whenSynchronizing_thenLocalSettingsStandAndNothingIsMigrated() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + await api.failReads(with: .transport("offline")) + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + prefs.pollIntervalSeconds = 240 + + await prefs.synchronize() + + XCTAssertEqual(prefs.pollIntervalSeconds, 240) + let writes = await api.writes + XCTAssertTrue(writes.isEmpty, "A failed read must never be read as an empty server") + XCTAssertFalse(prefs.hasMigratedToAppSettings) + } + + func test_givenTheMigrationWriteFails_whenSynchronizing_thenItIsNotMarkedDone() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + await api.failNextWrites(with: [.transport("offline"), .transport("offline"), .transport("offline")]) + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + + await prefs.synchronize() + + // Unconfirmed means unmigrated: `UserDefaults` stays the fallback and + // the next launch tries again. + XCTAssertFalse(prefs.hasMigratedToAppSettings) + } + + // MARK: - Boundary: a folder that belongs to another Mac + + func test_givenTheStoredFolderBelongsToAnotherMac_whenSynchronizing_thenThisMacIsNotConfigured() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let fromAnotherMac = SyncAgentConfiguration( + pollIntervalSeconds: 300, + folder: SyncFolderReference( + bookmark: Data("bm".utf8), + displayPath: "/Volumes/OtherMac/Vault", + machineID: "mac-2" + ) + ) + await api.seed(settings: fromAnotherMac.apply(to: [:]), version: 1) + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + + await prefs.synchronize() + + // The rest of the configuration still applies; only the folder is + // refused, and the agent will prompt for one. + XCTAssertEqual(prefs.pollIntervalSeconds, 300) + XCTAssertFalse(prefs.hasSyncFolder) + XCTAssertNil(prefs.syncFolderPath) + } + + func test_givenTheServerHasNoFolderButThisMacDoes_whenSynchronizing_thenTheLocalFolderSurvives() async { + // A folder chosen while offline. The stored configuration predates it, + // and there is no UI for unsetting a folder — so "no folder stored" is + // never evidence the user cleared one. + let defaults = freshDefaults() + defaults.set(Data("local-bm".utf8), forKey: "syncFolderBookmark") + defaults.set("/Users/someone/Local", forKey: "syncFolderPath") + let api = FakeDeviceSettingsAPI() + await api.seed( + settings: SyncAgentConfiguration(pollIntervalSeconds: 120).apply(to: [:]), + version: 1 + ) + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + + await prefs.synchronize() + + XCTAssertTrue(prefs.hasSyncFolder) + XCTAssertEqual(prefs.syncFolderPath, "/Users/someone/Local") + // …and the server is brought up to date rather than left disagreeing. + let stored = await api.storedSettings() + XCTAssertEqual(stored[DocumentSyncSettingsKeys.folderMachineID]?.stringValue, deviceID) + } + + // MARK: - Boundary: no device id published yet + + func test_givenTheMainAppHasNotPublishedADeviceID_whenSynchronizing_thenNothingIsReadOrWritten() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(nil)) + + await prefs.synchronize() + + let reads = await api.reads + let writes = await api.writes + XCTAssertTrue(reads.isEmpty) + XCTAssertTrue(writes.isEmpty) + XCTAssertFalse(prefs.hasMigratedToAppSettings) + } + + // MARK: - Reported status + + func test_givenARecentlyPublishedSync_whenAnotherCycleCompletes_thenItIsNotPublishedAgain() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + await prefs.synchronize() + let writesAfterMigration = await api.writes.count + + let now = Date() + // Awaited rather than slept on: the publish is a task the manager hands + // back precisely so this assertion needs no timer. + await prefs.recordSync(at: now)?.value + let inWindow = prefs.recordSync(at: now.addingTimeInterval(60)) + let alsoInWindow = prefs.recordSync(at: now.addingTimeInterval(120)) + + // The first crosses the throttle (nothing published yet); the rest fall + // inside the window. A settings PUT per poll cycle is what this prevents. + XCTAssertNil(inWindow) + XCTAssertNil(alsoInWindow) + let writesAfterReports = await api.writes.count + XCTAssertEqual(writesAfterReports, writesAfterMigration + 1) + } + + func test_givenTheThrottleWindowHasPassed_whenACycleCompletes_thenItIsPublishedAgain() async { + let defaults = freshDefaults() + let api = FakeDeviceSettingsAPI() + let prefs = makeManager(defaults: defaults, api: api, identity: StaticDeviceIdentity(deviceID)) + await prefs.synchronize() + let writesAfterMigration = await api.writes.count + + let now = Date() + await prefs.recordSync(at: now)?.value + await prefs.recordSync( + at: now.addingTimeInterval(SyncConfiguration.lastSyncPublishInterval + 1) + )?.value + + let writesAfterReports = await api.writes.count + XCTAssertEqual(writesAfterReports, writesAfterMigration + 2) + let stored = await api.storedSettings() + XCTAssertNotNil(stored[DocumentSyncSettingsKeys.lastSyncAt]?.stringValue) + } } diff --git a/SyncAgent/Tests/InterlinedListSyncCoreTests/RemoteConfigurationStoreTests.swift b/SyncAgent/Tests/InterlinedListSyncCoreTests/RemoteConfigurationStoreTests.swift new file mode 100644 index 0000000..8a5f4d5 --- /dev/null +++ b/SyncAgent/Tests/InterlinedListSyncCoreTests/RemoteConfigurationStoreTests.swift @@ -0,0 +1,312 @@ +import XCTest +@testable import InterlinedListSyncCore + +/// The read/write half of the move to per-machine app settings (GitHub issue +/// #104): what a read means, when a migration is allowed, and what the agent +/// does when its background write loses the compare-and-set race. +final class RemoteConfigurationStoreTests: XCTestCase { + + private let deviceID = "mac-1" + + private func makeStore( + api: FakeDeviceSettingsAPI, + identity: any DeviceIdentifying + ) -> RemoteConfigurationStore { + RemoteConfigurationStore(api: api, identity: identity, deviceName: "Studio Mac") + } + + private func sampleConfiguration(machineID: String? = nil) -> SyncAgentConfiguration { + SyncAgentConfiguration( + syncEnabled: true, + pollIntervalSeconds: 90, + folder: machineID.map { + SyncFolderReference( + bookmark: Data("bm".utf8), + displayPath: "/Users/someone/Vault", + machineID: $0 + ) + } + ) + } + + // MARK: - Happy path + + func test_givenNothingStored_whenSaving_thenItCreatesTheDocumentAtBaseVersionZero() async throws { + let api = FakeDeviceSettingsAPI() + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + try await store.save(sampleConfiguration(machineID: deviceID)) + + let writes = await api.writes + XCTAssertEqual(writes.count, 1) + XCTAssertEqual(writes.first?.baseVersion, 0) + XCTAssertEqual(writes.first?.deviceId, deviceID) + let stored = await api.storedSettings() + XCTAssertEqual(stored[DocumentSyncSettingsKeys.pollIntervalSeconds]?.doubleValue, 90) + } + + func test_givenAStoredConfiguration_whenLoading_thenItIsReturned() async throws { + let api = FakeDeviceSettingsAPI() + await api.seed( + settings: sampleConfiguration(machineID: deviceID).apply(to: [:]), + version: 4 + ) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + let state = await store.load() + + guard case .stored(let configuration) = state else { + return XCTFail("Expected a stored configuration, got \(state)") + } + XCTAssertEqual(configuration.pollIntervalSeconds, 90) + XCTAssertEqual(configuration.folder?.displayPath, "/Users/someone/Vault") + } + + func test_givenAStoredConfiguration_whenSaving_thenItWritesAgainstTheVersionItRead() async throws { + let api = FakeDeviceSettingsAPI() + await api.seed(settings: sampleConfiguration(machineID: deviceID).apply(to: [:]), version: 7) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + var updated = sampleConfiguration(machineID: deviceID) + updated.pollIntervalSeconds = 300 + try await store.save(updated) + + let writes = await api.writes + // No explicit load() first: the store reads before its first write so the + // overlay has the real document to preserve and a version to send. + XCTAssertEqual(writes.map(\.baseVersion), [7]) + let version = await api.storedVersion() + XCTAssertEqual(version, 8) + } + + // MARK: - Invalid input + + func test_givenNoPublishedDeviceID_whenLoading_thenItIsUnavailableAndNothingIsRequested() async { + let api = FakeDeviceSettingsAPI() + let store = makeStore(api: api, identity: StaticDeviceIdentity(nil)) + + let state = await store.load() + + XCTAssertEqual(state, .unavailable) + let reads = await api.reads + XCTAssertTrue(reads.isEmpty, "Nothing is addressable without a device id") + } + + func test_givenNoPublishedDeviceID_whenSaving_thenItThrowsWithoutCallingTheAPI() async { + let api = FakeDeviceSettingsAPI() + let store = makeStore(api: api, identity: StaticDeviceIdentity(nil)) + + do { + try await store.save(sampleConfiguration()) + XCTFail("Expected a failure") + } catch { + XCTAssertEqual(error as? APIError, .noDeviceIdentity) + } + let writes = await api.writes + XCTAssertTrue(writes.isEmpty) + } + + // MARK: - Upstream failure + + func test_givenTheReadFails_whenLoading_thenItIsUnavailableRatherThanAbsent() async { + let api = FakeDeviceSettingsAPI() + await api.failReads(with: .transport("offline")) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + let state = await store.load() + + // The distinction is the whole safety property: `.absent` would invite a + // migration that overwrites a server document this client never saw. + XCTAssertEqual(state, .unavailable) + } + + func test_givenTheWriteKeepsConflicting_whenSaving_thenItGivesUpRatherThanRetryingForever() async { + let api = FakeDeviceSettingsAPI() + await api.seed(settings: [:], version: 1) + await api.failNextWrites(with: [ + .versionConflict(current: nil), + .versionConflict(current: nil), + .versionConflict(current: nil) + ]) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + do { + try await store.save(sampleConfiguration(machineID: deviceID)) + XCTFail("Expected a failure") + } catch { + guard case APIError.versionConflict = error else { + return XCTFail("Expected a version conflict, got \(error)") + } + } + let writes = await api.writes + XCTAssertEqual(writes.count, 3, "Retries are bounded, not unlimited") + } + + // MARK: - Compare-and-set + + func test_givenTheDocumentMovedOn_whenSaving_thenItRebasesFromThe409AndSucceeds() async throws { + let api = FakeDeviceSettingsAPI() + let winner = DeviceSettingsDocument( + version: 9, + deviceId: deviceID, + settings: ["theme": .string("dark")] + ) + await api.seed(settings: ["theme": .string("dark")], version: 9) + // The store's first write is built on a stale version, exactly as a + // background write racing the Applications pane would be. + await api.failNextWrites(with: [.versionConflict(current: winner)]) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + try await store.save(sampleConfiguration(machineID: deviceID)) + + let writes = await api.writes + XCTAssertEqual(writes.count, 2) + XCTAssertEqual(writes.last?.baseVersion, 9, "Re-based onto the winning document") + // Re-basing must also adopt the winner's payload, or the retry would + // delete keys the other writer had just added. + XCTAssertEqual(writes.last?.settings["theme"]?.stringValue, "dark") + let reads = await api.reads + XCTAssertEqual(reads.count, 1, "The 409 body carried the winner; no extra read") + } + + func test_givenA409WithNoBody_whenSaving_thenItRereadsAndRetries() async throws { + let api = FakeDeviceSettingsAPI() + await api.seed(settings: ["theme": .string("dark")], version: 3) + await api.failNextWrites(with: [.versionConflict(current: nil)]) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + try await store.save(sampleConfiguration(machineID: deviceID)) + + let reads = await api.reads + XCTAssertEqual(reads.count, 2, "One read before the first write, one to re-base") + let writes = await api.writes + XCTAssertEqual(writes.last?.baseVersion, 3) + } + + func test_givenThisMacIsNotInTheRegistry_whenSaving_thenItRegistersOnceAndRetries() async throws { + let api = FakeDeviceSettingsAPI() + await api.failNextWrites(with: [.deviceNotRegistered]) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + try await store.save(sampleConfiguration(machineID: deviceID)) + + let registrations = await api.registrations + XCTAssertEqual(registrations, [deviceID]) + let writes = await api.writes + XCTAssertEqual(writes.count, 2) + } + + func test_givenRegistrationDoesNotHelp_whenSaving_thenItStopsInsteadOfLooping() async { + let api = FakeDeviceSettingsAPI() + await api.failNextWrites(with: [.deviceNotRegistered, .deviceNotRegistered]) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + do { + try await store.save(sampleConfiguration(machineID: deviceID)) + XCTFail("Expected a failure") + } catch { + XCTAssertEqual(error as? APIError, .deviceNotRegistered) + } + let registrations = await api.registrations + XCTAssertEqual(registrations.count, 1, "Registering twice would not change the answer") + } + + // MARK: - Boundary + + func test_givenNothingStored_whenLoading_thenItIsAbsent() async { + let api = FakeDeviceSettingsAPI() + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + let state = await store.load() + + XCTAssertEqual(state, .absent) + } + + func test_givenOnlyTheMainAppsKeys_whenLoadingThenSaving_thenThoseKeysSurvive() async throws { + let api = FakeDeviceSettingsAPI() + await api.seed(settings: ["sidebarWidth": .number(280)], version: 2) + let store = makeStore(api: api, identity: StaticDeviceIdentity(deviceID)) + + // A document with no agent keys reads as absent — the agent has never + // run here — but its contents must still survive the migration write. + let state = await store.load() + XCTAssertEqual(state, .absent) + try await store.save(sampleConfiguration(machineID: deviceID)) + + let stored = await api.storedSettings() + XCTAssertEqual(stored["sidebarWidth"]?.doubleValue, 280) + XCTAssertNotNil(stored[DocumentSyncSettingsKeys.enabled]) + } +} + +// MARK: - The launch decision + +/// ``ConfigurationResolver`` is four cases of pure logic guarding the one thing +/// in this feature that can destroy a user's settings, so it gets its own tests +/// with no network, Keychain, or clock in the way. +final class ConfigurationResolverTests: XCTestCase { + + private let local = SyncAgentConfiguration(syncEnabled: false, pollIntervalSeconds: 45) + private let remote = SyncAgentConfiguration(syncEnabled: true, pollIntervalSeconds: 300) + + func test_givenBothSourcesHoldAValue_whenResolving_thenTheStoredOneWinsAndNothingMigrates() { + let resolution = ConfigurationResolver.resolve( + remote: .stored(remote), + local: local, + hasMigrated: false + ) + + XCTAssertEqual(resolution.configuration, remote) + XCTAssertFalse(resolution.shouldMigrate) + } + + func test_givenAFreshInstallWithNothingStoredEitherSide_whenResolving_thenDefaultsMigrateUp() { + let defaults = SyncAgentConfiguration() + + let resolution = ConfigurationResolver.resolve( + remote: .absent, + local: defaults, + hasMigrated: false + ) + + XCTAssertEqual(resolution.configuration, defaults) + XCTAssertTrue(resolution.shouldMigrate) + } + + func test_givenLocalSettingsAndAnEmptyServer_whenResolving_thenTheyMigrateUpOnce() { + let resolution = ConfigurationResolver.resolve( + remote: .absent, + local: local, + hasMigrated: false + ) + + XCTAssertEqual(resolution.configuration, local) + XCTAssertTrue(resolution.shouldMigrate) + } + + func test_givenAlreadyMigratedAndTheDocumentIsGone_whenResolving_thenItIsNotRecreated() { + // The machine was deregistered, or the settings were deleted on purpose. + // Re-uploading them would resurrect what the user removed. + let resolution = ConfigurationResolver.resolve( + remote: .absent, + local: local, + hasMigrated: true + ) + + XCTAssertEqual(resolution.configuration, local) + XCTAssertFalse(resolution.shouldMigrate) + } + + func test_givenTheServerCannotBeReached_whenResolving_thenNothingMigrates() { + // The dangerous case: treating "could not ask" as "nothing stored" would + // overwrite a newer server document with a stale local one. + let resolution = ConfigurationResolver.resolve( + remote: .unavailable, + local: local, + hasMigrated: false + ) + + XCTAssertEqual(resolution.configuration, local) + XCTAssertFalse(resolution.shouldMigrate) + } +} diff --git a/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAPIClientTests.swift b/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAPIClientTests.swift index eaa2b7a..a8ecd47 100644 --- a/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAPIClientTests.swift +++ b/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAPIClientTests.swift @@ -100,4 +100,148 @@ final class SyncAPIClientTests: XCTestCase { let client = makeClient() try await client.deleteDocument(id: "gone") // must not throw } + + // MARK: - Per-machine app settings (GitHub issue #104) + // + // Path, method and body are asserted together on purpose. A wrong body is + // as breaking as a wrong path and far quieter — PR #102 found this exact + // family shipping `{"name":…}` for weeks while every path assertion passed. + + func test_givenAConfiguredMachine_whenWritingSettings_thenItPutsToTheDeviceRouteWithABaseVersion() async throws { + MockURLProtocol.handler = { request in + XCTAssertEqual(request.httpMethod, "PUT") + XCTAssertEqual( + request.url?.path, + "/api/user/app-settings/interlinedlist-macos/devices/mac-1/settings" + ) + let body = try XCTUnwrap(request.httpBodyStream.map { stream -> Data in + stream.open() + defer { stream.close() } + var data = Data() + var buffer = [UInt8](repeating: 0, count: 4096) + while stream.hasBytesAvailable { + let read = stream.read(&buffer, maxLength: buffer.count) + if read <= 0 { break } + data.append(buffer, count: read) + } + return data + } ?? request.httpBody) + let json = try XCTUnwrap(JSONSerialization.jsonObject(with: body) as? [String: Any]) + XCTAssertEqual(json["baseVersion"] as? Int, 3) + XCTAssertEqual(json["schemaVersion"] as? Int, SyncConfiguration.settingsSchemaVersion) + let settings = try XCTUnwrap(json["settings"] as? [String: Any]) + XCTAssertEqual(settings[DocumentSyncSettingsKeys.enabled] as? Bool, true) + + let response = #"{"appKey":"interlinedlist-macos","scope":"device","deviceId":"mac-1","version":4,"updatedAt":"2026-09-16T19:39:07.135Z","schemaVersion":1,"settings":{"documentSync.enabled":true}}"# + return ( + HTTPURLResponse(url: request.url!, statusCode: 200, httpVersion: nil, headerFields: nil)!, + Data(response.utf8) + ) + } + let client = makeClient() + + let document = try await client.writeDeviceSettings( + deviceId: "mac-1", + settings: [DocumentSyncSettingsKeys.enabled: .bool(true)], + baseVersion: 3 + ) + + XCTAssertEqual(document.version, 4) + XCTAssertEqual(document.deviceId, "mac-1") + } + + func test_givenAMachineThatHasNeverStoredSettings_whenReading_thenItAnswersNilRatherThanThrowing() async throws { + MockURLProtocol.handler = { request in + XCTAssertEqual(request.httpMethod, "GET") + return ( + HTTPURLResponse(url: request.url!, statusCode: 404, httpVersion: nil, headerFields: nil)!, + Data(#"{"error":"Not found","code":"not_found"}"#.utf8) + ) + } + let client = makeClient() + + let document = try await client.fetchDeviceSettings(deviceId: "mac-1") + + // First run, not a failure. The caller has to be able to tell this from + // "the request failed", because only this one may trigger a migration. + XCTAssertNil(document) + } + + func test_givenAStaleBaseVersion_whenWritingSettings_thenTheConflictCarriesTheWinningDocument() async { + MockURLProtocol.handler = { request in + let response = #"{"error":"version_conflict","code":"version_conflict","current":{"appKey":"interlinedlist-macos","scope":"device","deviceId":"mac-1","version":9,"schemaVersion":1,"settings":{"theme":"dark"}}}"# + return ( + HTTPURLResponse(url: request.url!, statusCode: 409, httpVersion: nil, headerFields: nil)!, + Data(response.utf8) + ) + } + let client = makeClient() + + do { + _ = try await client.writeDeviceSettings(deviceId: "mac-1", settings: [:], baseVersion: 1) + XCTFail("expected a version conflict") + } catch APIError.versionConflict(let current) { + // Lifting `current` out of the 409 is what lets a background retry + // re-base without spending another request. + XCTAssertEqual(current?.version, 9) + XCTAssertEqual(current?.settings["theme"]?.stringValue, "dark") + } catch { + XCTFail("expected a version conflict, got \(error)") + } + } + + func test_givenAnUnparseableConflictBody_whenWritingSettings_thenItStillSurfacesAsAConflict() async { + MockURLProtocol.handler = { request in + ( + HTTPURLResponse(url: request.url!, statusCode: 409, httpVersion: nil, headerFields: nil)!, + Data("not json".utf8) + ) + } + let client = makeClient() + + do { + _ = try await client.writeDeviceSettings(deviceId: "mac-1", settings: [:], baseVersion: 1) + XCTFail("expected a version conflict") + } catch APIError.versionConflict(let current) { + // The retry falls back to a fresh read rather than the whole write + // failing on a decoding error. + XCTAssertNil(current) + } catch { + XCTFail("expected a version conflict, got \(error)") + } + } + + func test_givenAnUnregisteredMachine_whenWritingSettings_thenItIsNotMistakenForAnEmptyDocument() async { + MockURLProtocol.handler = { request in + ( + HTTPURLResponse(url: request.url!, statusCode: 404, httpVersion: nil, headerFields: nil)!, + Data(#"{"error":"device not registered"}"#.utf8) + ) + } + let client = makeClient() + + do { + _ = try await client.writeDeviceSettings(deviceId: "mac-1", settings: [:], baseVersion: 0) + XCTFail("expected deviceNotRegistered") + } catch { + // `baseVersion: 0` creates a document happily, so a 404 here can only + // mean the device is missing — mapping it to "empty" would silently + // discard the user's settings. + XCTAssertEqual(error as? APIError, .deviceNotRegistered) + } + } + + func test_givenAnUnregisteredMachine_whenRegistering_thenItPostsTheLiveFieldNames() async throws { + MockURLProtocol.handler = { request in + XCTAssertEqual(request.httpMethod, "POST") + XCTAssertEqual(request.url?.path, "/api/user/app-settings/interlinedlist-macos/devices") + return ( + HTTPURLResponse(url: request.url!, statusCode: 200, httpVersion: nil, headerFields: nil)!, + Data(#"{"device":{"deviceId":"mac-1","deviceName":"Studio Mac","isDefault":true}}"#.utf8) + ) + } + let client = makeClient() + + try await client.registerDevice(deviceId: "mac-1", deviceName: "Studio Mac") + } } diff --git a/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAgentConfigurationTests.swift b/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAgentConfigurationTests.swift new file mode 100644 index 0000000..febbf95 --- /dev/null +++ b/SyncAgent/Tests/InterlinedListSyncCoreTests/SyncAgentConfigurationTests.swift @@ -0,0 +1,140 @@ +import XCTest +@testable import InterlinedListSyncCore + +/// The payload shape the agent stores in per-machine app settings (GitHub issue +/// #104) — in particular the rule that a security-scoped bookmark belonging to +/// another Mac is not a configuration, it is noise. +final class SyncAgentConfigurationTests: XCTestCase { + + private let thisMachine = "mac-1" + private let bookmark = Data("bookmark-bytes".utf8) + + private func configured(machineID: String) -> SyncAgentConfiguration { + SyncAgentConfiguration( + syncEnabled: false, + pollIntervalSeconds: 120, + launchAtLogin: true, + notificationsEnabled: false, + notifyOnCompletion: true, + notifyOnErrors: false, + notifyOnConflicts: false, + folder: SyncFolderReference( + bookmark: bookmark, + displayPath: "/Users/someone/Vault", + machineID: machineID + ), + lastSyncAt: Date(timeIntervalSince1970: 1_700_000_000) + ) + } + + // MARK: - Happy path + + func test_givenAConfigurationWithAFolder_whenWrittenAndReadBack_thenEveryFieldSurvives() throws { + let original = configured(machineID: thisMachine) + + let payload = original.apply(to: [:]) + let restored = try XCTUnwrap( + SyncAgentConfiguration(settings: payload, thisMachineID: thisMachine) + ) + + XCTAssertEqual(restored, original) + XCTAssertEqual( + payload[DocumentSyncSettingsKeys.schemaVersion]?.doubleValue, + Double(SyncConfiguration.settingsSchemaVersion) + ) + } + + func test_givenADocumentWrittenHere_whenRead_thenTheFolderIsAccepted() throws { + let payload = configured(machineID: thisMachine).apply(to: [:]) + + let restored = try XCTUnwrap( + SyncAgentConfiguration(settings: payload, thisMachineID: thisMachine) + ) + + XCTAssertEqual(restored.folder?.bookmark, bookmark) + XCTAssertEqual(restored.folder?.displayPath, "/Users/someone/Vault") + } + + // MARK: - Invalid input + + func test_givenABookmarkFromAnotherMac_whenRead_thenTheMachineIsNotConfiguredHere() throws { + let payload = configured(machineID: "mac-2").apply(to: [:]) + + let restored = try XCTUnwrap( + SyncAgentConfiguration(settings: payload, thisMachineID: thisMachine) + ) + + // Everything else still applies — only the folder is refused, because + // only the folder is machine-local. + XCTAssertNil(restored.folder) + XCTAssertEqual(restored.pollIntervalSeconds, 120) + XCTAssertFalse(restored.syncEnabled) + } + + func test_givenABookmarkWithNoMachineStamp_whenRead_thenTheFolderIsRefused() throws { + var payload = configured(machineID: thisMachine).apply(to: [:]) + payload[DocumentSyncSettingsKeys.folderMachineID] = nil + + let restored = try XCTUnwrap( + SyncAgentConfiguration(settings: payload, thisMachineID: thisMachine) + ) + + XCTAssertNil(restored.folder) + } + + func test_givenAPollIntervalBelowTheFloor_whenRead_thenItIsClamped() throws { + var payload = configured(machineID: thisMachine).apply(to: [:]) + payload[DocumentSyncSettingsKeys.pollIntervalSeconds] = .number(1) + + let restored = try XCTUnwrap( + SyncAgentConfiguration(settings: payload, thisMachineID: thisMachine) + ) + + XCTAssertEqual(restored.pollIntervalSeconds, SyncConfiguration.minPollInterval) + } + + // MARK: - Boundary + + func test_givenADocumentWithNoAgentKeys_whenRead_thenThereIsNoStoredConfiguration() { + // The main app may well have stored per-machine settings of its own. + // That is not a configuration this agent wrote, so the migration must + // still be allowed to run. + let payload: [String: SettingsValue] = ["sidebarWidth": .number(280)] + + XCTAssertNil(SyncAgentConfiguration(settings: payload, thisMachineID: thisMachine)) + } + + func test_givenAnEmptyDocument_whenRead_thenThereIsNoStoredConfiguration() { + XCTAssertNil(SyncAgentConfiguration(settings: [:], thisMachineID: thisMachine)) + } + + // MARK: - Shared document + + func test_givenKeysOwnedByAnotherClient_whenWriting_thenTheySurvive() { + let existing: [String: SettingsValue] = [ + "sidebarWidth": .number(280), + "theme": .string("dark") + ] + + let payload = configured(machineID: thisMachine).apply(to: existing) + + // The PUT replaces the document wholesale, so anything the overlay drops + // is deleted from the server. These keys belong to the main app. + XCTAssertEqual(payload["sidebarWidth"]?.doubleValue, 280) + XCTAssertEqual(payload["theme"]?.stringValue, "dark") + } + + func test_givenAStoredFolder_whenWritingAConfigurationWithoutOne_thenTheStaleFolderKeysGo() { + let existing = configured(machineID: thisMachine).apply(to: [:]) + var unconfigured = configured(machineID: thisMachine) + unconfigured.folder = nil + + let payload = unconfigured.apply(to: existing) + + // Rebuilt from scratch inside the namespace: leaving a stale bookmark + // behind would let a cleared folder come back on the next read. + XCTAssertNil(payload[DocumentSyncSettingsKeys.folderBookmark]) + XCTAssertNil(payload[DocumentSyncSettingsKeys.folderPath]) + XCTAssertNil(payload[DocumentSyncSettingsKeys.folderMachineID]) + } +}