From 79b2a6cce1cb36d1007bc5541d2912f74469158e Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 13:00:57 -0700 Subject: [PATCH] feat(settings): finish the Applications pane and fix its wire contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verify pass this issue asked for found the Applications/Devices pane in worse shape than "some actions missing". Storing real settings on the test account and re-probing every route exposed that the read/write half had never worked: the DTOs were written from the gap definition before any populated payload existed, and five of those guesses were wrong. Nothing caught it because nothing in production consumed them — only `deregisterDevice` was ever wired to a view. What the live probe (2026-09-16) actually showed: * `PUT` requires `baseVersion` in the body. Without it the server answers 400 outright, so every settings write this app could make was broken. The whole family is compare-and-set: a stale version answers 409 and writes nothing. * Devices carry `deviceName` and `isDefault`. The decoder looked for `name`/`deviceLabel` and `isMainWorkstation`/`isMain`, so every row would have rendered as its raw UUID with no main-workstation badge, silently. * `PATCH` accepts `deviceName` and `isDefault` — and says so when you get it wrong. Both shipped mutations, rename and promote, were sending fields the server rejects with a 400. * `POST` and `PATCH` wrap the device in a `device` key. Decoding it bare threw a decoding error on a successful 200. * `bootstrap` returns one document plus a `source` tag, not the shared/device pair the DTO modelled. Every field decoded to empty, always. So the DTOs are rewritten against captured payloads rather than tightened, and the speculative key aliases are gone: they never matched anything, and keeping them would hide the next mismatch just as well as they hid these. On top of that the three documented actions with no UI at all are now built — inspecting the shared and per-machine documents with their last-updated and size, copying a machine's settings to shared, and deleting the shared settings. Each confirmation states the real consequence, because neither "removal takes the machine's own settings with it" nor "copy replaces rather than merges" is inferable from the button. Two semantics are driven by evidence rather than assumption. Removing the main workstation returns `promotedDeviceId` naming the successor, so the client applies it instead of guessing; only when the server names nobody and the reconcile read also fails does the badge fall back to "unknown", which is the one case where showing the old flag would assert something now false. And this Mac is registered on first open of the pane but never again: `POST` is an upsert keyed on `deviceId`, so re-registering would overwrite `deviceName` and quietly undo the user's rename on every visit. The 409's `current` document is deliberately not plumbed through. Surfacing it means teaching `APIClient` to carry typed error bodies, which touches every endpoint's error path; re-reading costs one request and the blob is opaque, so there is nothing to merge field-by-field anyway. `appSettingsKey` is unchanged, as required — it is the namespace every stored setting lives under. Refs #56 Co-Authored-By: Claude Opus 5 --- App/Features/Settings/DevicesView.swift | 251 ++++++++-- App/Features/Settings/DevicesViewModel.swift | 237 ++++++++-- App/Features/Settings/SettingsRootView.swift | 9 +- AppTests/DevicesViewModelTests.swift | 293 +++++++++++- .../Support/StubSettingsClusterServices.swift | 123 ++++- .../Models/AppSettingsBag.swift | 220 +++++++-- .../Services/AppSettingsService.swift | 304 +++++++++--- .../AppSettingsServiceTests.swift | 348 ++++++++++---- .../Support/StubAPIClient.swift | 27 +- .../InterlinedKit/DTOs/AppSettingsDTO.swift | 433 ++++++++++++------ .../Endpoints/AppSettingsEndpoint.swift | 93 ++-- .../AppSettingsEndpointTests.swift | 309 ++++++++++--- 12 files changed, 2107 insertions(+), 540 deletions(-) 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 f8fd8a7..d03ccce 100644 --- a/App/Features/Settings/SettingsRootView.swift +++ b/App/Features/Settings/SettingsRootView.swift @@ -52,11 +52,14 @@ struct SettingsRootView: View { Label("Security", systemImage: "lock.shield") } - // 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") } // Document sync agent (work-consolidation.md §3b) — enable the background helper 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 a0d042f..20e1f4f 100644 --- a/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift +++ b/Packages/InterlinedDomain/Tests/InterlinedDomainTests/Support/StubAPIClient.swift @@ -26,6 +26,20 @@ actor StubAPIClient: APIClientProtocol { let method: String let path: String let query: [String: String] + /// The encoded JSON body, when the request had one. + /// + /// Captured because a wrong *body* is as breaking as a wrong path and + /// far quieter: the app-settings surface shipped for weeks sending + /// `{"name":…}` where the server demanded `{"deviceName":…}`, and every + /// path-and-method assertion passed the whole time. + let body: Data? + + /// The body decoded for assertions. Not `Equatable`, so it stays out of + /// the stored properties. + var bodyJSON: [String: Any]? { + guard let body else { return nil } + return (try? JSONSerialization.jsonObject(with: body)) as? [String: Any] + } } private var outcomes: [Outcome] = [] @@ -87,8 +101,19 @@ actor StubAPIClient: APIClientProtocol { for item in request.query where item.value != nil { query[item.name] = item.value } + // Encoded with the production encoder, so what a test inspects is + // byte-for-byte what the client would have put on the wire. + var body: Data? + if case .json(let payload) = request.body { + body = try? JSONCoders.makeEncoder().encode(payload) + } recorded.append( - RecordedRequest(method: request.method.rawValue, path: request.path, query: query) + RecordedRequest( + method: request.method.rawValue, + path: request.path, + query: query, + body: body + ) ) } } 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) } }