diff --git a/apps/mac/Sources/XBotCore/AppState.swift b/apps/mac/Sources/XBotCore/AppState.swift index 17f70cf..4b8ccc2 100644 --- a/apps/mac/Sources/XBotCore/AppState.swift +++ b/apps/mac/Sources/XBotCore/AppState.swift @@ -912,11 +912,15 @@ public final class AppState { /// /// Returns when the data is gone. Moving the app itself to the Trash is the person's to do, /// and the screen says so. - public func uninstall() async { + /// False when the container runtime could not be reached. Then nothing is removed — not the + /// keys, not the preferences — so the person can start Docker and try again from the same + /// place, rather than be left with an app that has forgotten everything except the gigabytes. + @discardableResult + public func uninstall() async -> Bool { isEngineBusy = true defer { isEngineBusy = false } - await runtime?.uninstall() + if let runtime, await !runtime.uninstall() { return false } // Every key this app has ever written. Listed rather than enumerated, because a wildcard // sweep of the login keychain is not something this app should ever perform. @@ -958,6 +962,7 @@ public final class AppState { questionsForYou = [:] engineHealth = nil engineBaseURL = nil + return true } public static var appVersion: String { diff --git a/apps/mac/Sources/XBotRuntime/FakeDriver.swift b/apps/mac/Sources/XBotRuntime/FakeDriver.swift index f881a14..4135afa 100644 --- a/apps/mac/Sources/XBotRuntime/FakeDriver.swift +++ b/apps/mac/Sources/XBotRuntime/FakeDriver.swift @@ -122,7 +122,12 @@ public actor FakeDriver: ContainerDriver { public func createVolume(_ name: String) async throws { volumes.insert(name) } public func volumeExists(_ name: String) async -> Bool { volumes.contains(name) } - public func removeVolume(_ name: String) async throws { volumes.remove(name) } + /// Refused while the daemon is down, as the real one is: with no daemon there is nobody to + /// delete anything, and a fake that said otherwise hid exactly that from the uninstall tests. + public func removeVolume(_ name: String) async throws { + guard case .ready = await probe() else { throw RuntimeError.daemonUnavailable } + volumes.remove(name) + } /// Records the exec and writes a stand-in file, so a caller that depends on the file existing /// is exercised rather than only the call being counted. diff --git a/apps/mac/Sources/XBotRuntime/RuntimeController.swift b/apps/mac/Sources/XBotRuntime/RuntimeController.swift index 579f71d..ee497ed 100644 --- a/apps/mac/Sources/XBotRuntime/RuntimeController.swift +++ b/apps/mac/Sources/XBotRuntime/RuntimeController.swift @@ -590,7 +590,13 @@ public actor RuntimeController { /// Every step tolerates work already done. An uninstall that fails halfway and cannot be run /// again leaves exactly the orphaned state it exists to prevent, so a missing container or an /// already-deleted volume is not an error. - public func uninstall() async { + /// Whether the engine's data is gone. False when the runtime could not be reached, in which case + /// nothing was removed and the caller must not report otherwise. + @discardableResult + public func uninstall() async -> Bool { + // Wake the runtime first. With no daemon every removal below fails inside a `try?`, and the + // volumes — the conversations, agents and browser logins — stay exactly where they were. + guard await ensureRuntimeReady() else { return false } await stop() if let handle = await driver.containerNamed(Self.engineContainerName) { try? await driver.remove(handle) @@ -604,6 +610,11 @@ public actor RuntimeController { // Back to stopped, not notDetected: the runtime is still installed and still working — it // is only xBot's own data that is gone. state = .stopped + // Measured rather than assumed: a volume that is still there is data that is still there. + for volume in [Self.dataVolume, Self.workspaceVolume, Self.profilesVolume] { + if await driver.volumeExists(volume) { return false } + } + return true } /// What the running engine is using. Nil when it is not running or Docker would not say. diff --git a/apps/mac/Sources/XBotUI/Settings/SettingsRootView.swift b/apps/mac/Sources/XBotUI/Settings/SettingsRootView.swift index f96ecad..a78b056 100644 --- a/apps/mac/Sources/XBotUI/Settings/SettingsRootView.swift +++ b/apps/mac/Sources/XBotUI/Settings/SettingsRootView.swift @@ -9,6 +9,7 @@ public struct AdvancedSettingsView: View { @State private var isConfirmingUninstall = false @State private var hasUninstalled = false + @State private var uninstallFailed = false public var body: some View { Form { @@ -79,6 +80,17 @@ public struct AdvancedSettingsView: View { ) .foregroundStyle(Palette.textSecondary) } else { + if uninstallFailed { + // Said, not skipped: the alternative was announcing success over every + // volume still on disk. Keys and preferences are kept, so a retry works. + Text( + String( + localized: + "Your data couldn't be removed, because Docker didn't respond. Make sure Docker Desktop or Colima is running, then try again." + ) + ) + .foregroundStyle(Palette.textSecondary) + } Button(String(localized: "Remove all xBot data…"), role: .destructive) { isConfirmingUninstall = true } @@ -107,8 +119,9 @@ public struct AdvancedSettingsView: View { ) { Button(String(localized: "Remove everything"), role: .destructive) { Task { - await state.uninstall() - hasUninstalled = true + let removed = await state.uninstall() + hasUninstalled = removed + uninstallFailed = !removed } } Button(String(localized: "Cancel"), role: .cancel) {} diff --git a/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift b/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift index 8a5a9a9..641f19a 100644 --- a/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift +++ b/apps/mac/Tests/XBotRuntimeTests/RuntimeTests.swift @@ -823,6 +823,52 @@ extension RuntimeControllerTests { #expect(await controller.state == .stopped) } + /** + Uninstall with the runtime stopped wakes it first. + + Docker Desktop quit, or the Colima VM paused — ordinary on a Mac, and likely on the day somebody + decides to remove an app. Every removal used to run against no daemon, fail inside a `try?`, + and leave every volume where it was while the screen said everything had been removed. + */ + @Test func uninstallWakesAStoppedRuntimeBeforeRemovingAnything() async { + let driver = FakeDriver() + let controller = RuntimeController( + driver: driver, + image: ImageReference(repository: "xbot/engine", tag: "1"), + health: { _ in EngineHealth(engineVersion: "0.0.5", schemaVersion: "0000") }, + ports: isolatedPortStore(), + dumpURL: isolatedDumpURL() + ) + await controller.start(environment: environment) + await driver.stopDaemon() + + let removed = await controller.uninstall() + + #expect(removed) + #expect(await driver.daemonStartRequested) + #expect(await driver.remainingVolumes.isEmpty) + } + + /// A runtime that will not come up means nothing was removed, and uninstall has to say so + /// rather than let the app wipe its keys and announce success over volumes it never touched. + @Test func uninstallThatCannotReachTheRuntimeSaysSo() async { + let driver = FakeDriver(script: .init(daemonStartSucceeds: false)) + let controller = RuntimeController( + driver: driver, + image: ImageReference(repository: "xbot/engine", tag: "1"), + health: { _ in EngineHealth(engineVersion: "0.0.5", schemaVersion: "0000") }, + ports: isolatedPortStore(), + dumpURL: isolatedDumpURL() + ) + await controller.start(environment: environment) + await driver.stopDaemon() + + let removed = await controller.uninstall() + + #expect(!removed) + #expect(!(await driver.remainingVolumes.isEmpty)) + } + /// The pre-upgrade dump is the database in plain SQL — agents, settings, everything the volume /// held. It lives outside the volume on purpose, so removing the volumes never touched it, and /// the screen said "Everything xBot stored has been removed" while a copy sat in Application diff --git a/docs/11-packaging-and-updates.md b/docs/11-packaging-and-updates.md index a11346e..72ebe11 100644 --- a/docs/11-packaging-and-updates.md +++ b/docs/11-packaging-and-updates.md @@ -312,6 +312,12 @@ is a bad citizen with a reputation problem. **Does not remove the container runtime**, because the user may have installed it for something else. It says so. +**It wakes the runtime first**, because nothing can be deleted without a daemon, and on the day +somebody removes an app Docker Desktop may well be quit. If the runtime will not come up, or a volume +is still there afterwards, uninstall reports failure and removes **nothing else** — the keys and +preferences stay, so trying again from the same button works. It used to run every removal against +no daemon inside a `try?` and then say everything was gone. + **Also ship a standalone uninstaller script** in the DMG for the user who already dragged the app to the Trash and then found the volumes. It is `Uninstall xBot.command`, copied from `scripts/uninstall-xbot.command`, and it mirrors `AppState.uninstall()` step for step.