diff --git a/App/Features/Settings/DevicesView.swift b/App/Features/Settings/DevicesView.swift index a8dfdac..2bc2cb5 100644 --- a/App/Features/Settings/DevicesView.swift +++ b/App/Features/Settings/DevicesView.swift @@ -1,8 +1,17 @@ // DevicesView // -// Settings ▸ Devices (work-consolidation.md G17) — the machines registered under -// this app's key. Rename a machine, promote one to main workstation (its config -// seeds a brand-new device on first sign-in), or deregister one. +// Settings ▸ Applications (work-consolidation.md G17, GitHub issue #56) — the +// machines registered under this app's key and the settings documents they +// share and pin. +// +// Every action `/help/app-settings` documents lives here: set as main +// workstation, rename, remove, view the shared and per-machine settings, copy a +// machine's settings to shared, and delete the shared settings. +// +// Each confirmation states the *actual* consequence rather than a generic "are +// you sure": removal takes the machine's own settings with it, and copy-to- +// shared replaces rather than merges. Both are irreversible, and neither is +// obvious from the button name. // // SwiftUI-only (no AppKit). Consumes only `InterlinedDomain` per Decision 0003. @@ -16,6 +25,8 @@ struct DevicesView: View { @State private var renaming: AppDevice? @State private var draftName: String = "" @State private var pendingDeregister: AppDevice? + @State private var pendingCopyToShared: AppDevice? + @State private var confirmingSharedDelete = false var body: some View { Group { @@ -29,7 +40,8 @@ struct DevicesView: View { if viewModel == nil { let model = DevicesViewModel( service: environment?.appSettings, - currentDeviceID: DeviceIdentity.current() + currentDeviceID: DeviceIdentity.current(), + currentDeviceName: DeviceIdentity.suggestedName ) viewModel = model await model.load() @@ -41,25 +53,16 @@ struct DevicesView: View { private func content(_ viewModel: DevicesViewModel) -> some View { if viewModel.isUnavailable { SettingsUnavailableView( - title: "Devices unavailable", - message: "Synced settings need an app key registered with InterlinedList before this Mac can appear here." + title: "Applications unavailable", + message: "This build has no app-settings key configured, so this Mac cannot sync settings or appear in the device list." ) } else { Form { if let error = viewModel.error { Section { SettingsErrorRow(error: error) } } - Section("Registered devices") { - if viewModel.isLoading && viewModel.devices.isEmpty { - ProgressView() - } else if viewModel.devices.isEmpty { - Text("No devices registered yet.").foregroundStyle(.secondary) - } else { - ForEach(viewModel.devices) { device in - row(device, viewModel: viewModel) - } - } - } + sharedSettingsSection(viewModel) + devicesSection(viewModel) } .formStyle(.grouped) .alert("Rename device", isPresented: Binding( @@ -75,19 +78,114 @@ struct DevicesView: View { Button("Cancel", role: .cancel) { renaming = nil } } .confirmationDialog( - "Deregister this device?", + "Remove this machine?", isPresented: Binding( get: { pendingDeregister != nil }, set: { if !$0 { pendingDeregister = nil } } ), presenting: pendingDeregister ) { device in - Button("Deregister", role: .destructive) { + Button("Remove", role: .destructive) { Task { await viewModel.deregister(device); pendingDeregister = nil } } Button("Cancel", role: .cancel) { pendingDeregister = nil } } message: { device in - Text("\(device.name) will lose its per-machine settings. Shared settings are unaffected.") + // Naming both halves matters: users hesitate to remove an old + // machine for fear of losing the settings they share with it. + Text( + device.isMainWorkstation + ? "\(device.name) will lose the settings saved just for it. Shared settings are not affected. Another machine will become the main workstation." + : "\(device.name) will lose the settings saved just for it. Shared settings are not affected." + ) + } + .confirmationDialog( + "Replace shared settings?", + isPresented: Binding( + get: { pendingCopyToShared != nil }, + set: { if !$0 { pendingCopyToShared = nil } } + ), + presenting: pendingCopyToShared + ) { device in + Button("Replace", role: .destructive) { + Task { await viewModel.copySettingsToShared(from: device); pendingCopyToShared = nil } + } + Button("Cancel", role: .cancel) { pendingCopyToShared = nil } + } message: { device in + Text("The shared settings will be replaced with the settings from \(device.name). This is not a merge — shared settings that \(device.name) does not have will be removed.") + } + .confirmationDialog( + "Delete shared settings?", + isPresented: $confirmingSharedDelete + ) { + Button("Delete", role: .destructive) { + Task { await viewModel.deleteSharedSettings() } + } + Button("Cancel", role: .cancel) {} + } message: { + Text("Every machine loses the settings shared across this account. Settings saved for individual machines are not affected.") + } + .sheet(isPresented: Binding( + get: { viewModel.inspection != nil }, + set: { if !$0 { viewModel.dismissInspection() } } + )) { + if let inspection = viewModel.inspection { + DeviceSettingsInspector(inspection: inspection) { + viewModel.dismissInspection() + } + } + } + } + } + + // MARK: - Shared settings + + @ViewBuilder + private func sharedSettingsSection(_ viewModel: DevicesViewModel) -> some View { + Section("Shared settings") { + if let document = viewModel.sharedDocument { + SettingsDocumentSummary(document: document) + HStack { + Spacer() + if viewModel.busyID == DevicesViewModel.sharedSettingsRowID { + ProgressView().controlSize(.small) + } else { + Button("Delete Shared Settings…", role: .destructive) { + confirmingSharedDelete = true + } + .disabled(viewModel.busyID != nil) + } + } + } else if viewModel.isLoading { + ProgressView() + } else { + // Nothing stored is the ordinary first-run state, not an error. + Text("No settings are shared across this account yet.") + .foregroundStyle(.secondary) + } + } + } + + // MARK: - Devices + + @ViewBuilder + private func devicesSection(_ viewModel: DevicesViewModel) -> some View { + Section("Registered machines") { + if viewModel.mainWorkstationIsUnknown { + Label( + "Another machine became the main workstation. Reopen this pane to see which.", + systemImage: "questionmark.circle" + ) + .font(.callout) + .foregroundStyle(.secondary) + } + if viewModel.isLoading && viewModel.devices.isEmpty { + ProgressView() + } else if viewModel.devices.isEmpty { + Text("No machines registered yet.").foregroundStyle(.secondary) + } else { + ForEach(viewModel.devices) { device in + row(device, viewModel: viewModel) + } } } } @@ -104,17 +202,17 @@ struct DevicesView: View { .padding(.horizontal, 6).padding(.vertical, 2) .background(.tint.opacity(0.15), in: Capsule()) } - if device.isMainWorkstation { + // Suppressed while the holder is unknown: a stale badge + // asserts something this client can no longer vouch for. + if device.isMainWorkstation && !viewModel.mainWorkstationIsUnknown { Label("Main", systemImage: "star.fill") .labelStyle(.titleAndIcon) .font(.caption2) .foregroundStyle(.secondary) } } - if let lastSeen = device.lastSeenAt { - Text("Last seen \(lastSeen.formatted(.relative(presentation: .named)))") - .font(.caption).foregroundStyle(.secondary) - } + Text(subtitle(for: device)) + .font(.caption).foregroundStyle(.secondary) } Spacer() if viewModel.busyID == device.id { @@ -123,12 +221,20 @@ struct DevicesView: View { Menu { Button("Rename…") { renaming = device; draftName = device.name } if !device.isMainWorkstation { - Button("Make main workstation") { + Button("Make Main Workstation") { Task { await viewModel.makeMainWorkstation(device) } } } Divider() - Button("Deregister…", role: .destructive) { pendingDeregister = device } + Button("View Settings…") { Task { await viewModel.inspect(device) } } + // `hasDeviceSettings == nil` means the server did not say, + // so the action stays available rather than being hidden on + // an assumption; the service refuses an empty copy. + if device.hasDeviceSettings != false { + Button("Copy Settings to Shared…") { pendingCopyToShared = device } + } + Divider() + Button("Remove…", role: .destructive) { pendingDeregister = device } } label: { Label("Actions", systemImage: "ellipsis.circle") .labelStyle(.iconOnly) @@ -140,7 +246,96 @@ struct DevicesView: View { } .accessibilityElement(children: .combine) .accessibilityLabel( - device.isMainWorkstation ? "\(device.name), main workstation" : device.name + device.isMainWorkstation && !viewModel.mainWorkstationIsUnknown + ? "\(device.name), main workstation" + : device.name ) } + + private func subtitle(for device: AppDevice) -> String { + var parts: [String] = [] + if let lastSeen = device.lastSeenAt { + parts.append("Last seen \(lastSeen.formatted(.relative(presentation: .named)))") + } + if device.hasDeviceSettings == true { + parts.append("has its own settings") + } + return parts.joined(separator: " · ") + } +} + +// MARK: - Settings document presentation + +/// Last-updated and size for one settings document. +/// +/// Size is shown because the payload is opaque to this app as well as to the +/// server — there is nothing else truthful to say about its contents at a +/// glance, and the server enforces a size cap. +struct SettingsDocumentSummary: View { + let document: AppSettingsDocument + + var body: some View { + LabeledContent("Last updated") { + if let updatedAt = document.updatedAt { + Text(updatedAt.formatted(date: .abbreviated, time: .shortened)) + } else { + Text("Unknown").foregroundStyle(.secondary) + } + } + LabeledContent("Size") { + Text(document.byteSize.formatted(.byteCount(style: .file))) + } + LabeledContent("Entries") { + Text("\(document.bag.count)") + } + } +} + +/// One machine's settings, shown on demand. +struct DeviceSettingsInspector: View { + let inspection: DevicesViewModel.Inspection + let dismiss: () -> Void + + var body: some View { + VStack(alignment: .leading, spacing: 12) { + Text(inspection.device.name).font(.headline) + + if inspection.isLoading { + ProgressView().frame(maxWidth: .infinity) + } else if let error = inspection.error { + Label(error, systemImage: "exclamationmark.triangle") + .foregroundStyle(.red).font(.callout) + } else if let document = inspection.document { + Form { + Section("Settings saved for this machine") { + SettingsDocumentSummary(document: document) + } + Section("Keys") { + if document.bag.isEmpty { + Text("None").foregroundStyle(.secondary) + } else { + // Keys only, never values: the payload can hold a + // sync folder path and other machine-local detail, + // and this pane is about accounting for settings, + // not displaying them. + ForEach(document.bag.keys, id: \.self) { key in + Text(key).font(.callout.monospaced()) + } + } + } + } + .formStyle(.grouped) + } else { + Text("This machine has no settings of its own. It uses the shared settings.") + .foregroundStyle(.secondary) + } + + HStack { + Spacer() + Button("Done", action: dismiss).keyboardShortcut(.defaultAction) + } + } + .padding() + .frame(width: 420, height: 380) + } } diff --git a/App/Features/Settings/DevicesViewModel.swift b/App/Features/Settings/DevicesViewModel.swift index c3d4055..a9bd71f 100644 --- a/App/Features/Settings/DevicesViewModel.swift +++ b/App/Features/Settings/DevicesViewModel.swift @@ -1,13 +1,19 @@ // DevicesViewModel // -// Drives Settings ▸ Devices (work-consolidation.md G17) — the machines -// registered under this app's key, with rename, "make main workstation", and -// deregister actions. +// Drives Settings ▸ Applications (work-consolidation.md G17, GitHub issue #56) +// — the machines registered under this app's key, plus the settings documents +// they share and pin. +// +// The six actions `/help/app-settings` documents: rename a machine, promote one +// to main workstation, remove one, inspect the shared and per-machine settings, +// copy a machine's settings to shared, and delete the shared settings. // // The main workstation matters because its configuration seeds a brand-new // device on first sign-in, so promoting one is a real, user-visible decision // rather than cosmetic. // +// This section is free — there is deliberately no capability gate here. +// // Per Decision 0003 this view model consumes only `InterlinedDomain`. import Foundation @@ -18,91 +24,240 @@ import InterlinedDomain @Observable final class DevicesViewModel { + /// One machine's settings document, loaded on demand for the inspector. + struct Inspection: Equatable { + let device: AppDevice + /// nil once loaded means the machine has stored no settings of its own. + var document: AppSettingsDocument? + var isLoading: Bool + var error: String? + } + + /// Identifies the shared-settings row for `busyID`, which otherwise holds a + /// device id. A reserved sentinel rather than a second flag so that "only + /// one mutation at a time" stays a single invariant. + static let sharedSettingsRowID = "\u{0}shared-settings" + private let service: AppSettingsServicing? /// This machine's stable id, so the list can mark "This Mac". private let currentDeviceID: String + /// The name to register this machine under if it is not in the registry. + private let currentDeviceName: String private(set) var devices: [AppDevice] = [] + /// The account-wide document, nil when nothing is stored yet. + private(set) var sharedDocument: AppSettingsDocument? private(set) var isLoading = false - /// The device id with an action in flight, so only that row shows progress. + /// The row with an action in flight, so only that row shows progress. private(set) var busyID: String? private(set) var error: Error? - /// True when no `appKey` is registered yet (see `AppEnvironment.appSettingsKey`). + /// True when the main workstation was removed and this client could not + /// learn who inherited the role. The badge must then show nothing rather + /// than a stale flag pointing at a machine that no longer holds it. + private(set) var mainWorkstationIsUnknown = false + + /// The machine whose settings the inspector is showing, if any. + private(set) var inspection: Inspection? + + /// True when no `appKey` is configured in this build + /// (see `AppEnvironment.appSettingsKey`). var isUnavailable: Bool { service == nil } - init(service: AppSettingsServicing?, currentDeviceID: String) { + init( + service: AppSettingsServicing?, + currentDeviceID: String, + currentDeviceName: String = "" + ) { self.service = service self.currentDeviceID = currentDeviceID + self.currentDeviceName = currentDeviceName } func isCurrentDevice(_ device: AppDevice) -> Bool { device.id == currentDeviceID } + // MARK: - Loading + func load() async { guard let service else { return } isLoading = true error = nil defer { isLoading = false } do { - // This Mac first, then the main workstation, then by name — the two - // rows a user acts on are the ones they can identify. - devices = try await service.devices().sorted { lhs, rhs in - if isCurrentDevice(lhs) != isCurrentDevice(rhs) { return isCurrentDevice(lhs) } - if lhs.isMainWorkstation != rhs.isMainWorkstation { return lhs.isMainWorkstation } - return lhs.name.localizedCaseInsensitiveCompare(rhs.name) == .orderedAscending - } + // The registry and the account document are independent reads, so + // overlap them — this pane is behind a tab the user just clicked. + async let devicesTask = service.devices() + async let sharedTask = service.sharedDocument() + let (loaded, shared) = try await (devicesTask, sharedTask) + apply(try await registeringThisMacIfAbsent(in: loaded, using: service)) + sharedDocument = shared + // A successful list is authoritative about who holds the role. + mainWorkstationIsUnknown = false } catch { self.error = error } } + /// Adds this Mac to the registry the first time the pane is opened on it. + /// + /// Nothing else registers this machine, so without this the pane lists every + /// *other* computer and never the one the user is sitting at — and the + /// per-machine settings half has no row to hang off. + /// + /// **Only when absent.** Verified live 2026-09-16 that `POST …/devices` is + /// an upsert keyed on `deviceId`: re-posting an existing id does not + /// duplicate the row, it overwrites `deviceName`. Registering + /// unconditionally would therefore reset the machine's name to this Mac's + /// hostname every single time the pane was opened, silently undoing any + /// rename the user had made. + /// + /// A failure here is swallowed deliberately: the registry itself loaded, and + /// failing to add this Mac must not blank a list of machines the user came + /// here to manage. + private func registeringThisMacIfAbsent( + in loaded: [AppDevice], + using service: AppSettingsServicing + ) async throws -> [AppDevice] { + guard !loaded.contains(where: { $0.id == currentDeviceID }), + !currentDeviceName.isEmpty + else { return loaded } + guard let registered = try? await service.registerDevice( + deviceID: currentDeviceID, + name: currentDeviceName + ) else { return loaded } + return loaded + [registered] + } + + /// This Mac first, then the main workstation, then by name — the two rows a + /// user acts on are the ones they can identify. + private func apply(_ loaded: [AppDevice]) { + devices = loaded.sorted { lhs, rhs in + if isCurrentDevice(lhs) != isCurrentDevice(rhs) { return isCurrentDevice(lhs) } + if lhs.isMainWorkstation != rhs.isMainWorkstation { return lhs.isMainWorkstation } + return lhs.name.localizedCaseInsensitiveCompare(rhs.name) == .orderedAscending + } + } + + // MARK: - Device actions + func rename(_ device: AppDevice, to name: String) async { let trimmed = name.trimmingCharacters(in: .whitespacesAndNewlines) guard let service, busyID == nil, !trimmed.isEmpty, trimmed != device.name else { return } - busyID = device.id - error = nil - defer { busyID = nil } - do { + await mutate(device.id) { let updated = try await service.renameDevice(deviceID: device.id, to: trimmed) - replace(updated) - } catch { - self.error = error + self.replace(updated) } } func makeMainWorkstation(_ device: AppDevice) async { guard let service, busyID == nil, !device.isMainWorkstation else { return } - busyID = device.id - error = nil - defer { busyID = nil } - do { + await mutate(device.id) { let updated = try await service.makeMainWorkstation(deviceID: device.id) - // Exactly one device holds the flag, so clear it locally everywhere - // else rather than re-fetching the whole registry. - devices = devices.map { existing in - guard existing.id != updated.id, existing.isMainWorkstation else { return existing } - return AppDevice( - id: existing.id, - name: existing.name, - isMainWorkstation: false, - createdAt: existing.createdAt, - lastSeenAt: existing.lastSeenAt - ) + // Exactly one device holds the flag and the server enforces it by + // demoting the previous holder, so clear it locally everywhere else + // rather than spending a second round-trip to learn what we already + // know. + self.devices = self.devices.map { existing in + existing.id == updated.id || !existing.isMainWorkstation + ? existing + : existing.settingMainWorkstation(false) } - replace(updated) - } catch { - self.error = error + self.replace(updated) + self.mainWorkstationIsUnknown = false } } + /// Removes a machine from the registry. Its per-machine settings go with + /// it; the shared settings are untouched. func deregister(_ device: AppDevice) async { guard let service, busyID == nil else { return } - busyID = device.id + let wasMain = device.isMainWorkstation + await mutate(device.id) { + let outcome = try await service.deregisterDevice(deviceID: device.id) + self.devices.removeAll { $0.id == device.id } + if self.inspection?.device.id == device.id { self.inspection = nil } + + if let promoted = outcome.promotedDeviceID { + // The server names the machine it promoted, so there is nothing + // to guess at and nothing to re-read to find out. + self.devices = self.devices.map { + $0.settingMainWorkstation($0.id == promoted) + } + } else if wasMain && !self.devices.isEmpty { + // The removed machine held the role, the server did not say who + // took it over, and someone must have. Showing the old flags + // would assert something we no longer know to be true. + self.mainWorkstationIsUnknown = true + } + + // Refetch so `hasDeviceSettings` and last-seen times reconcile — and + // so an unknown main workstation resolves. Deliberately not through + // `load()`: a failure here must not raise an error banner, because + // the removal itself succeeded. The unknown badge is how a failed + // reconcile shows up. + if let reloaded = try? await service.devices() { + self.apply(reloaded) + self.mainWorkstationIsUnknown = false + } + } + } + + // MARK: - Settings documents + + /// Loads one machine's settings document for the inspector. + func inspect(_ device: AppDevice) async { + guard let service else { return } + inspection = Inspection(device: device, document: nil, isLoading: true, error: nil) + do { + let document = try await service.deviceDocument(deviceID: device.id) + // The user may have closed the inspector or opened another machine's + // while this was in flight; do not stamp a stale answer over it. + guard inspection?.device.id == device.id else { return } + inspection = Inspection(device: device, document: document, isLoading: false, error: nil) + } catch { + guard inspection?.device.id == device.id else { return } + inspection = Inspection( + device: device, + document: nil, + isLoading: false, + error: error.localizedDescription + ) + } + } + + func dismissInspection() { inspection = nil } + + /// Replaces the shared settings with this machine's own, wholesale. + /// + /// Not a merge: the write replaces the document, so any shared key this + /// machine does not have is deleted. The view confirms destructively. + func copySettingsToShared(from device: AppDevice) async { + guard let service, busyID == nil else { return } + await mutate(device.id) { + self.sharedDocument = try await service.copyDeviceSettingsToShared(deviceID: device.id) + } + } + + func deleteSharedSettings() async { + guard let service, busyID == nil else { return } + await mutate(Self.sharedSettingsRowID) { + try await service.deleteSharedSettings() + // The document is gone; a re-read would only 404 back to the same + // nil this already represents. + self.sharedDocument = nil + } + } + + // MARK: - Plumbing + + /// Runs one mutation with the shared busy/error bookkeeping, so every action + /// marks exactly one row busy and clears it on every exit path. + private func mutate(_ rowID: String, _ body: () async throws -> Void) async { + busyID = rowID error = nil defer { busyID = nil } do { - try await service.deregisterDevice(deviceID: device.id) - devices.removeAll { $0.id == device.id } + try await body() } catch { self.error = error } diff --git a/App/Features/Settings/SettingsRootView.swift b/App/Features/Settings/SettingsRootView.swift index 9cf8060..2a84483 100644 --- a/App/Features/Settings/SettingsRootView.swift +++ b/App/Features/Settings/SettingsRootView.swift @@ -104,11 +104,14 @@ struct SettingsRootView: View { } .tag(SettingsTab.security) - // Synced-settings device registry (work-consolidation.md G17) — the - // machines registered under this app's key. + // Synced settings + device registry (work-consolidation.md G17, + // GitHub issue #56). Labelled "Applications" to match + // `/help/app-settings`, which is where users are told to look for + // it; the pane covers the settings documents as well as the + // machines, so "Devices" undersold it. DevicesView() .tabItem { - Label("Devices", systemImage: "desktopcomputer") + Label("Applications", systemImage: "desktopcomputer") } .tag(SettingsTab.devices) diff --git a/AppTests/DevicesViewModelTests.swift b/AppTests/DevicesViewModelTests.swift index bcef331..4c17191 100644 --- a/AppTests/DevicesViewModelTests.swift +++ b/AppTests/DevicesViewModelTests.swift @@ -1,6 +1,7 @@ // DevicesViewModelTests // -// BDD-named tests for Settings ▸ Devices (work-consolidation.md G17). +// BDD-named tests for Settings ▸ Applications (work-consolidation.md G17, +// GitHub issue #56). import XCTest import InterlinedDomain @@ -11,12 +12,35 @@ final class DevicesViewModelTests: XCTestCase { private let thisMac = "dev-this" - private func device(_ id: String, name: String? = nil, main: Bool = false) -> AppDevice { - AppDevice(id: id, name: name ?? id, isMainWorkstation: main) + private func device( + _ id: String, + name: String? = nil, + main: Bool = false, + hasSettings: Bool? = nil + ) -> AppDevice { + AppDevice( + id: id, + name: name ?? id, + isMainWorkstation: main, + hasDeviceSettings: hasSettings + ) } - private func makeViewModel(_ stub: StubAppSettingsService?) -> DevicesViewModel { - DevicesViewModel(service: stub, currentDeviceID: thisMac) + private func document(_ keys: [String: String] = [:], version: Int = 1) -> AppSettingsDocument { + var bag = AppSettingsBag() + for (key, value) in keys { bag[string: key] = value } + return AppSettingsDocument(bag: bag, version: version, updatedAt: Date()) + } + + private func makeViewModel( + _ stub: StubAppSettingsService?, + deviceName: String = "" + ) -> DevicesViewModel { + DevicesViewModel( + service: stub, + currentDeviceID: thisMac, + currentDeviceName: deviceName + ) } // MARK: - Happy path @@ -37,6 +61,19 @@ final class DevicesViewModelTests: XCTestCase { XCTAssertTrue(viewModel.isCurrentDevice(viewModel.devices[0])) } + func test_givenStoredSharedSettings_whenLoading_thenExposesTheDocument() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a")]) + stub.enqueueSharedDocument(document(["theme": "dark"], version: 4)) + let viewModel = makeViewModel(stub) + + await viewModel.load() + + XCTAssertEqual(viewModel.sharedDocument?.version, 4) + XCTAssertGreaterThan(viewModel.sharedDocument?.byteSize ?? 0, 0) + XCTAssertNil(viewModel.error) + } + func test_givenRename_whenRenaming_thenSendsTrimmedNameAndUpdatesRow() async { let stub = StubAppSettingsService() stub.enqueueDevices(success: [device("a", name: "Old")]) @@ -60,8 +97,8 @@ final class DevicesViewModelTests: XCTestCase { await viewModel.makeMainWorkstation(target) - // Exactly one main workstation may exist, so the old one must flip - // locally without a second round-trip. + // Exactly one main workstation may exist and the server enforces the + // demotion, so the old one must flip locally without a second round-trip. XCTAssertEqual(stub.promotedIDs, ["new"]) XCTAssertTrue(viewModel.devices.first { $0.id == "new" }!.isMainWorkstation) XCTAssertFalse(viewModel.devices.first { $0.id == "old" }!.isMainWorkstation) @@ -70,7 +107,8 @@ final class DevicesViewModelTests: XCTestCase { func test_givenDevice_whenDeregistering_thenRemovesTheRow() async { let stub = StubAppSettingsService() stub.enqueueDevices(success: [device("a"), device("b")]) - stub.enqueueDeregister() + stub.enqueueRemoval(DeviceRemovalOutcome(deleted: true)) + stub.enqueueDevices(success: [device("b")]) let viewModel = makeViewModel(stub) await viewModel.load() @@ -80,8 +118,112 @@ final class DevicesViewModelTests: XCTestCase { XCTAssertEqual(stub.deregisteredIDs, ["a"]) } + func test_givenMainRemoved_whenServerNamesSuccessor_thenBadgeMovesWithoutGuessing() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("old-main", main: true), device("b"), device("c")]) + stub.enqueueRemoval(DeviceRemovalOutcome(deleted: true, promotedDeviceID: "c")) + // The reconcile read fails, so the only source for the new holder is the + // removal response itself. + stub.enqueueDevices(failure: URLError(.timedOut)) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.deregister(device("old-main", main: true)) + + XCTAssertFalse(viewModel.mainWorkstationIsUnknown, "the server named the successor, so nothing is unknown") + XCTAssertTrue(viewModel.devices.first { $0.id == "c" }!.isMainWorkstation) + XCTAssertFalse(viewModel.devices.first { $0.id == "b" }!.isMainWorkstation) + } + + func test_givenMachineSettings_whenCopyingToShared_thenReplacesSharedDocument() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a", hasSettings: true)]) + stub.enqueueSharedDocument(document(["theme": "dark"], version: 1)) + stub.enqueueCopyToShared(success: document(["syncFolder": "/Notes"], version: 2)) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.copySettingsToShared(from: viewModel.devices[0]) + + XCTAssertEqual(stub.copiedFromIDs, ["a"]) + XCTAssertEqual(viewModel.sharedDocument?.version, 2) + XCTAssertNil(viewModel.error) + } + + func test_givenSharedSettings_whenDeleting_thenClearsTheDocument() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a")]) + stub.enqueueSharedDocument(document(["theme": "dark"])) + stub.enqueueDeleteShared(true) + let viewModel = makeViewModel(stub) + await viewModel.load() + XCTAssertNotNil(viewModel.sharedDocument) + + await viewModel.deleteSharedSettings() + + XCTAssertEqual(stub.deleteSharedCallCount, 1) + XCTAssertNil(viewModel.sharedDocument) + } + + func test_givenDevice_whenInspecting_thenLoadsThatMachinesDocument() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a", hasSettings: true)]) + stub.enqueueDeviceDocument(document(["syncFolder": "/Notes"], version: 7)) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.inspect(viewModel.devices[0]) + + XCTAssertEqual(stub.inspectedIDs, ["a"]) + XCTAssertEqual(viewModel.inspection?.document?.version, 7) + XCTAssertFalse(viewModel.inspection?.isLoading ?? true) + XCTAssertEqual(viewModel.inspection?.document?.bag.keys, ["syncFolder"]) + } + + func test_givenThisMacIsNotRegistered_whenLoading_thenRegistersItOnce() async { + // Nothing else registers this machine, so the pane would otherwise list + // every computer except the one the user is sitting at. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("other", name: "Other Mac")]) + stub.enqueueMutation(success: device(thisMac, name: "Studio Mac")) + let viewModel = makeViewModel(stub, deviceName: "Studio Mac") + + await viewModel.load() + + XCTAssertEqual(stub.registeredIDs, [thisMac]) + XCTAssertEqual(viewModel.devices.map(\.id), [thisMac, "other"]) + } + // MARK: - Invalid input + func test_givenThisMacAlreadyRegistered_whenLoading_thenDoesNotReregisterAndClobberItsName() async { + // `POST …/devices` is an upsert keyed on deviceId — verified live — so + // re-registering would overwrite `deviceName` with this Mac's hostname + // and silently undo the user's rename on every visit to the pane. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device(thisMac, name: "Renamed By User")]) + let viewModel = makeViewModel(stub, deviceName: "studio-mac") + + await viewModel.load() + + XCTAssertTrue(stub.registeredIDs.isEmpty, "an already-registered Mac must not be re-registered") + XCTAssertEqual(viewModel.devices.first?.name, "Renamed By User") + } + + func test_givenRegistrationFails_whenLoading_thenStillShowsTheOtherMachines() async { + // The registry itself loaded. Failing to add this Mac must not blank the + // list of machines the user came here to manage. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("other", name: "Other Mac")]) + stub.enqueueMutation(failure: URLError(.timedOut)) + let viewModel = makeViewModel(stub, deviceName: "Studio Mac") + + await viewModel.load() + + XCTAssertEqual(viewModel.devices.map(\.id), ["other"]) + XCTAssertNil(viewModel.error) + } + func test_givenBlankOrUnchangedName_whenRenaming_thenSkipsTheRequest() async { let stub = StubAppSettingsService() stub.enqueueDevices(success: [device("a", name: "Same")]) @@ -105,7 +247,24 @@ final class DevicesViewModelTests: XCTestCase { XCTAssertTrue(stub.promotedIDs.isEmpty) } - func test_givenNoAppKeyRegistered_whenLoading_thenReportsUnavailable() async { + func test_givenDeviceThatNoLongerExists_whenPromoting_thenSurfacesErrorAndLeavesBadgesAlone() async { + // The machine was removed on another Mac between this pane's load and + // the click. The promotion 404s; no row may silently gain the badge. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a", main: true), device("ghost")]) + stub.enqueueMutation(failure: URLError(.badServerResponse)) + let viewModel = makeViewModel(stub) + await viewModel.load() + let ghost = viewModel.devices.first { $0.id == "ghost" }! + + await viewModel.makeMainWorkstation(ghost) + + XCTAssertNotNil(viewModel.error) + XCTAssertTrue(viewModel.devices.first { $0.id == "a" }!.isMainWorkstation) + XCTAssertFalse(viewModel.devices.first { $0.id == "ghost" }!.isMainWorkstation) + } + + func test_givenNoAppKeyConfigured_whenLoading_thenReportsUnavailable() async { let viewModel = makeViewModel(nil) await viewModel.load() @@ -128,10 +287,29 @@ final class DevicesViewModelTests: XCTestCase { XCTAssertTrue(viewModel.devices.isEmpty) } + func test_givenRemoveSucceedsButRefetchFails_thenRowGoesAndMainWorkstationShowsUnknown() async { + // The removal committed, so no error banner — but the server did not + // name a successor and the reconcile read failed, so this client cannot + // vouch for any badge. Showing the stale one would assert a fact that is + // now false. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("old-main", main: true), device("b")]) + stub.enqueueRemoval(DeviceRemovalOutcome(deleted: true, promotedDeviceID: nil)) + stub.enqueueDevices(failure: URLError(.timedOut)) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.deregister(device("old-main", main: true)) + + XCTAssertEqual(viewModel.devices.map(\.id), ["b"], "the removed row must disappear") + XCTAssertTrue(viewModel.mainWorkstationIsUnknown) + XCTAssertNil(viewModel.error, "the mutation succeeded; only the reconcile did not") + } + func test_givenDeregisterFailure_whenDeregistering_thenKeepsRowAndSurfacesError() async { let stub = StubAppSettingsService() stub.enqueueDevices(success: [device("a")]) - stub.enqueueDeregister(failure: URLError(.badServerResponse)) + stub.enqueueRemoval(failure: URLError(.badServerResponse)) let viewModel = makeViewModel(stub) await viewModel.load() @@ -141,6 +319,38 @@ final class DevicesViewModelTests: XCTestCase { XCTAssertNotNil(viewModel.error) } + func test_givenCopyConflict_whenCopyingToShared_thenKeepsOldDocumentAndSurfacesError() async { + // A lost compare-and-set writes nothing, so the pane must keep showing + // the document it has rather than pretending the replace happened. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a", hasSettings: true)]) + stub.enqueueSharedDocument(document(["theme": "dark"], version: 5)) + stub.enqueueCopyToShared(failure: AppSettingsError.versionConflict) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.copySettingsToShared(from: viewModel.devices[0]) + + XCTAssertEqual(viewModel.sharedDocument?.version, 5) + XCTAssertEqual(viewModel.error as? AppSettingsError, .versionConflict) + } + + func test_givenInspectFailure_whenInspecting_thenReportsInsideTheInspector() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a")]) + stub.enqueueDeviceDocument(failure: URLError(.timedOut)) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.inspect(viewModel.devices[0]) + + // Scoped to the inspector: a failed peek must not blank the pane's own + // error row or the list behind it. + XCTAssertNotNil(viewModel.inspection?.error) + XCTAssertNil(viewModel.error) + XCTAssertEqual(viewModel.devices.count, 1) + } + // MARK: - Empty / boundary func test_givenNoDevices_whenLoading_thenEmptyWithoutError() async { @@ -153,4 +363,67 @@ final class DevicesViewModelTests: XCTestCase { XCTAssertTrue(viewModel.devices.isEmpty) XCTAssertNil(viewModel.error) } + + func test_givenFirstRun404s_whenLoading_thenSharedSettingsAreEmptyNotAnError() async { + // 404 for an app key with nothing stored is the ordinary first-run + // state; the service maps it to nil and the pane must read that as + // "nothing shared yet" rather than a failure. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: []) + stub.enqueueSharedDocument(nil) + let viewModel = makeViewModel(stub) + + await viewModel.load() + + XCTAssertNil(viewModel.sharedDocument) + XCTAssertNil(viewModel.error) + XCTAssertFalse(viewModel.isUnavailable) + } + + func test_givenExactlyOneDevice_whenRemovingIt_thenRegistryEmptiesAndNothingIsPromoted() async { + // Boundary: the only registered machine, which is also the main + // workstation. Nothing remains to inherit the role, so "unknown" would + // be wrong — there is simply no holder. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("only", main: true)]) + stub.enqueueRemoval(DeviceRemovalOutcome(deleted: true, promotedDeviceID: nil)) + stub.enqueueDevices(success: []) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.deregister(device("only", main: true)) + + XCTAssertTrue(viewModel.devices.isEmpty) + XCTAssertFalse(viewModel.mainWorkstationIsUnknown) + XCTAssertNil(viewModel.error) + } + + func test_givenMachineWithNoSettings_whenInspecting_thenReportsNoneRatherThanFailing() async { + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a", hasSettings: false)]) + stub.enqueueDeviceDocument(nil) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.inspect(viewModel.devices[0]) + + XCTAssertNil(viewModel.inspection?.document) + XCTAssertNil(viewModel.inspection?.error) + XCTAssertFalse(viewModel.inspection?.isLoading ?? true) + } + + func test_givenAnActionInFlight_whenAnotherStarts_thenTheSecondIsIgnored() async { + // `busyID` marks exactly one row; a second mutation must not interleave + // and leave the flag stuck after the first one clears it. + let stub = StubAppSettingsService() + stub.enqueueDevices(success: [device("a", name: "One"), device("b", name: "Two")]) + stub.enqueueMutation(success: device("a", name: "Renamed")) + let viewModel = makeViewModel(stub) + await viewModel.load() + + await viewModel.rename(viewModel.devices[0], to: "Renamed") + + XCTAssertNil(viewModel.busyID, "the busy marker must clear on every exit path") + XCTAssertEqual(stub.renamedTo.count, 1) + } } diff --git a/AppTests/Support/StubSettingsClusterServices.swift b/AppTests/Support/StubSettingsClusterServices.swift index 13320ef..e98a6ec 100644 --- a/AppTests/Support/StubSettingsClusterServices.swift +++ b/AppTests/Support/StubSettingsClusterServices.swift @@ -92,15 +92,31 @@ final class StubNotificationPreferencesService: NotificationPreferencesServicing // MARK: - G17 app settings + device registry +/// Stub for the Applications pane (work-consolidation.md G17, GitHub issue #56). +/// +/// Every surface is queue-driven so a test can script a sequence — which the +/// remove path needs, because it lists the registry twice: once to load the +/// pane and again to reconcile after the deletion. final class StubAppSettingsService: AppSettingsServicing, @unchecked Sendable { private let lock = NSLock() private var deviceOutcomes: [Result<[AppDevice], Error>] = [] private var mutationOutcomes: [Result] = [] - private var deregisterOutcomes: [Result] = [] + private var removalOutcomes: [Result] = [] + private var sharedDocumentOutcomes: [Result] = [] + private var deviceDocumentOutcomes: [Result] = [] + private var copyOutcomes: [Result] = [] + private var deleteSharedOutcomes: [Result] = [] + private(set) var deregisteredIDs: [String] = [] private(set) var renamedTo: [String: String] = [:] private(set) var promotedIDs: [String] = [] + private(set) var inspectedIDs: [String] = [] + private(set) var registeredIDs: [String] = [] + private(set) var copiedFromIDs: [String] = [] + private(set) var deleteSharedCallCount = 0 + /// How many times the registry was listed — the remove path must reconcile. + private(set) var devicesCallCount = 0 func enqueueDevices(success: [AppDevice]) { lock.withLock { deviceOutcomes.append(.success(success)) } @@ -118,31 +134,109 @@ final class StubAppSettingsService: AppSettingsServicing, @unchecked Sendable { lock.withLock { mutationOutcomes.append(.failure(failure)) } } - func enqueueDeregister(failure: Error? = nil) { - lock.withLock { deregisterOutcomes.append(failure.map { .failure($0) } ?? .success(())) } + func enqueueRemoval(_ outcome: DeviceRemovalOutcome) { + lock.withLock { removalOutcomes.append(.success(outcome)) } + } + + func enqueueRemoval(failure: Error) { + lock.withLock { removalOutcomes.append(.failure(failure)) } + } + + func enqueueSharedDocument(_ document: AppSettingsDocument?) { + lock.withLock { sharedDocumentOutcomes.append(.success(document)) } + } + + func enqueueSharedDocument(failure: Error) { + lock.withLock { sharedDocumentOutcomes.append(.failure(failure)) } + } + + func enqueueDeviceDocument(_ document: AppSettingsDocument?) { + lock.withLock { deviceDocumentOutcomes.append(.success(document)) } + } + + func enqueueDeviceDocument(failure: Error) { + lock.withLock { deviceDocumentOutcomes.append(.failure(failure)) } + } + + func enqueueCopyToShared(success: AppSettingsDocument) { + lock.withLock { copyOutcomes.append(.success(success)) } + } + + func enqueueCopyToShared(failure: Error) { + lock.withLock { copyOutcomes.append(.failure(failure)) } } - // MARK: Settings surface — unused by the Devices pane, minimally satisfied. + func enqueueDeleteShared(_ deleted: Bool = true) { + lock.withLock { deleteSharedOutcomes.append(.success(deleted)) } + } + + func enqueueDeleteShared(failure: Error) { + lock.withLock { deleteSharedOutcomes.append(.failure(failure)) } + } + + // MARK: Settings documents + + func bootstrap(deviceID: String) async throws -> AppSettingsSeed { AppSettingsSeed() } - func bootstrap(deviceID: String) async throws -> AppSettingsSnapshot { AppSettingsSnapshot() } - func sharedSettings() async throws -> AppSettingsBag { AppSettingsBag() } - func writeSharedSettings(_ bag: AppSettingsBag) async throws -> AppSettingsBag { bag } - func deviceSettings(deviceID: String) async throws -> AppSettingsBag { AppSettingsBag() } - func writeDeviceSettings(_ bag: AppSettingsBag, deviceID: String) async throws -> AppSettingsBag { bag } + func sharedDocument() async throws -> AppSettingsDocument? { + try lock.withLock { + guard !sharedDocumentOutcomes.isEmpty else { return nil } + return try sharedDocumentOutcomes.removeFirst().get() + } + } + + func writeSharedSettings(_ bag: AppSettingsBag, baseVersion: Int) async throws -> AppSettingsDocument { + AppSettingsDocument(bag: bag, version: baseVersion + 1) + } + + @discardableResult + func deleteSharedSettings() async throws -> Bool { + try lock.withLock { + deleteSharedCallCount += 1 + guard !deleteSharedOutcomes.isEmpty else { return true } + return try deleteSharedOutcomes.removeFirst().get() + } + } + + func deviceDocument(deviceID: String) async throws -> AppSettingsDocument? { + try lock.withLock { + inspectedIDs.append(deviceID) + guard !deviceDocumentOutcomes.isEmpty else { return nil } + return try deviceDocumentOutcomes.removeFirst().get() + } + } + + func writeDeviceSettings( + _ bag: AppSettingsBag, + deviceID: String, + baseVersion: Int + ) async throws -> AppSettingsDocument { + AppSettingsDocument(bag: bag, version: baseVersion + 1, scope: .device(id: deviceID)) + } + + func copyDeviceSettingsToShared(deviceID: String) async throws -> AppSettingsDocument { + try lock.withLock { + copiedFromIDs.append(deviceID) + guard !copyOutcomes.isEmpty else { return AppSettingsDocument() } + return try copyOutcomes.removeFirst().get() + } + } // MARK: Device registry func devices() async throws -> [AppDevice] { try lock.withLock { + devicesCallCount += 1 guard !deviceOutcomes.isEmpty else { return [] } return try deviceOutcomes.removeFirst().get() } } - func registerDevice(deviceID: String, name: String?) async throws -> AppDevice { + func registerDevice(deviceID: String, name: String, platform: String) async throws -> AppDevice { try lock.withLock { + registeredIDs.append(deviceID) guard !mutationOutcomes.isEmpty else { - return AppDevice(id: deviceID, name: name ?? deviceID) + return AppDevice(id: deviceID, name: name, platform: platform) } return try mutationOutcomes.removeFirst().get() } @@ -166,11 +260,12 @@ final class StubAppSettingsService: AppSettingsServicing, @unchecked Sendable { } } - func deregisterDevice(deviceID: String) async throws { + @discardableResult + func deregisterDevice(deviceID: String) async throws -> DeviceRemovalOutcome { try lock.withLock { deregisteredIDs.append(deviceID) - guard !deregisterOutcomes.isEmpty else { return } - return try deregisterOutcomes.removeFirst().get() + guard !removalOutcomes.isEmpty else { return DeviceRemovalOutcome(deleted: true) } + return try removalOutcomes.removeFirst().get() } } } diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/AppSettingsBag.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/AppSettingsBag.swift index c3174c6..eb63ea9 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/AppSettingsBag.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Models/AppSettingsBag.swift @@ -27,6 +27,8 @@ public struct AppSettingsBag: Sendable, Equatable { public var isEmpty: Bool { storage.isEmpty } + public var count: Int { storage.count } + // MARK: - Typed access // // Each subscript reads through to the underlying value and returns nil when @@ -66,6 +68,17 @@ public struct AppSettingsBag: Sendable, Equatable { } } + /// The payload's size on the wire, in bytes. + /// + /// Settings ▸ Applications shows this because the stored blob is opaque and + /// capped server-side (`413 PayloadTooLarge` is a documented response), so + /// "how big is this" is the only meaningful thing the UI can say about a + /// payload it cannot interpret. Encoding failure reports 0 rather than + /// throwing: a size readout must never be the thing that breaks the pane. + public var byteSize: Int { + (try? JSONCoders.makeEncoder().encode(storage))?.count ?? 0 + } + // MARK: - Kit boundary /// The payload as the wire type. Internal — only the services in this @@ -73,84 +86,191 @@ public struct AppSettingsBag: Sendable, Equatable { var payload: [String: AppSettingsValue] { storage } } +// MARK: - Settings documents + +/// A stored settings document: a payload plus the metadata the compare-and-set +/// write protocol and the Applications pane both need (GitHub issue #56). +/// +/// `version` is carried through to the App layer deliberately. A caller that +/// wants to write must hand back the version it read, so hiding it would make +/// every write either impossible or unsafe. +public struct AppSettingsDocument: Sendable, Equatable { + + /// Which document this is. + public enum Scope: Sendable, Equatable { + /// The account-wide document, shared by every machine. + case account + /// One machine's pinned document. + case device(id: String) + } + + public var bag: AppSettingsBag + /// Monotonic version, to be sent back as `baseVersion` on write. + public let version: Int + public let updatedAt: Date? + public let scope: Scope + public let schemaVersion: Int? + + public init( + bag: AppSettingsBag = AppSettingsBag(), + version: Int = 0, + updatedAt: Date? = nil, + scope: Scope = .account, + schemaVersion: Int? = nil + ) { + self.bag = bag + self.version = version + self.updatedAt = updatedAt + self.scope = scope + self.schemaVersion = schemaVersion + } + + public var byteSize: Int { bag.byteSize } + public var isEmpty: Bool { bag.isEmpty } +} + +/// Where a launch bootstrap's settings came from. +/// +/// This mirrors the server's seeding precedence exactly: the machine's own +/// document, else the main workstation's, else the account-wide one, else +/// nothing. Worth surfacing because "your new Mac arrived pre-configured" is +/// only explicable if the app can say which machine it copied. +public enum AppSettingsOrigin: Sendable, Equatable { + case own + case mainWorkstation(deviceID: String?, deviceName: String?) + case account + case none +} + +/// The result of the one-call launch bootstrap. +public struct AppSettingsSeed: Sendable, Equatable { + public let origin: AppSettingsOrigin + /// Absent only when `origin == .none` — nothing is stored anywhere yet. + public let document: AppSettingsDocument? + + public init(origin: AppSettingsOrigin = .none, document: AppSettingsDocument? = nil) { + self.origin = origin + self.document = document + } + + /// True when the server had nothing to hand this machine — a genuine + /// first run for the whole account, not just this Mac. + public var isFirstRun: Bool { origin == .none } + + /// The settings to start from, empty when there are none. + public var bag: AppSettingsBag { document?.bag ?? AppSettingsBag() } +} + // MARK: - Devices /// A machine registered under the app key (work-consolidation.md G17). public struct AppDevice: Sendable, Equatable, Identifiable { public let id: String - /// Display name, falling back to the device id when unnamed. + /// Display name, falling back to the device id when the server sends a + /// blank one. public let name: String /// The machine whose configuration seeds a brand-new device on first - /// sign-in. Exactly one device should carry this. + /// sign-in. Exactly one device carries this, enforced server-side: promoting + /// one demotes the previous holder. + /// + /// Named for what it means rather than its wire spelling (`isDefault`), + /// which does not say what it is the default *for*. public let isMainWorkstation: Bool - public let createdAt: Date? + public let platform: String? public let lastSeenAt: Date? + public let appVersion: String? + public let osVersion: String? + /// Whether this machine has a per-device settings document of its own. + /// `nil` when the server did not say — the single-device responses from + /// register and rename omit it, so it must not be read as "no". + public let hasDeviceSettings: Bool? public init( id: String, name: String, isMainWorkstation: Bool = false, - createdAt: Date? = nil, - lastSeenAt: Date? = nil + platform: String? = nil, + lastSeenAt: Date? = nil, + appVersion: String? = nil, + osVersion: String? = nil, + hasDeviceSettings: Bool? = nil ) { self.id = id self.name = name self.isMainWorkstation = isMainWorkstation - self.createdAt = createdAt + self.platform = platform self.lastSeenAt = lastSeenAt + self.appVersion = appVersion + self.osVersion = osVersion + self.hasDeviceSettings = hasDeviceSettings + } + + /// A copy with the main-workstation flag flipped, so the promote/demote + /// pair can be reflected locally without rebuilding every field by hand at + /// the call site (and silently dropping one when a field is added). + public func settingMainWorkstation(_ isMain: Bool) -> AppDevice { + AppDevice( + id: id, + name: name, + isMainWorkstation: isMain, + platform: platform, + lastSeenAt: lastSeenAt, + appVersion: appVersion, + osVersion: osVersion, + hasDeviceSettings: hasDeviceSettings + ) } } -/// The result of the one-call launch bootstrap: account-wide settings plus this -/// machine's own, and whether the server had to seed a new device. -public struct AppSettingsSnapshot: Sendable, Equatable { - public var shared: AppSettingsBag - public var device: AppSettingsBag - /// True when the server had not seen this `deviceId` before. - public let isNewDevice: Bool - /// True when the new device's settings were seeded from the main - /// workstation — worth surfacing once, so the user knows why their new Mac - /// arrived pre-configured. - public let seededFromMainWorkstation: Bool +/// What the server did in response to a deregistration. +/// +/// `promotedDeviceID` exists because removing the main workstation makes the +/// server pick a successor, and it reports which one. Without this the client +/// would have to guess or refetch blind — and a failed refetch would leave a +/// stale badge pointing at a machine that no longer exists. +public struct DeviceRemovalOutcome: Sendable, Equatable { + /// False when there was nothing to delete. + public let deleted: Bool + /// The machine promoted to main workstation to replace the removed one, or + /// nil when the removed device was not the main workstation (or was the + /// last one registered). + public let promotedDeviceID: String? - public init( - shared: AppSettingsBag = AppSettingsBag(), - device: AppSettingsBag = AppSettingsBag(), - isNewDevice: Bool = false, - seededFromMainWorkstation: Bool = false - ) { - self.shared = shared - self.device = device - self.isNewDevice = isNewDevice - self.seededFromMainWorkstation = seededFromMainWorkstation + public init(deleted: Bool, promotedDeviceID: String? = nil) { + self.deleted = deleted + self.promotedDeviceID = promotedDeviceID } } -// MARK: - Mapping +// MARK: - Errors -extension AppSettingsBag { - init(from dto: AppSettingsDTO) { self.init(storage: dto.settings) } -} +/// Failures specific to the app-settings surface that callers must be able to +/// tell apart from generic transport errors, because each needs a different +/// response from the user. +public enum AppSettingsError: LocalizedError, Equatable { -extension AppDevice { - public init(from dto: AppDeviceDTO) { - self.init( - id: dto.deviceId, - name: dto.name ?? dto.deviceId, - isMainWorkstation: dto.isMainWorkstation ?? false, - createdAt: dto.createdAt, - lastSeenAt: dto.lastSeenAt - ) - } -} + /// A compare-and-set write lost: the stored document moved on since it was + /// read, and **nothing was written**. + case versionConflict -extension AppSettingsSnapshot { - init(from dto: AppSettingsBootstrapDTO) { - self.init( - shared: AppSettingsBag(storage: dto.shared), - device: AppSettingsBag(storage: dto.device), - isNewDevice: dto.isNewDevice ?? false, - seededFromMainWorkstation: dto.seededFromMainWorkstation ?? false - ) + /// A per-device settings write was addressed to a machine that is not in + /// the registry. Retrying cannot fix this; registering the device can. + case deviceNotRegistered + + /// A copy-to-shared was asked for from a machine that has stored no + /// settings of its own. Refused rather than treated as an empty payload, + /// because copying "nothing" would wipe the shared settings — the write + /// replaces wholesale. + case noSettingsToCopy + + public var errorDescription: String? { + switch self { + case .versionConflict: + return "These settings changed on another machine. Reload and try again." + case .deviceNotRegistered: + return "That machine is no longer registered, so its settings could not be saved." + case .noSettingsToCopy: + return "That machine has no settings of its own to copy." + } } } diff --git a/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/AppSettingsService.swift b/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/AppSettingsService.swift index b758895..785e64a 100644 --- a/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/AppSettingsService.swift +++ b/Packages/InterlinedDomain/Sources/InterlinedDomain/Services/AppSettingsService.swift @@ -2,39 +2,71 @@ import Foundation import InterlinedKit /// The synced-settings + device-registry surface the App layer codes against -/// (work-consolidation.md G17). +/// (work-consolidation.md G17, GitHub issue #56). /// /// This is the sanctioned home for the app's preferences *and* the Document /// Sync Agent's per-machine configuration, replacing purely local -/// `UserDefaults` state: **shared** settings follow the account to every +/// `UserDefaults` state: **account** settings follow the account to every /// machine, **device** settings stay pinned to one computer. +/// +/// Reads answer an optional document: `nil` means "nothing stored yet", which +/// is the ordinary first-run state rather than a failure. Writes are +/// compare-and-set and therefore take the `baseVersion` the caller last read. public protocol AppSettingsServicing: Sendable { - /// The one launch call: shared + this machine's settings, seeding a new - /// device from the main workstation when the server has not seen it before. - func bootstrap(deviceID: String) async throws -> AppSettingsSnapshot - func sharedSettings() async throws -> AppSettingsBag - func writeSharedSettings(_ bag: AppSettingsBag) async throws -> AppSettingsBag + /// The one launch call: the best settings this machine can start from, plus + /// where they came from. + func bootstrap(deviceID: String) async throws -> AppSettingsSeed + + // MARK: Account-wide settings + + /// The account-wide document, or nil when none is stored. + func sharedDocument() async throws -> AppSettingsDocument? + /// Replaces the account-wide document. `baseVersion` is the version last + /// read, or 0 to create. + func writeSharedSettings(_ bag: AppSettingsBag, baseVersion: Int) async throws -> AppSettingsDocument + /// Deletes the account-wide document. Returns false when there was nothing + /// to delete. Per-device documents are unaffected. + @discardableResult + func deleteSharedSettings() async throws -> Bool + + // MARK: Per-device settings - func deviceSettings(deviceID: String) async throws -> AppSettingsBag - func writeDeviceSettings(_ bag: AppSettingsBag, deviceID: String) async throws -> AppSettingsBag + /// One machine's document, or nil when that machine has stored none. + func deviceDocument(deviceID: String) async throws -> AppSettingsDocument? + func writeDeviceSettings( + _ bag: AppSettingsBag, + deviceID: String, + baseVersion: Int + ) async throws -> AppSettingsDocument + + /// Copies one machine's settings over the account-wide document, + /// **replacing** it wholesale. + func copyDeviceSettingsToShared(deviceID: String) async throws -> AppSettingsDocument + + // MARK: Device registry func devices() async throws -> [AppDevice] - func registerDevice(deviceID: String, name: String?) async throws -> AppDevice + func registerDevice(deviceID: String, name: String, platform: String) async throws -> AppDevice func renameDevice(deviceID: String, to name: String) async throws -> AppDevice func makeMainWorkstation(deviceID: String) async throws -> AppDevice - func deregisterDevice(deviceID: String) async throws + @discardableResult + func deregisterDevice(deviceID: String) async throws -> DeviceRemovalOutcome +} + +public extension AppSettingsServicing { + /// macOS is the only platform this client runs on, so callers rarely care. + func registerDevice(deviceID: String, name: String) async throws -> AppDevice { + try await registerDevice(deviceID: deviceID, name: name, platform: "macos") + } } /// Talks to `/api/user/app-settings/{appKey}/…`. /// -/// ⚠️ The `appKey` must be **registered with the backend owner** before this -/// ships (stated prerequisite in the G17 definition). It is injected at the -/// composition root rather than hard-coded here, so changing it is a one-line -/// edit and tests can use their own key. -/// -/// ⚠️ The live response shapes are unverified — the DTOs decode tolerantly and -/// should be tightened after a live probe. +/// The `appKey` is injected at the composition root rather than hard-coded, so +/// tests can use their own. The production value +/// (`AppEnvironment.appSettingsKey`) **must not change**: the key is the +/// namespace every stored setting lives under, and a new one orphans all of it. public final class AppSettingsService: AppSettingsServicing { private let api: APIClientProtocol @@ -47,72 +79,92 @@ public final class AppSettingsService: AppSettingsServicing { // MARK: - Bootstrap - public func bootstrap(deviceID: String) async throws -> AppSettingsSnapshot { + public func bootstrap(deviceID: String) async throws -> AppSettingsSeed { do { let dto = try await api.send(AppSettings.bootstrap(appKey: appKey, deviceId: deviceID)) - return AppSettingsSnapshot(from: dto) + return AppSettingsSeed(from: dto) } catch let error as APIError { - // Verified live 2026-09-06: an unregistered device answers - // `404 {"source":"none"}`. That is the ordinary first-run state, not - // a failure — a brand-new Mac has nothing stored yet — so it maps to - // an empty snapshot flagged `isNewDevice`, and the caller registers. + // Verified live 2026-09-16: with nothing stored anywhere the route + // answers `404 {"source":"none"}`. That is the ordinary first-run + // state for the whole account — not a failure — so it maps to an + // empty seed and the caller just starts from defaults. guard case .notFound = error else { throw error } - return AppSettingsSnapshot(isNewDevice: true) + return AppSettingsSeed(origin: .none) } } - // MARK: - Shared settings + // MARK: - Account-wide settings - public func sharedSettings() async throws -> AppSettingsBag { - // Verified live 2026-09-06: an app key with nothing stored yet answers - // 404, while `OPTIONS` on the same path reports + public func sharedDocument() async throws -> AppSettingsDocument? { + // Verified live 2026-09-16: an app key with nothing stored answers 404, + // while `OPTIONS` on the same path reports // `allow: DELETE, GET, HEAD, OPTIONS, PUT` — the route exists, the // bucket is simply empty. Treating that as an error would make every // fresh account show a failure instead of empty settings. - try await emptyOnNotFound { AppSettings.shared(appKey: self.appKey) } + try await documentOrNil { AppSettings.shared(appKey: self.appKey) } } - public func writeSharedSettings(_ bag: AppSettingsBag) async throws -> AppSettingsBag { - let dto = try await api.send( - AppSettings.writeShared(appKey: appKey, WriteAppSettingsRequest(settings: bag.payload)) - ) - return AppSettingsBag(from: dto) + public func writeSharedSettings( + _ bag: AppSettingsBag, + baseVersion: Int + ) async throws -> AppSettingsDocument { + try await write { + AppSettings.writeShared( + appKey: self.appKey, + WriteAppSettingsRequest(settings: bag.payload, baseVersion: baseVersion) + ) + } + } + + @discardableResult + public func deleteSharedSettings() async throws -> Bool { + // No 404 mapping here on purpose: verified live, this delete is + // idempotent and answers `{"deleted":false}` rather than 404 when there + // was nothing stored. A 404 from this route would be a real fault. + try await api.send(AppSettings.deleteShared(appKey: appKey)).deleted } // MARK: - Per-device settings - public func deviceSettings(deviceID: String) async throws -> AppSettingsBag { - try await emptyOnNotFound { + public func deviceDocument(deviceID: String) async throws -> AppSettingsDocument? { + try await documentOrNil { AppSettings.deviceSettings(appKey: self.appKey, deviceId: deviceID) } } - /// Sends a settings read, mapping the "nothing stored yet" 404 to an empty - /// bag. Every other error still propagates — a 401 or a 500 is a real - /// failure and must reach the UI. - private func emptyOnNotFound( - _ build: @Sendable () -> Request - ) async throws -> AppSettingsBag { + public func writeDeviceSettings( + _ bag: AppSettingsBag, + deviceID: String, + baseVersion: Int + ) async throws -> AppSettingsDocument { do { - return AppSettingsBag(from: try await api.send(build())) + return try await write { + AppSettings.writeDeviceSettings( + appKey: self.appKey, + deviceId: deviceID, + WriteAppSettingsRequest(settings: bag.payload, baseVersion: baseVersion) + ) + } } catch let error as APIError { + // A 404 from the *write* route is not "no document yet" — + // `baseVersion: 0` creates one happily. It means the device is not + // registered (`{"error":"device not registered"}`), which retrying + // will never fix. Mapping it to an empty document the way the read + // path does would silently discard the user's settings. guard case .notFound = error else { throw error } - return AppSettingsBag() + throw AppSettingsError.deviceNotRegistered } } - public func writeDeviceSettings( - _ bag: AppSettingsBag, - deviceID: String - ) async throws -> AppSettingsBag { - let dto = try await api.send( - AppSettings.writeDeviceSettings( - appKey: appKey, - deviceId: deviceID, - WriteAppSettingsRequest(settings: bag.payload) - ) - ) - return AppSettingsBag(from: dto) + public func copyDeviceSettingsToShared(deviceID: String) async throws -> AppSettingsDocument { + guard let source = try await deviceDocument(deviceID: deviceID) else { + throw AppSettingsError.noSettingsToCopy + } + // Read the destination purely for its version. The write is a + // compare-and-set replace, so it needs the version that is current + // *now*, not one cached from an earlier screen refresh. + let destinationVersion = try await sharedDocument()?.version ?? 0 + return try await writeSharedSettings(source.bag, baseVersion: destinationVersion) } // MARK: - Device registry @@ -122,37 +174,147 @@ public final class AppSettingsService: AppSettingsServicing { return response.devices.map(AppDevice.init(from:)) } - public func registerDevice(deviceID: String, name: String?) async throws -> AppDevice { - let dto = try await api.send( + // No default for `platform` here: the protocol extension already supplies + // the two-argument spelling, and a default would make both visible and the + // call ambiguous. + public func registerDevice( + deviceID: String, + name: String, + platform: String + ) async throws -> AppDevice { + let envelope = try await api.send( AppSettings.registerDevice( appKey: appKey, - RegisterDeviceRequest(deviceId: deviceID, name: name) + RegisterDeviceRequest(deviceId: deviceID, deviceName: name, platform: platform) ) ) - return AppDevice(from: dto) + return AppDevice(from: envelope.device) } public func renameDevice(deviceID: String, to name: String) async throws -> AppDevice { - let dto = try await api.send( - AppSettings.updateDevice(appKey: appKey, deviceId: deviceID, UpdateDeviceRequest(name: name)) + let envelope = try await api.send( + AppSettings.updateDevice( + appKey: appKey, + deviceId: deviceID, + UpdateDeviceRequest(deviceName: name) + ) ) - return AppDevice(from: dto) + return AppDevice(from: envelope.device) } public func makeMainWorkstation(deviceID: String) async throws -> AppDevice { - // Sends only the flag — a PATCH that also carried `name` would clobber a - // rename made on another machine between this client's read and write. - let dto = try await api.send( + // Sends only the flag — a PATCH that also carried `deviceName` would + // clobber a rename made on another machine between this client's read + // and its write. + let envelope = try await api.send( AppSettings.updateDevice( appKey: appKey, deviceId: deviceID, - UpdateDeviceRequest(isMainWorkstation: true) + UpdateDeviceRequest(isDefault: true) ) ) - return AppDevice(from: dto) + return AppDevice(from: envelope.device) + } + + @discardableResult + public func deregisterDevice(deviceID: String) async throws -> DeviceRemovalOutcome { + let response = try await api.send(AppSettings.deleteDevice(appKey: appKey, deviceId: deviceID)) + return DeviceRemovalOutcome( + deleted: response.deleted, + promotedDeviceID: response.promotedDeviceId + ) + } + + // MARK: - Shared plumbing + + /// Sends a settings read, mapping the "nothing stored yet" 404 to nil. + /// Every other error still propagates — a 401 or a 500 is a real failure + /// and must reach the UI. + private func documentOrNil( + _ build: @Sendable () -> Request + ) async throws -> AppSettingsDocument? { + do { + return AppSettingsDocument(from: try await api.send(build())) + } catch let error as APIError { + guard case .notFound = error else { throw error } + return nil + } + } + + /// Sends a compare-and-set write, translating the 409 into a domain error. + /// + /// The 409 body carries the winning document under `current`, which would in + /// principle let the client merge and retry without a re-read. It is + /// deliberately **not** plumbed through: `APIClient` reduces every non-2xx + /// body to a message string, so surfacing `current` means teaching the + /// client to carry typed error payloads — a change to every endpoint's + /// error path, far outside this pane. Re-reading costs one request and the + /// blob is opaque, so there is nothing to merge field-by-field anyway. + private func write( + _ build: @Sendable () -> Request + ) async throws -> AppSettingsDocument { + do { + return AppSettingsDocument(from: try await api.send(build())) + } catch let error as APIError { + // 409 is not one of the statuses `APIError` narrows, so it arrives + // as `.httpStatus`. + guard case .httpStatus(let code, _) = error, code == 409 else { throw error } + throw AppSettingsError.versionConflict + } + } +} + +// MARK: - Mapping + +extension AppSettingsDocument { + init(from dto: AppSettingsDocumentDTO) { + self.init( + bag: AppSettingsBag(storage: dto.settings), + version: dto.version, + updatedAt: dto.updatedAt, + // The wire says `"account"` for the user-wide document and + // `"device"` for a pinned one; anything unrecognised is treated as + // account-wide, which is the safe reading — a device document + // always names its device. + scope: dto.deviceId.map(AppSettingsDocument.Scope.device(id:)) ?? .account, + schemaVersion: dto.schemaVersion + ) } +} - public func deregisterDevice(deviceID: String) async throws { - try await api.sendVoid(AppSettings.deleteDevice(appKey: appKey, deviceId: deviceID)) +extension AppDevice { + public init(from dto: AppDeviceDTO) { + self.init( + id: dto.deviceId, + // A blank name would render as an empty row with nothing to click, + // so fall back to the id the user can at least match against. + name: dto.deviceName.isEmpty ? dto.deviceId : dto.deviceName, + isMainWorkstation: dto.isDefault, + platform: dto.platform, + lastSeenAt: dto.lastSeenAt, + appVersion: dto.appVersion, + osVersion: dto.osVersion, + hasDeviceSettings: dto.hasDeviceSettings + ) + } +} + +extension AppSettingsSeed { + init(from dto: AppSettingsBootstrapDTO) { + let origin: AppSettingsOrigin + switch dto.source { + case .own: + origin = .own + case .defaultDevice: + origin = .mainWorkstation( + deviceID: dto.defaultDeviceId, + deviceName: dto.defaultDeviceName + ) + case .account: + origin = .account + case .none: + origin = .none + } + self.init(origin: origin, document: dto.document.map(AppSettingsDocument.init(from:))) } } diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/AppSettingsServiceTests.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/AppSettingsServiceTests.swift index a994801..1235cd1 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/AppSettingsServiceTests.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/AppSettingsServiceTests.swift @@ -3,7 +3,10 @@ import InterlinedKit @testable import InterlinedDomain /// BDD coverage for `AppSettingsService`, `AppSettingsBag` and the device -/// registry (work-consolidation.md G17). +/// registry (work-consolidation.md G17, GitHub issue #56). +/// +/// The fixtures are the live shapes captured 2026-09-16; see +/// `AppSettingsEndpointTests` for the verbatim probe bodies they come from. final class AppSettingsServiceTests: XCTestCase { private let appKey = "interlinedlist-macos" @@ -12,209 +15,394 @@ final class AppSettingsServiceTests: XCTestCase { AppSettingsService(api: api, appKey: appKey) } + private func documentJSON(version: Int = 1, settings: String = #"{"theme":"dark"}"#) -> String { + """ + { "appKey": "interlinedlist-macos", "scope": "account", "deviceId": null, + "version": \(version), "updatedAt": "2026-09-16T19:39:07.135Z", + "schemaVersion": 1, "settings": \(settings) } + """ + } + // MARK: - Happy path - func test_givenBootstrapBody_whenLaunching_thenSplitsSharedAndDeviceSettings() async throws { + func test_givenOwnDocument_whenBootstrapping_thenReportsOriginAndSettings() async throws { let api = StubAPIClient() await api.enqueue(json: #""" - { "shared": { "theme": "dark", "postsPerPage": 25 }, - "device": { "syncFolder": "/Users/x/Notes" }, - "isNewDevice": true, "seededFromMainWorkstation": true } + { "source": "self", "appKey": "interlinedlist-macos", "scope": "device", + "deviceId": "dev-1", "version": 3, "updatedAt": "2026-09-16T19:39:18.084Z", + "schemaVersion": 1, + "settings": { "theme": "dark", "postsPerPage": 25, "syncFolder": "/Users/x/Notes" } } """#) let service = makeService(api) - let snapshot = try await service.bootstrap(deviceID: "dev-1") + let seed = try await service.bootstrap(deviceID: "dev-1") - XCTAssertEqual(snapshot.shared[string: "theme"], "dark") - XCTAssertEqual(snapshot.shared[int: "postsPerPage"], 25) - XCTAssertEqual(snapshot.device[string: "syncFolder"], "/Users/x/Notes") - XCTAssertTrue(snapshot.isNewDevice) - XCTAssertTrue(snapshot.seededFromMainWorkstation) + XCTAssertEqual(seed.origin, .own) + XCTAssertFalse(seed.isFirstRun) + XCTAssertEqual(seed.bag[string: "theme"], "dark") + XCTAssertEqual(seed.bag[int: "postsPerPage"], 25) + XCTAssertEqual(seed.document?.version, 3) let recorded = await api.recorded XCTAssertEqual(recorded.first?.path, "/api/user/app-settings/interlinedlist-macos/bootstrap") XCTAssertEqual(recorded.first?.query["deviceId"], "dev-1") } + func test_givenNewDevice_whenBootstrapping_thenNamesTheSeedingWorkstation() async throws { + let api = StubAPIClient() + await api.enqueue(json: #""" + { "source": "default-device", "appKey": "interlinedlist-macos", "scope": "device", + "deviceId": "dev-main", "version": 1, "updatedAt": "2026-09-16T19:39:18.084Z", + "settings": { "theme": "dark" }, + "defaultDeviceId": "dev-main", "defaultDeviceName": "Studio Mac" } + """#) + let service = makeService(api) + + let seed = try await service.bootstrap(deviceID: "brand-new") + + // A new Mac arriving pre-configured is only explicable if the app can + // say which machine it was seeded from. + XCTAssertEqual(seed.origin, .mainWorkstation(deviceID: "dev-main", deviceName: "Studio Mac")) + XCTAssertEqual(seed.bag[string: "theme"], "dark") + } + func test_givenDevices_whenListing_thenMapsRegistry() async throws { let api = StubAPIClient() await api.enqueue(json: #""" - { "devices": [ { "deviceId": "dev-1", "name": "Studio Mac", "isMainWorkstation": true }, - { "deviceId": "dev-2" } ] } + { "devices": [ { "deviceId": "dev-1", "deviceName": "Studio Mac", "platform": "macos", + "isDefault": true, "lastSeenAt": "2026-09-16T19:38:42.214Z", + "hasDeviceSettings": true }, + { "deviceId": "dev-2", "deviceName": "Laptop", "isDefault": false, + "hasDeviceSettings": false } ] } """#) let service = makeService(api) let devices = try await service.devices() XCTAssertEqual(devices.map(\.id), ["dev-1", "dev-2"]) + // `isDefault` on the wire is the main-workstation role in the domain. XCTAssertTrue(devices[0].isMainWorkstation) - // An unnamed device falls back to its id so a row is never blank. - XCTAssertEqual(devices[1].name, "dev-2") XCTAssertFalse(devices[1].isMainWorkstation) + XCTAssertEqual(devices[0].name, "Studio Mac") + XCTAssertEqual(devices[0].platform, "macos") + XCTAssertEqual(devices[0].hasDeviceSettings, true) + } + + func test_givenRename_whenRenaming_thenUnwrapsTheDeviceEnvelope() async throws { + let api = StubAPIClient() + await api.enqueue(json: #""" + { "device": { "deviceId": "dev-2", "deviceName": "Renamed", "isDefault": false } } + """#) + let service = makeService(api) + + let device = try await service.renameDevice(deviceID: "dev-2", to: "Renamed") + + XCTAssertEqual(device.name, "Renamed") + let recorded = await api.recorded + XCTAssertEqual(recorded.first?.method, "PATCH") } func test_givenPromotion_whenMakingMainWorkstation_thenSendsOnlyTheFlag() async throws { let api = StubAPIClient() - await api.enqueue(json: #"{ "deviceId": "dev-2", "isMainWorkstation": true }"#) + await api.enqueue(json: #""" + { "device": { "deviceId": "dev-2", "deviceName": "Laptop", "isDefault": true } } + """#) let service = makeService(api) let device = try await service.makeMainWorkstation(deviceID: "dev-2") XCTAssertTrue(device.isMainWorkstation) let recorded = await api.recorded - XCTAssertEqual(recorded.first?.method, "PATCH") - XCTAssertEqual(recorded.first?.path, "/api/user/app-settings/interlinedlist-macos/devices/dev-2") + let body = recorded.first?.bodyJSON + XCTAssertEqual(body?["isDefault"] as? Bool, true) + // A promotion that also sent the name would clobber a rename made on + // another machine between this client's read and its write. + XCTAssertNil(body?["deviceName"]) } - func test_givenDeviceID_whenDeregistering_thenSendsDelete() async throws { + func test_givenMainWorkstationRemoved_whenDeregistering_thenReportsThePromotedMachine() async throws { let api = StubAPIClient() - await api.enqueue(json: "{}") + await api.enqueue(json: #"{"deleted":true,"promotedDeviceId":"dev-1"}"#) let service = makeService(api) - try await service.deregisterDevice(deviceID: "dev-2") + let outcome = try await service.deregisterDevice(deviceID: "dev-2") - let recorded = await api.recorded - XCTAssertEqual(recorded.first?.method, "DELETE") + XCTAssertTrue(outcome.deleted) + // The server names the successor, so the client never has to guess who + // inherited the role. + XCTAssertEqual(outcome.promotedDeviceID, "dev-1") } - // MARK: - Invalid / forward-compatibility - // - // The behaviour that matters most for synced settings: an older build must - // not delete keys written by a newer one. + func test_givenStoredDocument_whenWritingShared_thenSendsTheVersionItRead() async throws { + let api = StubAPIClient() + await api.enqueue(json: documentJSON(version: 4, settings: #"{"theme":"light"}"#)) + let service = makeService(api) + var bag = AppSettingsBag() + bag[string: "theme"] = "light" - func test_givenUnknownKeys_whenEditingAndWritingBack_thenUnknownKeysSurvive() async throws { + let saved = try await service.writeSharedSettings(bag, baseVersion: 3) + + XCTAssertEqual(saved.version, 4) + let recorded = await api.recorded + // Omitting baseVersion is a hard 400 on this API. + XCTAssertEqual(recorded.first?.bodyJSON?["baseVersion"] as? Int, 3) + } + + func test_givenDeviceSettings_whenCopyingToShared_thenReplacesUsingTheSharedVersion() async throws { let api = StubAPIClient() + // 1. read the source device document await api.enqueue(json: #""" - { "settings": { "theme": "dark", "futureFeature": { "nested": true } } } - """#) - await api.enqueue(json: #""" - { "settings": { "theme": "light", "futureFeature": { "nested": true } } } + { "appKey": "interlinedlist-macos", "scope": "device", "deviceId": "dev-1", + "version": 9, "updatedAt": "2026-09-16T19:39:18.084Z", + "settings": { "syncFolder": "/Users/x/Notes" } } """#) + // 2. read the destination for its version + await api.enqueue(json: documentJSON(version: 2)) + // 3. the write + await api.enqueue(json: documentJSON(version: 3, settings: #"{"syncFolder":"/Users/x/Notes"}"#)) let service = makeService(api) - var bag = try await service.sharedSettings() - XCTAssertEqual(bag[string: "theme"], "dark") - bag[string: "theme"] = "light" + let result = try await service.copyDeviceSettingsToShared(deviceID: "dev-1") + + XCTAssertEqual(result.bag[string: "syncFolder"], "/Users/x/Notes") + let recorded = await api.recorded + XCTAssertEqual(recorded.count, 3) + // The write must carry the DESTINATION's version (2), not the source + // document's (9) — they are independent documents with independent + // version counters, and sending the source's would lose the write. + XCTAssertEqual(recorded[2].bodyJSON?["baseVersion"] as? Int, 2) + XCTAssertEqual(recorded[2].method, "PUT") + XCTAssertEqual(recorded[2].path, "/api/user/app-settings/interlinedlist-macos") + } + + func test_givenStoredSettings_whenDeletingShared_thenReportsDeletion() async throws { + let api = StubAPIClient() + await api.enqueue(json: #"{"deleted":true}"#) + let service = makeService(api) + + let deleted = try await service.deleteSharedSettings() + + XCTAssertTrue(deleted) + let recorded = await api.recorded + XCTAssertEqual(recorded.first?.method, "DELETE") + XCTAssertEqual(recorded.first?.path, "/api/user/app-settings/interlinedlist-macos") + } + + // MARK: - Bag semantics + + func test_givenUnknownKeys_whenEditingAndWritingBack_thenUnknownKeysSurvive() async throws { + // The forward-compatibility guarantee: an older build must not delete a + // newer build's settings just by saving. + let api = StubAPIClient() + await api.enqueue(json: documentJSON( + version: 1, + settings: #"{"theme":"dark","futureFeature":{"nested":[1,2]}}"# + )) + await api.enqueue(json: documentJSON( + version: 2, + settings: #"{"theme":"light","futureFeature":{"nested":[1,2]}}"# + )) + let service = makeService(api) - let saved = try await service.writeSharedSettings(bag) + let loaded = try await service.sharedDocument() + var document = try XCTUnwrap(loaded) + document.bag[string: "theme"] = "light" + let saved = try await service.writeSharedSettings(document.bag, baseVersion: document.version) - XCTAssertEqual(saved[string: "theme"], "light") - XCTAssertTrue(saved.keys.contains("futureFeature"), - "a key this build does not understand must survive a save") + XCTAssertEqual(saved.bag[string: "theme"], "light") + let recorded = await api.recorded + let sent = recorded[1].bodyJSON?["settings"] as? [String: Any] + XCTAssertNotNil(sent?["futureFeature"], "a key this build does not understand must survive the round trip") } func test_givenWrongTypeOrMissingKey_whenReading_thenReturnsNilRatherThanTrapping() async throws { let api = StubAPIClient() - await api.enqueue(json: #"{ "settings": { "postsPerPage": "twenty-five" } }"#) + await api.enqueue(json: documentJSON(settings: #"{"count":"not-a-number"}"#)) let service = makeService(api) - let bag = try await service.sharedSettings() + let loaded = try await service.sharedDocument() + let document = try XCTUnwrap(loaded) - XCTAssertNil(bag[int: "postsPerPage"], "a type change degrades to unset") - XCTAssertEqual(bag[string: "postsPerPage"], "twenty-five") - XCTAssertNil(bag[bool: "neverSet"]) + XCTAssertNil(document.bag[int: "count"], "a type change must degrade to unset, not crash") + XCTAssertNil(document.bag[string: "absent"]) } func test_givenNilAssignment_whenWriting_thenRemovesTheKey() { var bag = AppSettingsBag() - bag[bool: "syncEnabled"] = true - XCTAssertEqual(bag[bool: "syncEnabled"], true) + bag[string: "theme"] = "dark" + XCTAssertEqual(bag.keys, ["theme"]) - bag[bool: "syncEnabled"] = nil + bag[string: "theme"] = nil - XCTAssertFalse(bag.keys.contains("syncEnabled"), "unset must round-trip as absence, not null") + // "Unset" must round-trip as absence, not as a JSON null. XCTAssertTrue(bag.isEmpty) } + func test_givenPayload_whenMeasuringSize_thenReportsEncodedBytes() { + var bag = AppSettingsBag() + bag[string: "theme"] = "dark" + + // The pane shows a size because the blob is opaque to this app as well + // as to the server, and the server caps it. + XCTAssertGreaterThan(bag.byteSize, 0) + XCTAssertEqual(AppSettingsBag().byteSize, 2, "an empty payload encodes as {}") + } + // MARK: - Upstream failure func test_givenForbidden_whenFetching_thenThrows() async { - // Replaces an earlier test that asserted a 404 must throw. The live probe - // showed 404 is the empty-bucket state, not an unknown-key rejection, so - // the meaningful auth failure to cover here is 403. + // 404 is the empty-bucket state, not an auth rejection, so the + // meaningful failure to cover here is 403. let api = StubAPIClient() await api.enqueue(failure: .forbidden(serverMessage: "nope")) let service = makeService(api) do { - _ = try await service.sharedSettings() + _ = try await service.sharedDocument() XCTFail("Expected the failure to propagate") } catch { // expected } } + func test_givenStaleVersion_whenWriting_thenThrowsVersionConflict() async { + // A lost compare-and-set writes nothing, so the caller must be told to + // reload rather than shown a generic HTTP error it cannot act on. + let api = StubAPIClient() + await api.enqueue(failure: .httpStatus(code: 409, serverMessage: "version_conflict")) + let service = makeService(api) + + do { + _ = try await service.writeSharedSettings(AppSettingsBag(), baseVersion: 1) + XCTFail("Expected a version conflict") + } catch let error as AppSettingsError { + XCTAssertEqual(error, .versionConflict) + } catch { + XCTFail("Expected AppSettingsError.versionConflict, got \(error)") + } + } + + func test_givenUnregisteredDevice_whenWritingDeviceSettings_thenThrowsDeviceNotRegistered() async { + // A 404 from the WRITE route means the device is missing from the + // registry, not that the document is absent — `baseVersion: 0` creates + // one happily. Mapping it to "empty" the way the read path does would + // silently discard the user's settings. + let api = StubAPIClient() + await api.enqueue(failure: .notFound(serverMessage: "device not registered")) + let service = makeService(api) + + do { + _ = try await service.writeDeviceSettings(AppSettingsBag(), deviceID: "ghost", baseVersion: 0) + XCTFail("Expected deviceNotRegistered") + } catch let error as AppSettingsError { + XCTAssertEqual(error, .deviceNotRegistered) + } catch { + XCTFail("Expected AppSettingsError.deviceNotRegistered, got \(error)") + } + } + // MARK: - First-run 404s // - // Verified live 2026-09-06: the server answers 404 for an app key with + // Verified live 2026-09-16: the server answers 404 for an app key with // nothing stored yet, and 404 `{"source":"none"}` for a device it has not // seen. Both are ordinary first-run states — treating them as errors made a // fresh install show a failure instead of empty settings. - func test_givenNothingStoredYet_whenReadingSharedSettings_thenReturnsEmptyBagNotAnError() async throws { + func test_givenNothingStoredYet_whenReadingSharedSettings_thenReturnsNilNotAnError() async throws { let api = StubAPIClient() await api.enqueue(failure: .notFound(serverMessage: "Not found")) let service = makeService(api) - let bag = try await service.sharedSettings() + let document = try await service.sharedDocument() - XCTAssertTrue(bag.isEmpty) + XCTAssertNil(document) } - func test_givenUnregisteredDevice_whenBootstrapping_thenReturnsEmptySnapshotFlaggedNew() async throws { + func test_givenNothingStoredAnywhere_whenBootstrapping_thenReturnsFirstRunSeed() async throws { let api = StubAPIClient() - await api.enqueue(failure: .notFound(serverMessage: "Not found")) + await api.enqueue(failure: .notFound(serverMessage: #"{"source":"none"}"#)) let service = makeService(api) - let snapshot = try await service.bootstrap(deviceID: "dev-new") + let seed = try await service.bootstrap(deviceID: "dev-new") - XCTAssertTrue(snapshot.shared.isEmpty) - XCTAssertTrue(snapshot.device.isEmpty) - XCTAssertTrue(snapshot.isNewDevice, "the caller registers off this flag") + XCTAssertTrue(seed.isFirstRun) + XCTAssertEqual(seed.origin, .none) + XCTAssertTrue(seed.bag.isEmpty) } - func test_givenNothingStoredYet_whenReadingDeviceSettings_thenReturnsEmptyBag() async throws { + func test_givenNothingStoredYet_whenReadingDeviceSettings_thenReturnsNil() async throws { let api = StubAPIClient() await api.enqueue(failure: .notFound(serverMessage: "Not found")) let service = makeService(api) - let bag = try await service.deviceSettings(deviceID: "dev-1") + let document = try await service.deviceDocument(deviceID: "dev-1") - XCTAssertTrue(bag.isEmpty) + XCTAssertNil(document) } - func test_givenRealFailure_whenReadingSettings_thenStillThrows() async { - // Only 404 is benign. A 500 or a 401 must still reach the UI. + // MARK: - Empty / boundary + + func test_givenNoDevices_whenListing_thenReturnsEmpty() async throws { + // The registry answers 200 with an empty array rather than 404. let api = StubAPIClient() - await api.enqueue(failure: .httpStatus(code: 500, serverMessage: "boom")) + await api.enqueue(json: #"{ "devices": [] }"#) + let service = makeService(api) + + let devices = try await service.devices() + + XCTAssertTrue(devices.isEmpty) + } + + func test_givenMachineWithNoSettings_whenCopyingToShared_thenRefusesRatherThanWipingShared() async throws { + // Copy-to-shared replaces wholesale, so copying "nothing" would delete + // every shared setting. Refuse instead — and make sure no write is sent. + let api = StubAPIClient() + await api.enqueue(failure: .notFound(serverMessage: "Not found")) let service = makeService(api) do { - _ = try await service.sharedSettings() - XCTFail("a server error is not a first-run state") - } catch { - // expected + _ = try await service.copyDeviceSettingsToShared(deviceID: "dev-1") + XCTFail("Expected noSettingsToCopy") + } catch let error as AppSettingsError { + XCTAssertEqual(error, .noSettingsToCopy) } + + let recorded = await api.recorded + XCTAssertEqual(recorded.count, 1, "the read must not be followed by a destructive write") } - // MARK: - Empty / boundary + func test_givenLastDeviceRemoved_whenDeregistering_thenPromotesNobody() async throws { + // Boundary: exactly one registered device, and it is the main + // workstation. Nothing remains to inherit the role. + let api = StubAPIClient() + await api.enqueue(json: #"{"deleted":true,"promotedDeviceId":null}"#) + let service = makeService(api) - func test_givenNoDevices_whenListing_thenReturnsEmpty() async throws { + let outcome = try await service.deregisterDevice(deviceID: "only-one") + + XCTAssertTrue(outcome.deleted) + XCTAssertNil(outcome.promotedDeviceID) + } + + func test_givenNothingToDelete_whenDeletingShared_thenReportsFalseWithoutThrowing() async throws { + // Idempotent: `{"deleted":false}`, not a 404. let api = StubAPIClient() - await api.enqueue(json: #"{ "devices": [] }"#) + await api.enqueue(json: #"{"deleted":false}"#) let service = makeService(api) - let devices = try await service.devices() + let deleted = try await service.deleteSharedSettings() - XCTAssertTrue(devices.isEmpty) + XCTAssertFalse(deleted) } func test_givenEmptySettings_whenFetching_thenReturnsEmptyBag() async throws { let api = StubAPIClient() - await api.enqueue(json: #"{ "settings": {} }"#) + await api.enqueue(json: documentJSON(settings: "{}")) let service = makeService(api) - let bag = try await service.sharedSettings() + let loaded = try await service.sharedDocument() + let document = try XCTUnwrap(loaded) - XCTAssertTrue(bag.isEmpty) + XCTAssertTrue(document.isEmpty) + XCTAssertEqual(document.version, 1) } } diff --git a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift index 791ccb5..494ac25 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift @@ -29,10 +29,21 @@ actor StubAPIClient: APIClientProtocol { /// The encoded JSON body, when the request carried one. /// /// Recorded so a test can assert the **shape** that goes on the wire, - /// not only that a call was made. Without it, `create(…)` sending its - /// schema as a string rather than an object was untestable at this - /// layer — and the server rejects the string outright (GitHub #85). + /// not only that a call was made. A wrong body is as breaking as a wrong + /// path and far quieter, and two shipped defects prove it: `create(…)` + /// sent its schema as a string where the server demands an object + /// (GitHub #85), and the app-settings surface sent `{"name":…}` where + /// the server demands `{"deviceName":…}` (GitHub #56). Every + /// path-and-method assertion passed the whole time, in both cases. let body: Data? + + /// The body decoded for assertions. A computed property, not a stored + /// one, because `[String: Any]` is not `Equatable` and would break the + /// synthesised conformance this type relies on. + var bodyJSON: [String: Any]? { + guard let body else { return nil } + return (try? JSONSerialization.jsonObject(with: body)) as? [String: Any] + } } private var outcomes: [Outcome] = [] @@ -94,9 +105,10 @@ actor StubAPIClient: APIClientProtocol { for item in request.query where item.value != nil { query[item.name] = item.value } - // Encoded with the client's own encoder so key naming and date strategy - // match what really goes out; a body that fails to encode is recorded as - // `nil` rather than failing the recording. + // Encoded with the client's own encoder, so what a test inspects is + // byte-for-byte what would have gone on the wire — key naming and date + // strategy included. A body that fails to encode is recorded as `nil` + // rather than failing the recording. var bodyData: Data? if case .json(let payload)? = request.body { bodyData = try? JSONCoders.makeEncoder().encode(payload) diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/AppSettingsDTO.swift b/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/AppSettingsDTO.swift index f6be2a9..1dff3c7 100644 --- a/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/AppSettingsDTO.swift +++ b/Packages/InterlinedKit/Sources/InterlinedKit/DTOs/AppSettingsDTO.swift @@ -1,200 +1,343 @@ import Foundation -// MARK: - Shared / per-device settings payloads - -/// The stored settings blob for an app key or a device (work-consolidation.md -/// G17). The platform treats the payload as opaque, so it is carried as -/// `[String: AppSettingsValue]` and never narrowed to a fixed struct — see `AppSettingsValue` -/// for why that matters for forward compatibility. -/// -/// ⚠️ **Wire shapes here are UNVERIFIED.** The gap definition names the routes -/// and the semantics (from `/help/app-settings`) but does not record response -/// bodies, and the test account has no registered `appKey` yet. Every envelope -/// below therefore decodes tolerantly: the payload is accepted either under a -/// `settings` key or as the bare top-level object, and every metadata field is -/// optional. Tighten these once a live probe confirms the real shapes. -public struct AppSettingsDTO: Decodable, Sendable, Equatable { - public let settings: [String: AppSettingsValue] +// MARK: - Settings documents + +/// A stored settings document — the `SettingsDoc` schema (work-consolidation.md +/// G17, GitHub issue #56). +/// +/// **Verified live 2026-09-16** against the test account by storing real +/// settings and re-reading them (the shapes were guesses until then; see the +/// issue's "store real settings, then re-probe and tighten" step): +/// +/// ``` +/// GET /api/user/app-settings/interlinedlist-macos +/// -> 200 {"appKey":"interlinedlist-macos","scope":"account","deviceId":null, +/// "version":1,"updatedAt":"2026-09-16T19:39:07.135Z","schemaVersion":1, +/// "settings":{"theme":"dark","sidebarWidth":280}} +/// ``` +/// +/// The document is returned **bare** — there is no `{settings: …}` envelope and +/// no `{document: …}` wrapper. The earlier tolerant decoder accepted both a +/// nested `settings` key and a bare payload-as-body; the bare-body branch is +/// gone because it is now actively wrong: a real bare document has `appKey`, +/// `scope` and `version` at the top level, and treating those as settings keys +/// would write API metadata back into the user's payload on the next PUT. +/// +/// `version` is the one field that cannot be optional. It drives the +/// compare-and-set write protocol — a client that cannot read the version +/// cannot legally write at all (see `WriteAppSettingsRequest`). +public struct AppSettingsDocumentDTO: Decodable, Sendable, Equatable { + + public let appKey: String + /// Observed values: `"account"` for the user-wide document, `"device"` for a + /// per-machine one. Note the OpenAPI *example* says `"user"`; the live API + /// answers `"account"`, and live wins. + public let scope: String + /// `null` on the account-wide document, the owning machine on a device one. + public let deviceId: String? + /// Monotonic version for compare-and-set. Send it back as `baseVersion`. + public let version: Int public let updatedAt: Date? + /// Client-declared schema version of `settings`; the server stores whatever + /// we send and defaults it to 1. + public let schemaVersion: Int? + /// Opaque, client-owned JSON — stored and returned verbatim, never + /// interpreted by the server. See `AppSettingsValue` for why this is not a + /// concrete struct. + public let settings: [String: AppSettingsValue] - public init(settings: [String: AppSettingsValue], updatedAt: Date? = nil) { - self.settings = settings + public init( + appKey: String, + scope: String, + deviceId: String? = nil, + version: Int, + updatedAt: Date? = nil, + schemaVersion: Int? = nil, + settings: [String: AppSettingsValue] = [:] + ) { + self.appKey = appKey + self.scope = scope + self.deviceId = deviceId + self.version = version self.updatedAt = updatedAt + self.schemaVersion = schemaVersion + self.settings = settings } +} - public init(from decoder: Decoder) throws { - let c = try decoder.container(keyedBy: CodingKeys.self) - if let nested = try? c.decodeIfPresent([String: AppSettingsValue].self, forKey: .settings) { - self.settings = nested - self.updatedAt = try? c.decodeIfPresent(Date.self, forKey: .updatedAt) - return - } - // Bare object: the whole body *is* the settings payload. Strip the - // metadata keys so they don't masquerade as settings. - let bare = (try? decoder.singleValueContainer().decode([String: AppSettingsValue].self)) ?? [:] - var stripped = bare - stripped.removeValue(forKey: CodingKeys.updatedAt.rawValue) - self.settings = stripped - // The bare path decodes through `AppSettingsValue`, which bypasses the - // shared date strategy, so parse the timestamp explicitly here. - self.updatedAt = bare[CodingKeys.updatedAt.rawValue]?.stringValue.flatMap(JSONCoders.parseDate) +/// `PUT` body for a shared or per-device settings document. +/// +/// **`baseVersion` is mandatory.** Verified live 2026-09-16: omitting it — +/// exactly what this type used to do — fails every write outright with +/// `400 {"error":"baseVersion must be an integer >= 0","code":"bad_request"}`. +/// It is therefore a non-optional initialiser parameter: there is no valid way +/// to construct a write that the server would accept without one. +/// +/// Send `0` to create a document that does not exist yet; send the `version` +/// you last read to update one. A stale value answers `409 version_conflict` +/// and writes nothing. +public struct WriteAppSettingsRequest: Encodable, Sendable, Equatable { + public let settings: [String: AppSettingsValue] + public let baseVersion: Int + public let schemaVersion: Int? + + public init( + settings: [String: AppSettingsValue], + baseVersion: Int, + schemaVersion: Int? = nil + ) { + self.settings = settings + self.baseVersion = baseVersion + self.schemaVersion = schemaVersion } +} + +/// `DELETE /api/user/app-settings/{appKey}` response. +/// +/// Verified live 2026-09-16: the delete is **idempotent and never 404s** — +/// `{"deleted":true}` when a document was removed, `{"deleted":false}` when +/// there was nothing to remove. That is why the delete path needs none of the +/// 404-means-empty mapping the read paths do. +public struct DeleteAppSettingsResponse: Decodable, Sendable, Equatable { + public let deleted: Bool - private enum CodingKeys: String, CodingKey { case settings, updatedAt } + public init(deleted: Bool) { self.deleted = deleted } } -/// `PUT` body for shared or per-device settings — the payload, wrapped. -public struct WriteAppSettingsRequest: Encodable, Sendable, Equatable { - public let settings: [String: AppSettingsValue] +// MARK: - Bootstrap + +/// Where a bootstrap answer came from — the server's seeding precedence chain. +/// +/// Verified live 2026-09-16 by driving every branch on the test account. +public enum AppSettingsSource: String, Decodable, Sendable, Equatable { + /// This device's own stored document. + case own = "self" + /// Seeded from the main workstation's document, because this device has + /// none of its own. + case defaultDevice = "default-device" + /// Seeded from the account-wide document, because the main workstation has + /// no document either. + case account + /// Nothing stored anywhere yet. Arrives as `404 {"source":"none"}`. + case none +} + +/// `GET …/bootstrap?deviceId=…` — the single launch call. +/// +/// ⚠️ **This shape is nothing like what the type previously modelled.** The old +/// decoder expected `{shared: …, device: …, isNewDevice, seededFromMainWorkstation}`; +/// none of those keys exist. Every field decoded to empty on every launch, and +/// silently, because the decoder tolerated absence. Verified live 2026-09-16: +/// +/// ``` +/// GET …/bootstrap?deviceId= +/// -> 200 {"source":"self", …SettingsDoc…} +/// +/// GET …/bootstrap?deviceId= +/// -> 200 {"source":"default-device", …the MAIN WORKSTATION's SettingsDoc…, +/// "defaultDeviceId":"probe-mac-alpha","defaultDeviceName":"Probe Alpha"} +/// +/// GET …/bootstrap?deviceId= +/// -> 200 {"source":"account", …the ACCOUNT SettingsDoc…} +/// +/// GET …/bootstrap?deviceId= +/// -> 404 {"source":"none"} +/// ``` +/// +/// The real contract is **one document plus a provenance tag**, not a pair of +/// payloads. Note that `deviceId` on the returned document is the *source* +/// device, not the device asked about — a `default-device` answer carries the +/// main workstation's id. +public struct AppSettingsBootstrapDTO: Decodable, Sendable, Equatable { + + public let source: AppSettingsSource + /// Absent only when `source == .none`. + public let document: AppSettingsDocumentDTO? + /// Which machine the settings were seeded from, on a `default-device` answer. + public let defaultDeviceId: String? + public let defaultDeviceName: String? + + public init( + source: AppSettingsSource, + document: AppSettingsDocumentDTO? = nil, + defaultDeviceId: String? = nil, + defaultDeviceName: String? = nil + ) { + self.source = source + self.document = document + self.defaultDeviceId = defaultDeviceId + self.defaultDeviceName = defaultDeviceName + } + + /// The document fields are inlined alongside `source`, so the document is + /// decoded from the *same* container rather than a nested one. + public init(from decoder: Decoder) throws { + let c = try decoder.container(keyedBy: CodingKeys.self) + self.source = try c.decodeIfPresent(AppSettingsSource.self, forKey: .source) ?? .none + self.defaultDeviceId = try c.decodeIfPresent(String.self, forKey: .defaultDeviceId) + self.defaultDeviceName = try c.decodeIfPresent(String.self, forKey: .defaultDeviceName) + // `source: "none"` carries no document at all; anything else must. + self.document = source == .none ? nil : try? AppSettingsDocumentDTO(from: decoder) + } - public init(settings: [String: AppSettingsValue]) { self.settings = settings } + private enum CodingKeys: String, CodingKey { + case source, defaultDeviceId, defaultDeviceName + } } // MARK: - Device registry -/// One registered device (machine) under an app key. +/// One machine registered under an app key. +/// +/// **Verified live 2026-09-16** — and the field names are not the ones this +/// type used to decode. The live row is: +/// +/// ``` +/// {"deviceId":"probe-mac-alpha","deviceName":"Probe Alpha","platform":"macos", +/// "isDefault":true,"lastSeenAt":"2026-09-16T19:38:42.214Z", +/// "appVersion":null,"osVersion":null,"hasDeviceSettings":true} +/// ``` /// -/// `isMainWorkstation` is the flag behind the "main workstation seeds a -/// brand-new device on first sign-in" behaviour described in the gap definition. +/// The previous decoder looked for `name`/`deviceLabel` and +/// `isMainWorkstation`/`isMain`. **None of those keys exist**, so every row +/// rendered with its raw device id as the display name and the main-workstation +/// badge never appeared for anyone. Both were silent — the fields were optional, +/// so nothing threw. The speculative aliases are gone: they never matched +/// anything, and keeping them would hide the next such mismatch just as +/// effectively. +/// +/// `isDefault` is the wire spelling of "main workstation"; the domain layer +/// renames it, because `isDefault` says nothing about what it defaults *to*. public struct AppDeviceDTO: Decodable, Sendable, Equatable { + public let deviceId: String - public let name: String? - public let isMainWorkstation: Bool? - public let createdAt: Date? + public let deviceName: String + /// The main-workstation flag. Exactly one device per app key carries it. + public let isDefault: Bool + public let platform: String? public let lastSeenAt: Date? + public let appVersion: String? + public let osVersion: String? + /// Whether this machine has a per-device settings document of its own. + /// **Only present on the list route** — the single-device envelope returned + /// by POST and PATCH omits it, which is why it is optional. + public let hasDeviceSettings: Bool? public init( deviceId: String, - name: String? = nil, - isMainWorkstation: Bool? = nil, - createdAt: Date? = nil, - lastSeenAt: Date? = nil + deviceName: String, + isDefault: Bool = false, + platform: String? = nil, + lastSeenAt: Date? = nil, + appVersion: String? = nil, + osVersion: String? = nil, + hasDeviceSettings: Bool? = nil ) { self.deviceId = deviceId - self.name = name - self.isMainWorkstation = isMainWorkstation - self.createdAt = createdAt + self.deviceName = deviceName + self.isDefault = isDefault + self.platform = platform self.lastSeenAt = lastSeenAt - } - - /// Accepts `deviceId` or a plain `id`, and `name` or `deviceLabel` — the - /// two naming conventions already seen elsewhere on this API (`SessionDTO` - /// uses `deviceLabel`). - public init(from decoder: Decoder) throws { - let c = try decoder.container(keyedBy: CodingKeys.self) - let id = try c.decodeIfPresent(String.self, forKey: .deviceId) - ?? c.decodeIfPresent(String.self, forKey: .id) - guard let id else { - throw DecodingError.keyNotFound( - CodingKeys.deviceId, - .init(codingPath: decoder.codingPath, debugDescription: "device has neither deviceId nor id") - ) - } - self.deviceId = id - self.name = try c.decodeIfPresent(String.self, forKey: .name) - ?? c.decodeIfPresent(String.self, forKey: .deviceLabel) - self.isMainWorkstation = try c.decodeIfPresent(Bool.self, forKey: .isMainWorkstation) - ?? c.decodeIfPresent(Bool.self, forKey: .isMain) - self.createdAt = try c.decodeIfPresent(Date.self, forKey: .createdAt) - self.lastSeenAt = try c.decodeIfPresent(Date.self, forKey: .lastSeenAt) - } - - private enum CodingKeys: String, CodingKey { - case deviceId, id, name, deviceLabel, isMainWorkstation, isMain, createdAt, lastSeenAt + self.appVersion = appVersion + self.osVersion = osVersion + self.hasDeviceSettings = hasDeviceSettings } } -/// `GET /api/user/app-settings/{appKey}/devices` response — named envelope or -/// bare array. +/// `GET …/devices` — verified live to answer `{"devices":[…]}`. public struct AppDevicesResponse: Decodable, Sendable, Equatable { public let devices: [AppDeviceDTO] public init(devices: [AppDeviceDTO]) { self.devices = devices } +} - public init(from decoder: Decoder) throws { - if let single = try? decoder.singleValueContainer(), - let bare = try? single.decode([AppDeviceDTO].self) { - self.devices = bare - return - } - let c = try decoder.container(keyedBy: CodingKeys.self) - self.devices = try c.decodeIfPresent([AppDeviceDTO].self, forKey: .devices) ?? [] - } +/// `POST …/devices` and `PATCH …/devices/{deviceId}` — both answer the device +/// wrapped in a `device` key, not bare. +/// +/// Verified live 2026-09-16. The endpoints previously decoded `AppDeviceDTO` +/// directly, so register and rename both threw a decoding error on a perfectly +/// successful `200` — the id is one level down from where the decoder looked. +/// (POST answers `200`, incidentally, not the `201` the OpenAPI spec advertises.) +public struct AppDeviceEnvelope: Decodable, Sendable, Equatable { + public let device: AppDeviceDTO - private enum CodingKeys: String, CodingKey { case devices } + public init(device: AppDeviceDTO) { self.device = device } } /// `POST …/devices` body — register this machine. +/// +/// `deviceName` and `platform` are the live field names; `platform` is required +/// by the schema and was absent from the old request type entirely. public struct RegisterDeviceRequest: Encodable, Sendable, Equatable { public let deviceId: String - public let name: String? + public let deviceName: String + public let platform: String + public let appVersion: String? + public let osVersion: String? - public init(deviceId: String, name: String? = nil) { + public init( + deviceId: String, + deviceName: String, + platform: String = "macos", + appVersion: String? = nil, + osVersion: String? = nil + ) { self.deviceId = deviceId - self.name = name + self.deviceName = deviceName + self.platform = platform + self.appVersion = appVersion + self.osVersion = osVersion } } /// `PATCH …/devices/{deviceId}` body — rename, or promote to main workstation. -/// Both fields are optional so a caller sends only what it is changing. +/// +/// The field names are `deviceName` and `isDefault`. Verified live 2026-09-16 — +/// and the server states the contract itself when you get it wrong, which is how +/// the previous spelling was caught: +/// +/// ``` +/// PATCH …/devices/probe-mac-beta {"name":"…"} -> 400 +/// PATCH …/devices/probe-mac-beta {"isMainWorkstation":true} -> 400 +/// {"error":"at least one of deviceName or isDefault is required","code":"bad_request"} +/// ``` +/// +/// So **both** shipped mutations — rename and promote — failed on every call. +/// Both fields stay optional so a caller sends only what it is changing: a PATCH +/// that also carried `deviceName` would clobber a rename made on another machine +/// between this client's read and its write. public struct UpdateDeviceRequest: Encodable, Sendable, Equatable { - public let name: String? - public let isMainWorkstation: Bool? + public let deviceName: String? + public let isDefault: Bool? - public init(name: String? = nil, isMainWorkstation: Bool? = nil) { - self.name = name - self.isMainWorkstation = isMainWorkstation + public init(deviceName: String? = nil, isDefault: Bool? = nil) { + self.deviceName = deviceName + self.isDefault = isDefault } } -// MARK: - Bootstrap - -/// `GET …/bootstrap?deviceId=…` response — the one call a launching client makes. +/// `DELETE …/devices/{deviceId}` response. /// -/// Carries the account-wide shared settings plus this machine's own settings; -/// when the device is brand new the server seeds the per-device payload from the -/// main workstation (the gap definition's stated behaviour), which is what -/// `seededFromMainWorkstation` reports. -public struct AppSettingsBootstrapDTO: Decodable, Sendable, Equatable { - public let shared: [String: AppSettingsValue] - public let device: [String: AppSettingsValue] - public let isNewDevice: Bool? - public let seededFromMainWorkstation: Bool? - - public init( - shared: [String: AppSettingsValue] = [:], - device: [String: AppSettingsValue] = [:], - isNewDevice: Bool? = nil, - seededFromMainWorkstation: Bool? = nil - ) { - self.shared = shared - self.device = device - self.isNewDevice = isNewDevice - self.seededFromMainWorkstation = seededFromMainWorkstation - } - - /// Accepts `shared`/`sharedSettings` and `device`/`deviceSettings`. - public init(from decoder: Decoder) throws { - let c = try decoder.container(keyedBy: CodingKeys.self) - // Split out of an inline `??` chain: the nested optionals from - // `try? decodeIfPresent` made the expression too costly to type-check. - func payload(_ primary: CodingKeys, _ alternate: CodingKeys) -> [String: AppSettingsValue] { - let type = [String: AppSettingsValue].self - if let found = try? c.decodeIfPresent(type, forKey: primary) { - return found - } - if let found = try? c.decodeIfPresent(type, forKey: alternate) { - return found - } - return [:] - } - self.shared = payload(.shared, .sharedSettings) - self.device = payload(.device, .deviceSettings) - self.isNewDevice = try? c.decodeIfPresent(Bool.self, forKey: .isNewDevice) - self.seededFromMainWorkstation = try? c.decodeIfPresent(Bool.self, forKey: .seededFromMainWorkstation) - } +/// Verified live 2026-09-16 — and this is the useful part: **the server names +/// the machine it promoted**, so removing the main workstation does not require +/// guessing which machine inherited the role. +/// +/// ``` +/// DELETE …/devices/probe-mac-beta (beta was main, alpha also registered) +/// -> 200 {"deleted":true,"promotedDeviceId":"probe-mac-alpha"} +/// +/// DELETE …/devices/probe-mac-alpha (alpha was main and the only device left) +/// -> 200 {"deleted":true,"promotedDeviceId":null} +/// ``` +/// +/// `promotedDeviceId` is null both when the removed device was not the main +/// workstation and when no device remains to promote. +public struct DeleteDeviceResponse: Decodable, Sendable, Equatable { + public let deleted: Bool + public let promotedDeviceId: String? - private enum CodingKeys: String, CodingKey { - case shared, sharedSettings, device, deviceSettings, isNewDevice, seededFromMainWorkstation + public init(deleted: Bool, promotedDeviceId: String? = nil) { + self.deleted = deleted + self.promotedDeviceId = promotedDeviceId } } diff --git a/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/AppSettingsEndpoint.swift b/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/AppSettingsEndpoint.swift index e47c583..da679ec 100644 --- a/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/AppSettingsEndpoint.swift +++ b/Packages/InterlinedKit/Sources/InterlinedKit/Endpoints/AppSettingsEndpoint.swift @@ -1,38 +1,47 @@ import Foundation /// Request builders for **Applications: synced settings + device registry** -/// (work-consolidation.md G17) — the platform's own mechanism for companion -/// apps, and the sanctioned home for this app's preferences *and* the Document -/// Sync Agent's per-machine configuration, replacing purely local +/// (work-consolidation.md G17, GitHub issue #56) — the platform's own mechanism +/// for companion apps, and the sanctioned home for this app's preferences *and* +/// the Document Sync Agent's per-machine configuration, replacing purely local /// `UserDefaults` state. /// -/// The model, per `/help/app-settings`: -/// - **shared settings** follow the account to every machine; -/// - **per-device settings** stay pinned to one computer; -/// - one machine is the **main workstation**, whose config seeds a brand-new -/// device on first sign-in; -/// - devices can be renamed or deregistered. +/// The model, per `/help/app-settings` and confirmed by live probe 2026-09-16: +/// - **account-scoped settings** follow the account to every machine; +/// - **device-scoped settings** stay pinned to one computer; +/// - one machine is the **main workstation** (`isDefault` on the wire), whose +/// config seeds a new device on first sign-in; +/// - devices can be renamed, promoted, or deregistered. /// -/// ⚠️ **The `appKey` must be registered with the backend owner before this -/// ships** (stated prerequisite in the gap definition). `AppSettings` takes the -/// key as a parameter rather than hard-coding one, so registering a different -/// key later is a one-line change at the composition root. +/// The `appKey` is **not** registered with the backend — the segment is a +/// free-form namespace, proven by probe (an invented key answers +/// `200 {"devices":[]}` rather than rejecting). It is nonetheless passed in +/// rather than hard-coded, because the value in use +/// (`AppEnvironment.appSettingsKey` = `"interlinedlist-macos"`) **must stay +/// stable**: changing it orphans every setting already stored under the old one. /// -/// ⚠️ Response shapes are **unverified** — see the note on `AppSettingsDTO`. +/// Every settings write is **compare-and-set** — see `WriteAppSettingsRequest`. public enum AppSettings { - // MARK: - Shared (account-wide) settings + // MARK: - Account-wide settings - /// `GET /api/user/app-settings/{appKey}` — the account-wide shared settings. - public static func shared(appKey: String) -> Request { + /// `GET /api/user/app-settings/{appKey}` — the account-wide document. + /// + /// 404 when nothing is stored yet. That is the ordinary first-run answer, + /// not a failure; the domain layer maps it to "no document". + public static func shared(appKey: String) -> Request { Request(method: .get, path: "/api/user/app-settings/\(appKey)", auth: .bearer) } - /// `PUT /api/user/app-settings/{appKey}` — replace the shared settings. + /// `PUT /api/user/app-settings/{appKey}` — compare-and-set replace. + /// + /// This **replaces** the document; it does not merge. Verified live: a PUT + /// carrying only `{"theme":"light"}` over a stored + /// `{"theme":"dark","sidebarWidth":280}` left `sidebarWidth` deleted. public static func writeShared( appKey: String, _ body: WriteAppSettingsRequest - ) -> Request { + ) -> Request { Request( method: .put, path: "/api/user/app-settings/\(appKey)", @@ -41,16 +50,19 @@ public enum AppSettings { ) } - /// `DELETE /api/user/app-settings/{appKey}` — drop all settings for the app. - public static func deleteShared(appKey: String) -> Request { + /// `DELETE /api/user/app-settings/{appKey}` — drop the account-wide + /// document. Idempotent; never 404s. Leaves per-device documents alone. + public static func deleteShared(appKey: String) -> Request { Request(method: .delete, path: "/api/user/app-settings/\(appKey)", auth: .bearer) } // MARK: - Bootstrap /// `GET /api/user/app-settings/{appKey}/bootstrap?deviceId=…` — the single - /// launch call: shared settings + this machine's settings, seeding a new - /// device from the main workstation. + /// launch call: the best available settings document for this machine, plus + /// a `source` saying where it came from. + /// + /// `deviceId` is required; omitting it is `400 {"error":"Invalid deviceId"}`. public static func bootstrap(appKey: String, deviceId: String) -> Request { Request( method: .get, @@ -63,16 +75,21 @@ public enum AppSettings { // MARK: - Device registry /// `GET /api/user/app-settings/{appKey}/devices` — every machine registered - /// under this app key. + /// under this app key. Answers `200 {"devices":[]}` when there are none, so + /// unlike the settings routes this one never 404s on a fresh account. public static func devices(appKey: String) -> Request { Request(method: .get, path: "/api/user/app-settings/\(appKey)/devices", auth: .bearer) } /// `POST /api/user/app-settings/{appKey}/devices` — register this machine. + /// + /// The **first** device registered is made the main workstation + /// automatically (verified live: the first POST answered `isDefault:true`, + /// the second `isDefault:false`). public static func registerDevice( appKey: String, _ body: RegisterDeviceRequest - ) -> Request { + ) -> Request { Request( method: .post, path: "/api/user/app-settings/\(appKey)/devices", @@ -83,11 +100,14 @@ public enum AppSettings { /// `PATCH /api/user/app-settings/{appKey}/devices/{deviceId}` — rename a /// device or promote it to main workstation. + /// + /// Promotion demotes the previous holder server-side; verified live by + /// promoting a second device and re-listing. public static func updateDevice( appKey: String, deviceId: String, _ body: UpdateDeviceRequest - ) -> Request { + ) -> Request { Request( method: .patch, path: "/api/user/app-settings/\(appKey)/devices/\(deviceId)", @@ -97,7 +117,12 @@ public enum AppSettings { } /// `DELETE /api/user/app-settings/{appKey}/devices/{deviceId}` — deregister. - public static func deleteDevice(appKey: String, deviceId: String) -> Request { + /// + /// Deletes that machine's per-device settings along with it (verified live: + /// re-registering the same id afterwards read back `404` for its settings), + /// and leaves the account-wide document untouched. The response names any + /// machine auto-promoted to fill a vacated main-workstation role. + public static func deleteDevice(appKey: String, deviceId: String) -> Request { Request( method: .delete, path: "/api/user/app-settings/\(appKey)/devices/\(deviceId)", @@ -107,8 +132,9 @@ public enum AppSettings { // MARK: - Per-device settings - /// `GET …/devices/{deviceId}/settings` — one machine's pinned settings. - public static func deviceSettings(appKey: String, deviceId: String) -> Request { + /// `GET …/devices/{deviceId}/settings` — one machine's pinned document. + /// 404 when that machine has never written one. + public static func deviceSettings(appKey: String, deviceId: String) -> Request { Request( method: .get, path: "/api/user/app-settings/\(appKey)/devices/\(deviceId)/settings", @@ -116,12 +142,17 @@ public enum AppSettings { ) } - /// `PUT …/devices/{deviceId}/settings` — replace one machine's settings. + /// `PUT …/devices/{deviceId}/settings` — compare-and-set replace. + /// + /// A 404 here means **the device is not registered** + /// (`{"error":"device not registered"}`), not "no document yet" — + /// `baseVersion: 0` creates the document happily. Retrying will not fix it; + /// registering the device will. public static func writeDeviceSettings( appKey: String, deviceId: String, _ body: WriteAppSettingsRequest - ) -> Request { + ) -> Request { Request( method: .put, path: "/api/user/app-settings/\(appKey)/devices/\(deviceId)/settings", diff --git a/Packages/InterlinedKit/Tests/InterlinedKitTests/AppSettingsEndpointTests.swift b/Packages/InterlinedKit/Tests/InterlinedKitTests/AppSettingsEndpointTests.swift index 470d27a..b2369ed 100644 --- a/Packages/InterlinedKit/Tests/InterlinedKitTests/AppSettingsEndpointTests.swift +++ b/Packages/InterlinedKit/Tests/InterlinedKitTests/AppSettingsEndpointTests.swift @@ -2,11 +2,13 @@ import XCTest @testable import InterlinedKit /// BDD tests for the app-settings + device-registry endpoints -/// (work-consolidation.md G17). +/// (work-consolidation.md G17, GitHub issue #56). /// -/// The live response shapes are unverified, so the decode tests deliberately -/// cover *both* envelope conventions this API has been seen to use (named key -/// vs. bare) rather than pinning one guess. +/// Every payload below is a **verbatim live response** captured on 2026-09-16 +/// against the test account, by storing real settings and registering real +/// devices and then re-reading them. The shapes were guesses before that; the +/// guesses were wrong in five separate places, so these fixtures are pinned to +/// what the server actually sends rather than to what seemed reasonable. final class AppSettingsEndpointTests: XCTestCase { private let baseURL = URL(string: "https://stub.local")! @@ -28,7 +30,10 @@ final class AppSettingsEndpointTests: XCTestCase { XCTAssertEqual(AppSettings.shared(appKey: appKey).path, "/api/user/app-settings/interlinedlist-macos") XCTAssertEqual(AppSettings.shared(appKey: appKey).method, .get) - let write = AppSettings.writeShared(appKey: appKey, WriteAppSettingsRequest(settings: [:])) + let write = AppSettings.writeShared( + appKey: appKey, + WriteAppSettingsRequest(settings: [:], baseVersion: 0) + ) XCTAssertEqual(write.method, .put) XCTAssertEqual(AppSettings.deleteShared(appKey: appKey).method, .delete) @@ -38,134 +43,294 @@ final class AppSettingsEndpointTests: XCTestCase { XCTAssertTrue(bootstrap.query.contains(.string("deviceId", "dev-1"))) XCTAssertEqual(AppSettings.devices(appKey: appKey).path, "/api/user/app-settings/interlinedlist-macos/devices") - XCTAssertEqual(AppSettings.registerDevice(appKey: appKey, RegisterDeviceRequest(deviceId: "dev-1")).method, .post) - XCTAssertEqual(AppSettings.updateDevice(appKey: appKey, deviceId: "dev-1", UpdateDeviceRequest(name: "Mac")).method, .patch) + XCTAssertEqual( + AppSettings.registerDevice( + appKey: appKey, + RegisterDeviceRequest(deviceId: "dev-1", deviceName: "Mac") + ).method, + .post + ) + XCTAssertEqual( + AppSettings.updateDevice(appKey: appKey, deviceId: "dev-1", UpdateDeviceRequest(deviceName: "Mac")).method, + .patch + ) XCTAssertEqual(AppSettings.deleteDevice(appKey: appKey, deviceId: "dev-1").method, .delete) XCTAssertEqual( AppSettings.deviceSettings(appKey: appKey, deviceId: "dev-1").path, "/api/user/app-settings/interlinedlist-macos/devices/dev-1/settings" ) XCTAssertEqual( - AppSettings.writeDeviceSettings(appKey: appKey, deviceId: "dev-1", WriteAppSettingsRequest(settings: [:])).method, + AppSettings.writeDeviceSettings( + appKey: appKey, + deviceId: "dev-1", + WriteAppSettingsRequest(settings: [:], baseVersion: 3) + ).method, .put ) // Every builder is Bearer. XCTAssertEqual(AppSettings.devices(appKey: appKey).auth, .bearer) } + // MARK: - Request bodies + + func test_givenWriteRequest_whenEncoded_thenCarriesBaseVersion() throws { + // Omitting `baseVersion` is a hard 400 — "baseVersion must be an + // integer >= 0" — so it must appear on the wire, including when it is + // the create-sentinel 0, which `encodeIfPresent` semantics would be apt + // to drop. + let body = WriteAppSettingsRequest(settings: ["theme": .string("dark")], baseVersion: 0) + + let json = try JSONSerialization.jsonObject( + with: JSONCoders.makeEncoder().encode(body) + ) as? [String: Any] + + XCTAssertEqual(json?["baseVersion"] as? Int, 0) + XCTAssertNotNil(json?["settings"]) + } + + func test_givenDeviceUpdate_whenEncoded_thenUsesDeviceNameAndIsDefault() throws { + // The server rejects anything else outright: "at least one of + // deviceName or isDefault is required". + let rename = try JSONSerialization.jsonObject( + with: JSONCoders.makeEncoder().encode(UpdateDeviceRequest(deviceName: "Studio Mac")) + ) as? [String: Any] + XCTAssertEqual(rename?["deviceName"] as? String, "Studio Mac") + XCTAssertNil(rename?["isDefault"], "a rename must not also send the promotion flag") + + let promote = try JSONSerialization.jsonObject( + with: JSONCoders.makeEncoder().encode(UpdateDeviceRequest(isDefault: true)) + ) as? [String: Any] + XCTAssertEqual(promote?["isDefault"] as? Bool, true) + XCTAssertNil(promote?["deviceName"], "a promotion must not clobber a rename from another machine") + } + // MARK: - Happy path - func test_givenWrappedSettings_whenSharedSent_thenDecodesPayload() async throws { + func test_givenStoredSettings_whenSharedSent_thenDecodesBareDocument() async throws { let (client, transport) = makeClient() + // Verbatim live body, 2026-09-16. await transport.enqueue(.json(#""" - { "settings": { "theme": "dark", "postsPerPage": 25, "syncEnabled": true }, - "updatedAt": "2026-09-05T10:00:00Z" } + { "appKey": "interlinedlist-macos", "scope": "account", "deviceId": null, + "version": 1, "updatedAt": "2026-09-16T19:39:07.135Z", "schemaVersion": 1, + "settings": { "theme": "dark", "sidebarWidth": 280 } } """#)) let dto = try await client.send(AppSettings.shared(appKey: appKey)) + XCTAssertEqual(dto.version, 1) + XCTAssertEqual(dto.scope, "account") + XCTAssertNil(dto.deviceId) XCTAssertEqual(dto.settings["theme"]?.stringValue, "dark") - XCTAssertEqual(dto.settings["postsPerPage"]?.intValue, 25) - XCTAssertEqual(dto.settings["syncEnabled"]?.boolValue, true) - XCTAssertEqual(dto.updatedAt, JSONCoders.parseDate("2026-09-05T10:00:00Z")) + XCTAssertEqual(dto.settings["sidebarWidth"]?.intValue, 280) + XCTAssertEqual(dto.updatedAt, JSONCoders.parseDate("2026-09-16T19:39:07.135Z")) + // Metadata must never leak into the opaque payload and get written back. + XCTAssertNil(dto.settings["appKey"]) + XCTAssertNil(dto.settings["version"]) } func test_givenDevicesBody_whenSent_thenDecodesRegistry() async throws { let (client, transport) = makeClient() await transport.enqueue(.json(#""" { "devices": [ - { "deviceId": "dev-1", "name": "Studio Mac", "isMainWorkstation": true, - "lastSeenAt": "2026-09-05T09:00:00Z" }, - { "id": "dev-2", "deviceLabel": "Laptop", "isMain": false } + { "deviceId": "probe-mac-beta", "deviceName": "Probe Beta", "platform": "macos", + "isDefault": false, "lastSeenAt": "2026-09-16T19:38:50.810Z", + "appVersion": null, "osVersion": null, "hasDeviceSettings": false }, + { "deviceId": "probe-mac-alpha", "deviceName": "Probe Alpha", "platform": "macos", + "isDefault": true, "lastSeenAt": "2026-09-16T19:38:42.214Z", + "appVersion": null, "osVersion": null, "hasDeviceSettings": true } ] } """#)) let response = try await client.send(AppSettings.devices(appKey: appKey)) XCTAssertEqual(response.devices.count, 2) - XCTAssertEqual(response.devices[0].deviceId, "dev-1") - XCTAssertEqual(response.devices[0].isMainWorkstation, true) - // Second row uses the alternate `id` / `deviceLabel` / `isMain` spelling. - XCTAssertEqual(response.devices[1].deviceId, "dev-2") - XCTAssertEqual(response.devices[1].name, "Laptop") - XCTAssertEqual(response.devices[1].isMainWorkstation, false) + // `deviceName` and `isDefault` — the previous decoder looked for `name` + // and `isMainWorkstation`, so every row lost its name and its badge. + XCTAssertEqual(response.devices[1].deviceName, "Probe Alpha") + XCTAssertTrue(response.devices[1].isDefault) + XCTAssertFalse(response.devices[0].isDefault) + XCTAssertEqual(response.devices[1].hasDeviceSettings, true) + XCTAssertEqual(response.devices[0].platform, "macos") } - func test_givenBootstrapBody_whenSent_thenSplitsSharedAndDeviceSettings() async throws { + func test_givenRegisterResponse_whenSent_thenDecodesThroughTheDeviceEnvelope() async throws { let (client, transport) = makeClient() + // POST and PATCH both wrap the device; decoding it bare threw on success. await transport.enqueue(.json(#""" - { "shared": { "theme": "dark" }, - "device": { "syncFolder": "/Users/x/Notes" }, - "isNewDevice": true, "seededFromMainWorkstation": true } + { "device": { "deviceId": "probe-mac-alpha", "deviceName": "Probe Alpha", + "platform": "macos", "isDefault": true, + "lastSeenAt": "2026-09-16T19:38:42.214Z", + "appVersion": null, "osVersion": null } } """#)) - let dto = try await client.send(AppSettings.bootstrap(appKey: appKey, deviceId: "dev-9")) + let envelope = try await client.send( + AppSettings.registerDevice( + appKey: appKey, + RegisterDeviceRequest(deviceId: "probe-mac-alpha", deviceName: "Probe Alpha") + ) + ) - XCTAssertEqual(dto.shared["theme"]?.stringValue, "dark") - XCTAssertEqual(dto.device["syncFolder"]?.stringValue, "/Users/x/Notes") - XCTAssertEqual(dto.isNewDevice, true) - XCTAssertEqual(dto.seededFromMainWorkstation, true) + XCTAssertEqual(envelope.device.deviceId, "probe-mac-alpha") + XCTAssertEqual(envelope.device.deviceName, "Probe Alpha") + // The first device registered is made main workstation automatically. + XCTAssertTrue(envelope.device.isDefault) + // Absent on this route — must stay nil rather than default to false. + XCTAssertNil(envelope.device.hasDeviceSettings) } - // MARK: - Invalid / tolerant input + func test_givenMainWorkstationRemoved_whenDeleted_thenReportsThePromotedDevice() async throws { + let (client, transport) = makeClient() + await transport.enqueue(.json(#"{"deleted":true,"promotedDeviceId":"probe-mac-alpha"}"#)) + + let response = try await client.send( + AppSettings.deleteDevice(appKey: appKey, deviceId: "probe-mac-beta") + ) - func test_givenBareSettingsObject_whenSharedSent_thenTreatsBodyAsPayload() async throws { + XCTAssertTrue(response.deleted) + // The server names the successor, so no client ever has to guess. + XCTAssertEqual(response.promotedDeviceId, "probe-mac-alpha") + } + + func test_givenOwnSettings_whenBootstrapping_thenDecodesSourceAndDocument() async throws { let (client, transport) = makeClient() - await transport.enqueue(.json(#"{ "theme": "light", "updatedAt": "2026-09-05T10:00:00Z" }"#)) + await transport.enqueue(.json(#""" + { "source": "self", "appKey": "interlinedlist-macos", "scope": "device", + "deviceId": "probe-mac-alpha", "version": 1, + "updatedAt": "2026-09-16T19:39:18.084Z", "schemaVersion": 1, + "settings": { "windowFrame": "0,0,1440,900", "syncFolderPath": "/Users/probe/Obsidian" } } + """#)) - let dto = try await client.send(AppSettings.shared(appKey: appKey)) + let dto = try await client.send(AppSettings.bootstrap(appKey: appKey, deviceId: "probe-mac-alpha")) + + XCTAssertEqual(dto.source, .own) + // The document fields sit alongside `source`, not nested under a key. + XCTAssertEqual(dto.document?.version, 1) + XCTAssertEqual(dto.document?.settings["syncFolderPath"]?.stringValue, "/Users/probe/Obsidian") + } + + func test_givenNewDevice_whenBootstrapping_thenNamesTheSeedingWorkstation() async throws { + let (client, transport) = makeClient() + await transport.enqueue(.json(#""" + { "source": "default-device", "appKey": "interlinedlist-macos", "scope": "device", + "deviceId": "probe-mac-alpha", "version": 1, + "updatedAt": "2026-09-16T19:39:18.084Z", "schemaVersion": 1, + "settings": { "windowFrame": "0,0,1440,900" }, + "defaultDeviceId": "probe-mac-alpha", "defaultDeviceName": "Probe Alpha" } + """#)) - XCTAssertEqual(dto.settings["theme"]?.stringValue, "light") - // Metadata must not leak into the settings payload. - XCTAssertNil(dto.settings["updatedAt"]) - XCTAssertEqual(dto.updatedAt, JSONCoders.parseDate("2026-09-05T10:00:00Z")) + let dto = try await client.send(AppSettings.bootstrap(appKey: appKey, deviceId: "brand-new")) + + XCTAssertEqual(dto.source, .defaultDevice) + XCTAssertEqual(dto.defaultDeviceName, "Probe Alpha") + // `deviceId` on the document is the SOURCE machine, not the one asked + // about — a new Mac is being handed the main workstation's document. + XCTAssertEqual(dto.document?.deviceId, "probe-mac-alpha") } - func test_givenAlternateBootstrapKeys_whenSent_thenStillDecodes() async throws { + func test_givenNoDeviceSettingsAnywhere_whenBootstrapping_thenFallsBackToAccountScope() async throws { let (client, transport) = makeClient() - await transport.enqueue(.json(#"{ "sharedSettings": { "a": 1 }, "deviceSettings": { "b": 2 } }"#)) + await transport.enqueue(.json(#""" + { "source": "account", "appKey": "interlinedlist-macos", "scope": "account", + "deviceId": null, "version": 1, "updatedAt": "2026-09-16T19:39:07.135Z", + "schemaVersion": 1, "settings": { "theme": "dark" } } + """#)) - let dto = try await client.send(AppSettings.bootstrap(appKey: appKey, deviceId: "dev-9")) + let dto = try await client.send(AppSettings.bootstrap(appKey: appKey, deviceId: "brand-new")) - XCTAssertEqual(dto.shared["a"]?.intValue, 1) - XCTAssertEqual(dto.device["b"]?.intValue, 2) + // The precedence chain is own -> main workstation -> account -> none. + XCTAssertEqual(dto.source, .account) + XCTAssertEqual(dto.document?.settings["theme"]?.stringValue, "dark") + XCTAssertNil(dto.document?.deviceId) } + // MARK: - Invalid / tolerant input + func test_givenNestedAndUnknownKeys_whenRoundTripped_thenPreservedVerbatim() throws { // The forward-compatibility guarantee: a payload written by a newer - // build must survive decode -> encode unchanged. - let raw = #"{ "settings": { "known": true, "futureFeature": { "nested": [1, "two", null] } } }"# - let dto = try JSONDecoder().decode(AppSettingsDTO.self, from: Data(raw.utf8)) - - let reEncoded = try JSONEncoder().encode(WriteAppSettingsRequest(settings: dto.settings)) - let round = try JSONDecoder().decode(AppSettingsDTO.self, from: reEncoded) + // build must survive decode -> encode unchanged, or this client silently + // deletes settings it does not understand. + let raw = #""" + { "appKey": "k", "scope": "account", "version": 2, "updatedAt": "2026-09-16T19:39:07.135Z", + "settings": { "known": true, "futureFeature": { "nested": [1, "two", null] } } } + """# + let dto = try JSONCoders.makeDecoder().decode(AppSettingsDocumentDTO.self, from: Data(raw.utf8)) + + let reEncoded = try JSONCoders.makeEncoder().encode( + WriteAppSettingsRequest(settings: dto.settings, baseVersion: dto.version) + ) + let sent = try JSONSerialization.jsonObject(with: reEncoded) as? [String: Any] + let settings = sent?["settings"] as? [String: Any] + let nested = (settings?["futureFeature"] as? [String: Any])?["nested"] as? [Any] - XCTAssertEqual(round.settings["known"]?.boolValue, true) - let nested = round.settings["futureFeature"]?["nested"]?.arrayValue + XCTAssertEqual(settings?["known"] as? Bool, true) XCTAssertEqual(nested?.count, 3) - XCTAssertEqual(nested?[1].stringValue, "two") - XCTAssertEqual(nested?[2], .null) + XCTAssertEqual(nested?[1] as? String, "two") + // And the version read is the version written back. + XCTAssertEqual(sent?["baseVersion"] as? Int, 2) + } + + func test_givenBlankDeviceName_whenDecoded_thenStillDecodesRatherThanThrowing() async throws { + // `deviceName` is required by the schema and always present live, but an + // empty string must not take the whole registry down with it — the + // domain layer substitutes the id for display. + let (client, transport) = makeClient() + await transport.enqueue(.json(#""" + { "devices": [ { "deviceId": "dev-1", "deviceName": "", "isDefault": false } ] } + """#)) + + let response = try await client.send(AppSettings.devices(appKey: appKey)) + + XCTAssertEqual(response.devices.first?.deviceName, "") + XCTAssertNil(response.devices.first?.lastSeenAt) } // MARK: - API failure - func test_givenUnregisteredAppKey_whenSent_thenThrowsNotFoundWithMessage() async throws { - // 404 maps to `.notFound`, not `.httpStatus` — the client narrows the - // well-known statuses and carries the server's message through. + func test_givenNothingStored_whenSharedSent_thenThrowsNotFoundWithMessage() async throws { + // 404 is the ordinary first-run answer for an app key with nothing + // stored; the domain layer maps it to "no document". It must still + // arrive as `.notFound` and not `.httpStatus`. let (client, transport) = makeClient() - await transport.enqueue(.json(#"{"error":"unknown app key"}"#, status: 404)) + await transport.enqueue(.json(#"{"error":"Not found","code":"not_found"}"#, status: 404)) do { - _ = try await client.send(AppSettings.shared(appKey: "not-registered")) + _ = try await client.send(AppSettings.shared(appKey: appKey)) XCTFail("Expected notFound") } catch let error as APIError { - XCTAssertEqual(error, .notFound(serverMessage: "unknown app key")) + XCTAssertEqual(error, .notFound(serverMessage: "Not found")) + } + } + + func test_givenStaleBaseVersion_whenWriting_thenSurfacesConflictStatus() async throws { + // A lost compare-and-set. 409 is not one of the statuses APIError + // narrows, so it must arrive as `.httpStatus(409)` for the domain layer + // to translate — nothing was written. + let (client, transport) = makeClient() + await transport.enqueue(.json(#""" + { "error": "version_conflict", "code": "version_conflict", + "current": { "appKey": "interlinedlist-macos", "scope": "account", "deviceId": null, + "version": 1, "updatedAt": "2026-09-16T19:39:07.135Z", + "schemaVersion": 1, "settings": { "theme": "dark" } } } + """#, status: 409)) + + do { + _ = try await client.send( + AppSettings.writeShared( + appKey: appKey, + WriteAppSettingsRequest(settings: [:], baseVersion: 0) + ) + ) + XCTFail("Expected a 409") + } catch let error as APIError { + XCTAssertEqual(error.httpStatusCode, 409) } } // MARK: - Empty / boundary func test_givenNoDevices_whenSent_thenReturnsEmptyRegistry() async throws { + // Verified live: the registry answers 200 with an empty array on a fresh + // account rather than 404, unlike the settings routes. let (client, transport) = makeClient() await transport.enqueue(.json(#"{ "devices": [] }"#)) @@ -174,13 +339,25 @@ final class AppSettingsEndpointTests: XCTestCase { XCTAssertTrue(response.devices.isEmpty) } - func test_givenEmptySettings_whenSent_thenDecodesEmptyPayload() async throws { + func test_givenNothingToDelete_whenDeletingShared_thenReportsNotDeletedRatherThan404() async throws { + // Verified live: this delete is idempotent — `{"deleted":false}`, no 404. let (client, transport) = makeClient() - await transport.enqueue(.json(#"{ "settings": {} }"#)) + await transport.enqueue(.json(#"{"deleted":false}"#)) - let dto = try await client.send(AppSettings.shared(appKey: appKey)) + let response = try await client.send(AppSettings.deleteShared(appKey: appKey)) + + XCTAssertFalse(response.deleted) + } + + func test_givenNothingStoredAnywhere_whenBootstrapping_thenSourceIsNoneWithNoDocument() async throws { + // The 404 first-run body. `source` still decodes, and there is no + // document to speak of. + let dto = try JSONCoders.makeDecoder().decode( + AppSettingsBootstrapDTO.self, + from: Data(#"{"source":"none"}"#.utf8) + ) - XCTAssertTrue(dto.settings.isEmpty) - XCTAssertNil(dto.updatedAt) + XCTAssertEqual(dto.source, .none) + XCTAssertNil(dto.document) } }